Skip to content

Conversation

@msapiro
Copy link
Contributor

@msapiro msapiro commented Jan 20, 2020

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

@peteroupc
Copy link

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:

In the absence of a MIME-Version field, a receiving mail user agent (whether conforming to MIME requirements or not) may optionally choose to interpret the body of the message according to local conventions. Many such conventions are currently in use and it should be noted that in practice non-MIME messages can contain just about anything.

@msapiro
Copy link
Contributor Author

msapiro commented Jan 20, 2020

@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.

@peteroupc
Copy link

Since the header field makes no difference, my previous comment is irrelevant.

if munge_cte:
msg = deepcopy(msg)
msg.replace_header('content-transfer-encoding', munge_cte[0])
if msg.get('content-transfer-encoding') is not None:
Copy link
Member

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.

@warsaw
Copy link
Member

warsaw commented Oct 19, 2020

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.

Copy link
Member

@warsaw warsaw left a 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.

@miss-islington
Copy link
Contributor

@msapiro: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit bf83822 into python:master Oct 19, 2020
@miss-islington
Copy link
Contributor

Thanks @msapiro for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 19, 2020
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>
@bedevere-bot
Copy link

GH-22796 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 19, 2020
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>
@bedevere-bot
Copy link

GH-22797 is a backport of this pull request to the 3.8 branch.

miss-islington added a commit that referenced this pull request Oct 19, 2020
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>
miss-islington added a commit that referenced this pull request Oct 19, 2020
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>
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
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
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