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
Fixed #35378 -- Added maximum header length for long addresses. #18110
base: main
Are you sure you want to change the base?
Conversation
Added maxheaderlen parameter to email.generator in Django's EmailMessage.message class to limit length of long addresses with special characters while folding using 7bit Content-Transfer-Encoding.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lufafajoshua Thanks for this patch 👍, left small comments.
# Ticket #35378 | ||
# should encode long addresses with special characters using | ||
# 7bit Content-Transfer-Encoding |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We no longer add comments in the tests until its really important. So maybe these can be removed.
s = msg.message().as_bytes() | ||
self.assertIn(b"Content-Transfer-Encoding: 7bit", s) | ||
s = msg.message().as_string() | ||
self.assertIn("Content-Transfer-Encoding: 7bit", s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reverted your changes from message.py
and was expecting this test to fail but surprisingly it passed.
The main issues mentioned in the ticket was the incorrect folding of long address headers when using 7bit Content-Transfer-Encoding , So if you'll check for the Content-Transfer-Encoding: 7bit
in message string it will always be there and the test will never fail.
In your solution, after you set maxheaderlen=0
, it will treat all the long addresses as one string and there wont be any new line (\n) in between the address. So, in my opinion it will be better if you check the encoded long address for a new line symbol in somewhere between the address. If its not there then it means that its no longer folding the long addresses and treating it as one single string. Does that make sense to you?
s = msg.message().as_bytes() | |
self.assertIn(b"Content-Transfer-Encoding: 7bit", s) | |
s = msg.message().as_string() | |
self.assertIn("Content-Transfer-Encoding: 7bit", s) | |
s = msg.message().as_bytes() | |
self.assertIn(b"=?utf-8?b?TmfGsOG7nWkgbmjhuq1uIGEgdmVyeSB2ZXJ5IGxvbmcsIG5hbWU=?= <to@example.com>\n", s) | |
s = msg.message().as_string() | |
self.assertIn("=?utf-8?b?TmfGsOG7nWkgbmjhuq1uIGEgdmVyeSB2ZXJ5IGxvbmcsIG5hbWU=?= <to@example.com>\n", s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, a test update is required 👍
Something like this might work
def test_long_address_folding(self):
msg = EmailMessage(
"Subject",
"Long address with special characters",
"from@example.com",
['"Người nhận a very very long, name" <to@example.com>'],
).message()
msg.policy = policy.default.clone(cte_type="7bit")
self.assertIn(
"=?utf-8?b?TmfGsOG7nWkgbmjhuq1uIGEgdmVyeSB2ZXJ5IGxvbmcs?= name <to@example.com>",
msg.as_bytes().decode("ascii")
)
This would use the example from the discussion.
This also fails and I am not sure adding maxheaderlen=0
is the right solution. It goes against previous design decisions (see ticket-31784) and RFC 2822 stating that
Each line of characters MUST be no more than 998 characters, and SHOULD be no more than 78 characters, excluding the CRLF.
Allowing longer line length was discussed and discouraged in this comment.
Allowing longer lines would solve the problem (no folding needed), but has similar compatibility concerns to using 8bit (or worse).
In short, I think more investigation is needed 🤔
Trac ticket number
ticket-35378
Branch description
Incorrect folding of long address headers with special characters when using 7bit Content-Transfer-Encoding in EmailMessage.
Checklist
main
branch.