[Django] #31784: When sending emails with a name over 75 characters long new lines are introduced which is incompatible with the latest minor versions of python 3.6/7/8
Django
introduced which is incompatible with the latest minor versions of python
3.6/7/8
---------------------------------------+------------------------
Reporter: Nick Orr | Owner: nobody
Type: Bug | Status: new
Component: Core (Mail) | Version: 2.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
---------------------------------------+------------------------
In the process of sending an email the addresses is sanatized:
django/core/mail/message.py:98 => def sanitize_address(addr, encoding)
The Name portion is encoded via the Header class
email.header.Header.encode which will introduce newlines at 75 characters.
Unfortunately the most recent python security update no longer allows that
to happen. So when Address(nm, addr_spec=addr) is called in
sanitize_address a new error is raised from the Python Standard library.
The update to python can be found here:
https://github.com/python/cpython/commit/f91a0b6df14d6c5133fe3d5889fad7d84fc0c046
#diff-3c5a266cd05e7d4173bf110ee93edd16
Essentially Django can no longer send emails with names longer then 75
chracters.
--
Ticket URL: <https://code.djangoproject.com/ticket/31784>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
versions of python 3.6/7/8.
-----------------------------+--------------------------------------
Reporter: Nick Orr | Owner: nobody
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* status: new => closed
* resolution: => invalid
Comment:
`name-addr` like other message headers has line length limit (see
[https://tools.ietf.org/html/rfc2822#section-2.1.1 RFC 2822]) on the other
hand it cannot contain CR and LF newline characters because they will be
vulnerable for header injection attacks (see
[https://bugs.python.org/issue39073 issue39073]). As far as I'm aware
there is not much we can do to keep it safe and allowed for the `name-
addr` longer than 78 chars.
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:1>
Django
python.
-----------------------------+--------------------------------------
Reporter: Nick Orr | Owner: nobody
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:2>
Django
python.
-----------------------------+--------------------------------------
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Nick Orr):
RFC 2822:
> Each line of characters MUST be no more than 998 characters, and SHOULD
be no more than 78 characters
Instead of Django being ok with feeding an underlying Python library data
that will cause an exception as with all things Python it seems like
Django would be better off getting out of the way and letting the users
shoot themselves in the foot.
I'd suggest that this call:
nm = Header(nm, encoding).encode()
Include the actual RFC line length limit of 998 in either the class
instantiation or the encode method as either of those accept maxlinelen as
a kwarg.
Or as a non-changing compromise it could be hived off into setting with
the default being 75 as it stands now. That would allow developers to
overwrite the setting if needed. Though it would also add yet another
setting for people to know about.
Either way the current implementation is likely to cause a great deal of
pain as developers find out that an email addressed to: "Django with a
really long name for reasons that make sense in the context of a project
<nor...@djangoproject.com>" are inexplicably crashing in core Python code
they've never seen before.
Most of us would rather send a wonky, if technically correct, email versus
just having that email vanish into the ether.
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:3>
Django
python.
-----------------------------+--------------------------------------
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
Hi Nick.
If this is an issue, surely it's an issue in the Python stdlib? Is there a
discussion there that we can point to?
What do the Python core devs says when you quote that line of RFC 2822?
Since this is a security sensitive issue, we can't bypass the stdlib
implementation without being 100% sure that the judgement is correct. (If
we were able to make that case, surely it would be convincing for the
stdlib implementation too?)
**Maybe** there's an issue for Django here, but we need to pursue the
Python course first. (Perhaps folks are already raising it there...?)
I hope that makes sense.
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:4>
Django
python.
-----------------------------+--------------------------------------
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Carlton Gibson):
> Include the actual RFC line length limit of 998 in either the class
instantiation or the encode method as either of those accept maxlinelen as
a kwarg.
Maybe we could do this.
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:5>
Django
python.
-----------------------------+--------------------------------------
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Nick Orr):
https://github.com/python/cpython/pull/19007/files
This issue is caused by a recently issued patch for Python as linked
above. As far as I can tell the standard library does not really care
about the length in this context. And you are correct we wouldn't want to
bypass using the stdlib here we might just not want to insert newlines
when they aren't necessarily required.
I haven't brought it up with the Python folks but from their point of view
I'm sure they see their implementation as fine. More so since the
headerregistry.py file in question has this in its preamble:
> Eventually HeaderRegistry will be a public API, but it isn't yet, and
will probably change some before that happens.
Just for clarity this is the line in question in Django.
https://github.com/django/django/blob/master/django/core/mail/message.py#L99
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:6>
Django
python.
-----------------------------+--------------------------------------
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* cc: Joachim Jablon (added)
Comment:
Joachim, Can I ask for your opinion?
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:7>
Django
python.
-----------------------------+--------------------------------------
Type: Bug | Status: new
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Florian Apolloner):
* status: closed => new
* resolution: invalid =>
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:8>
Django
python.
-----------------------------+------------------------------------
Type: Bug | Status: new
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Florian Apolloner):
* cc: Florian Apolloner (added)
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:9>
Django
python.
-----------------------------+------------------------------------
Type: Bug | Status: new
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
@Nick: How are you generating those emails in the first place currently? I
nowhere see a possibility to specify an actual name in
https://docs.djangoproject.com/en/3.0/topics/email/
I also cannot reproduce it with:
{{{
In [12]: e = EmailMessage('subject', 'content', 'fr...@example.com',
['t...@example.com' * 99])
In [13]: e.message()
Out[13]: <django.core.mail.message.SafeMIMEText at 0x7fc900437050>
In [14]: e.message().as_bytes()
Out[14]: b'Content-Type: text/plain; charset="utf-8"\nMIME-Version: 1.0
\nContent-Transfer-Encoding: 7bit\nSubject: subject\nFrom:
fr...@example.com\nTo: \n
t...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.comto@exampl...@example.com\nDate:
Tue, 14 Jul 2020 20:41:37 -0000\nMessage-ID:
<159475929766.21330.4162456845040318158@apollo13>\n\ncontent'
}}}
I assume you are manually injecting `to` headers?
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:10>
Django
python.
-----------------------------+------------------------------------
Type: Bug | Status: new
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Florian Apolloner):
Oh, it needs a non-ascii character to trigger encoding in the first place:
{{{
from django.conf import settings
settings.configure()
from django.core.mail.message import EmailMessage
e = EmailMessage('subject', 'content', 'fr...@example.com', ['"TestUser
ä%s" <t...@example.com>' % ('0' * 100)])
print(e.message().as_string())
}}}
raises
{{{
Traceback (most recent call last):
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 62, in forbid_multi_line_headers
val.encode('ascii')
UnicodeEncodeError: 'ascii' codec can't encode character '\xe4' in
position 10: ordinal not in range(128)
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "testing.py", line 7, in <module>
print(e.message().as_string())
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 242, in message
self._set_list_header_if_not_empty(msg, 'To', self.to)
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 397, in _set_list_header_if_not_empty
msg[header] = value
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 154, in __setitem__
name, val = forbid_multi_line_headers(name, val, self.encoding)
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 65, in forbid_multi_line_headers
val = ', '.join(sanitize_address(addr, encoding) for addr in
getaddresses((val,)))
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 65, in <genexpr>
val = ', '.join(sanitize_address(addr, encoding) for addr in
getaddresses((val,)))
File "/home/florian/sources/django.git/django/core/mail/message.py",
line 107, in sanitize_address
parsed_address = Address(nm, username=localpart, domain=domain)
File "/usr/lib64/python3.7/email/headerregistry.py", line 37, in
__init__
raise ValueError("invalid arguments; address parts cannot contain CR
or LF")
ValueError: invalid arguments; address parts cannot contain CR or LF
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:11>
Django
python.
-----------------------------+------------------------------------
Type: Bug | Status: assigned
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* owner: nobody => felixxm
* status: new => assigned
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/13192 Draft PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:15>
Django
python.
-----------------------------+------------------------------------
Reporter: Nick Orr | Owner: felixxm
Type: Bug | Status: assigned
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Joachim Jablon):
@felixmm I appreciate the ping but I’m afraid I’m not familiar enough with
the intricacies of that part of the email RFCs to be really of any help.
Florian’s patch seems to make quite some sense given the bug, I’m not sure
of a compelling reason not to go in this direction.
I’ll review the PR :)
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:16>
Django
python.
Reporter: Nick Orr | Owner: Florian Apolloner
Type: Bug | Status: assigned
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Ready for checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by felixxm):
* owner: felixxm => Florian Apolloner
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:17>
Django
python.
-----------------------------+---------------------------------------------
Reporter: Nick Orr | Owner: Florian Apolloner
Type: Bug | Status: assigned
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"f405954ea24dcce7ed01e488a0778be7e441b757" f405954]:
{{{
#!CommitTicketReference repository=""
revision="f405954ea24dcce7ed01e488a0778be7e441b757"
Refs #31784 -- Added test for preventing header injection in display name
of email addresses.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:18>
Django
python.
-----------------------------+---------------------------------------------
Reporter: Nick Orr | Owner: Florian Apolloner
Component: Core (Mail) | Version: 2.2
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak <felisiak.mariusz@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"96a3ea39ef0790dbc413dde0a3e19f6a769356a2" 96a3ea39]:
{{{
#!CommitTicketReference repository=""
revision="96a3ea39ef0790dbc413dde0a3e19f6a769356a2"
Fixed #31784 -- Fixed crash when sending emails on Python 3.6.11+, 3.7.8+,
and 3.8.4+.
Fixed sending emails crash on email addresses with display names longer
then 75 chars on Python 3.6.11+, 3.7.8+, and 3.8.4+.
Wrapped display names were passed to email.headerregistry.Address()
what caused raising an exception because address parts cannot contain
CR or LF.
See https://bugs.python.org/issue39073
Co-Authored-By: Mariusz Felisiak <felisiak...@gmail.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:19>
Django
python.
-----------------------------+---------------------------------------------
Reporter: Nick Orr | Owner: Florian Apolloner
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"bfe404deb9dac43b44a14a6c9668e3346a9f5a4c" bfe404de]:
{{{
#!CommitTicketReference repository=""
revision="bfe404deb9dac43b44a14a6c9668e3346a9f5a4c"
[3.1.x] Fixed #31784 -- Fixed crash when sending emails on Python 3.6.11+,
3.7.8+, and 3.8.4+.
Fixed sending emails crash on email addresses with display names longer
then 75 chars on Python 3.6.11+, 3.7.8+, and 3.8.4+.
Wrapped display names were passed to email.headerregistry.Address()
what caused raising an exception because address parts cannot contain
CR or LF.
See https://bugs.python.org/issue39073
Co-Authored-By: Mariusz Felisiak <felisiak...@gmail.com>
Backport of 96a3ea39ef0790dbc413dde0a3e19f6a769356a2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:20>
Django
python.
-----------------------------+---------------------------------------------
Reporter: Nick Orr | Owner: Florian Apolloner
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"ccc088f8ced67a9ea57a6ee1e00964f2cf6baffd" ccc088f8]:
{{{
#!CommitTicketReference repository=""
revision="ccc088f8ced67a9ea57a6ee1e00964f2cf6baffd"
[3.0.x] Fixed #31784 -- Fixed crash when sending emails on Python 3.6.11+,
3.7.8+, and 3.8.4+.
Fixed sending emails crash on email addresses with display names longer
then 75 chars on Python 3.6.11+, 3.7.8+, and 3.8.4+.
Wrapped display names were passed to email.headerregistry.Address()
what caused raising an exception because address parts cannot contain
CR or LF.
See https://bugs.python.org/issue39073
Co-Authored-By: Mariusz Felisiak <felisiak...@gmail.com>
Backport of 96a3ea39ef0790dbc413dde0a3e19f6a769356a2 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:21>
Django
python.
-----------------------------+---------------------------------------------
Reporter: Nick Orr | Owner: Florian Apolloner
Type: Bug | Status: closed
Keywords: | Triage Stage: Ready for checkin
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"1a3835fdf364acb3beebccffbc2aa0c1efe66150" 1a3835f]:
{{{
#!CommitTicketReference repository=""
revision="1a3835fdf364acb3beebccffbc2aa0c1efe66150"
[2.2.x] Fixed #31784 -- Fixed crash when sending emails on Python 3.6.11+,
3.7.8+, and 3.8.4+.
Fixed sending emails crash on email addresses with display names longer
then 75 chars on Python 3.6.11+, 3.7.8+, and 3.8.4+.
Wrapped display names were passed to email.headerregistry.Address()
what caused raising an exception because address parts cannot contain
CR or LF.
See https://bugs.python.org/issue39073
Co-Authored-By: Mariusz Felisiak <felisiak...@gmail.com>
Backport of 96a3ea39ef0790dbc413dde0a3e19f6a769356a2 from master.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/31784#comment:22>