Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Refs #34900 -- Fixed SafeMIMEText.set_payload() crash on Python 3.13. #17979

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

felixxm
Copy link
Member

@felixxm felixxm commented Mar 15, 2024

Payloads with surrogates are passed to the set_payload() since python/cpython@f97f25e.

ticket-34900

Logs

Payloads with surrogates are passed to the set_payload() since
python/cpython@f97f25e
@felixxm felixxm requested a review from a team March 15, 2024 06:52
@felixxm
Copy link
Member Author

felixxm commented Mar 15, 2024

@hramezani Thanks for checking 👍

@felixxm felixxm merged commit b231bcd into django:main Mar 15, 2024
@felixxm felixxm deleted the mail-surrogates branch March 15, 2024 11:52
@awais786
Copy link

awais786 commented Apr 17, 2024

@felixxm can you cherry-pick this commit for django4.2 ? since this issue is appearing on python3.11.9 also.

@charettes
Copy link
Member

charettes commented Apr 17, 2024

@awais786 no, it doesn't qualify per our backporting policy

Scratch that, it was indeed backported 0d3ddca and should be part of the next 4.2.x release.

@nessita
Copy link
Contributor

nessita commented Apr 17, 2024

@awais786 no, it doesn't qualify per our backporting policy

Scratch that, it was indeed backported 0d3ddca and should be part of the next 4.2.x release.

Yes, we made an exception since this is a compatibility issue with supported versions of Python.

@aliceh75
Copy link

aliceh75 commented Apr 17, 2024

Hi all 😄 Does anyone know if there's a workaround until 5.0.5 gets released ?

@nessita
Copy link
Contributor

nessita commented Apr 17, 2024

Hi all 😄 Does anyone know if there's a workaround until 5.0.5 gets released ?

The next release for both 4.2.12 and 5.0.5 is planned for May, 6th. If you need a workaround sooner than that, you can apply the diff from 2e6ae1e in your Django installation.

@aliceh75
Copy link

aliceh75 commented Apr 18, 2024

Thanks @nessita !

For anyone else needing a workaround:

  • Probably best option is to stick to a Python version which doesn't have the issue - I can confirm the issue isn't present in 3.11.8 (but appears in 3.11.9)
  • Second best option is to patch Django installation as suggested by @nessita

Neither of these were simple for us due to the way our containers are build. Less ideal, but much simpler to implement, was to monkeypatch Django. Given that there is a known fix released in two weeks, we assessed it was ok to do so.

We monkeypatched from the ready signal of the relevant app's AppConfig:

from django.apps import AppConfig


class TheRelevantApp(AppConfig):
    default_auto_field = "django.db.models.BigAutoField"
    name = "therelevantapp"

    def ready(self):
        # This is a patch to fix this issue: https://github.com/django/django/pull/17979
        # The fix will come out with Django 5.0.5 on May 6th, so we should remove the patch
        # when we upgrade 
        import django
        from email import charset as Charset
        from email.mime.text import MIMEText

        utf8_charset_qp = Charset.Charset("utf-8")
        utf8_charset = Charset.Charset("utf-8")
        RFC5322_EMAIL_LINE_LENGTH_LIMIT = 998

        class SafeMIMEText(django.core.mail.message.SafeMIMEText):
            def set_payload(self, payload, charset=None):
                if charset == "utf-8" and not isinstance(charset, Charset.Charset):
                    has_long_lines = any(
                        len(line.encode(errors="surrogateescape")) > RFC5322_EMAIL_LINE_LENGTH_LIMIT
                        for line in payload.splitlines()
                    )
                    # Quoted-Printable encoding has the side effect of shortening long
                    # lines, if any (#22561).
                    charset = utf8_charset_qp if has_long_lines else utf8_charset
                MIMEText.set_payload(self, payload, charset=charset)

        django.core.mail.message.SafeMIMEText = SafeMIMEText

PS: this will fail if you use mypy and django-stubs, as the django-stubs plugin actually executes this code, but in that context django.core.mail doesn't exist ! You can fix that by only executing this code if hasattr(django.core, "mail")

henrikskog added a commit to dotkom/onlineweb4 that referenced this pull request Apr 21, 2024
Mail sending in the current version of django has a bug with some versions of python. 3.8 should be compatible.

See django/django#17979 (comment)
henrikhorluck added a commit to dotkom/onlineweb4 that referenced this pull request Apr 21, 2024
* fix email sending bug

Mail sending in the current version of django has a bug with some versions of python. 3.8 should be compatible.

See django/django#17979 (comment)

* fix wrong version

* Update Dockerfile

---------

Co-authored-by: Henrik Hørlück Berg <henrik@horluck.no>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.