-
-
Notifications
You must be signed in to change notification settings - Fork 34k
bpo-27321 Fix email.generator.py to not replace a non-existent header. #18074
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
Conversation
|
Note that the email message example in the issue you linked has no MIME-Version header field. Thus, its interpretation is effectively outside the scope of MIME. As RFC 2045 sec. 4 says:
|
|
@peteroupc I'm not sure I understand what you are trying to say in your comment. If you are saying the issue is invalid because the message attached to the bug report had no MIME-Version header, that was the actual message that caused the issue and the issue occurs whether or not the message contains a MIME-Version header. If you are saying something else, I don't understand what? It is arguably correct that messages which cause this exception to be thrown are not valid messages, but they exist in the wild and should not cause the exception. In any case, I added a MIME-Version: header to the test message in this PR, but it makes no difference. The test passes with the fix and throws the exception without it. |
|
Since the header field makes no difference, my previous comment is irrelevant. |
Lib/email/generator.py
Outdated
| if munge_cte: | ||
| msg = deepcopy(msg) | ||
| msg.replace_header('content-transfer-encoding', munge_cte[0]) | ||
| if msg.get('content-transfer-encoding') is not None: |
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 generally dislike double negatives, and would recommend switching the tests so that is None is tried first. I will try to make that change if @msapiro allows others to modify this PR.
|
I'm only suggesting backporting this to 3.9 and 3.8 since everything earlier is in security fix only mode, and I don't consider this to be a security bug. |
warsaw
left a comment
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.
LGTM now, so I've turned on automerge.
|
@msapiro: Status check is done, and it's a success ✅ . |
|
Thanks @msapiro for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9. |
pythonGH-18074) This PR replaces pythonGH-1977. The reason for the replacement is two-fold. The fix itself is different is that if the CTE header doesn't exist in the original message, it is inserted. This is important because the new CTE could be quoted-printable whereas the original is implicit 8bit. Also the tests are different. The test_nonascii_as_string_without_cte test in pythonGH-1977 doesn't actually test the issue in that it passes without the fix. The test_nonascii_as_string_without_content_type_and_cte test is improved here, and even though it doesn't fail without the fix, it is included for completeness. Automerge-Triggered-By: @warsaw (cherry picked from commit bf83822) Co-authored-by: Mark Sapiro <mark@msapiro.net>
|
GH-22796 is a backport of this pull request to the 3.9 branch. |
pythonGH-18074) This PR replaces pythonGH-1977. The reason for the replacement is two-fold. The fix itself is different is that if the CTE header doesn't exist in the original message, it is inserted. This is important because the new CTE could be quoted-printable whereas the original is implicit 8bit. Also the tests are different. The test_nonascii_as_string_without_cte test in pythonGH-1977 doesn't actually test the issue in that it passes without the fix. The test_nonascii_as_string_without_content_type_and_cte test is improved here, and even though it doesn't fail without the fix, it is included for completeness. Automerge-Triggered-By: @warsaw (cherry picked from commit bf83822) Co-authored-by: Mark Sapiro <mark@msapiro.net>
|
GH-22797 is a backport of this pull request to the 3.8 branch. |
GH-18074) This PR replaces GH-1977. The reason for the replacement is two-fold. The fix itself is different is that if the CTE header doesn't exist in the original message, it is inserted. This is important because the new CTE could be quoted-printable whereas the original is implicit 8bit. Also the tests are different. The test_nonascii_as_string_without_cte test in GH-1977 doesn't actually test the issue in that it passes without the fix. The test_nonascii_as_string_without_content_type_and_cte test is improved here, and even though it doesn't fail without the fix, it is included for completeness. Automerge-Triggered-By: @warsaw (cherry picked from commit bf83822) Co-authored-by: Mark Sapiro <mark@msapiro.net>
GH-18074) This PR replaces GH-1977. The reason for the replacement is two-fold. The fix itself is different is that if the CTE header doesn't exist in the original message, it is inserted. This is important because the new CTE could be quoted-printable whereas the original is implicit 8bit. Also the tests are different. The test_nonascii_as_string_without_cte test in GH-1977 doesn't actually test the issue in that it passes without the fix. The test_nonascii_as_string_without_content_type_and_cte test is improved here, and even though it doesn't fail without the fix, it is included for completeness. Automerge-Triggered-By: @warsaw (cherry picked from commit bf83822) Co-authored-by: Mark Sapiro <mark@msapiro.net>
pythonGH-18074) This PR replaces python#1977. The reason for the replacement is two-fold. The fix itself is different is that if the CTE header doesn't exist in the original message, it is inserted. This is important because the new CTE could be quoted-printable whereas the original is implicit 8bit. Also the tests are different. The test_nonascii_as_string_without_cte test in python#1977 doesn't actually test the issue in that it passes without the fix. The test_nonascii_as_string_without_content_type_and_cte test is improved here, and even though it doesn't fail without the fix, it is included for completeness. Automerge-Triggered-By: @warsaw
This PR replaces #1977. The reason for the replacement is two-fold.
The fix itself is different is that if the CTE header doesn't exist in the original message, it is inserted. This is important because the new CTE could be quoted-printable whereas the original is implicit 8bit.
Also the tests are different. The test_nonascii_as_string_without_cte test in #1977 doesn't actually test the issue in that it passes without the fix. The test_nonascii_as_string_without_content_type_and_cte test is improved here, and even though it doesn't fail without the fix, it is included for completeness.
https://bugs.python.org/issue27321
Automerge-Triggered-By: @warsaw