Skip to content

bpo-30532: email.policy.SMTP.fold() mangles long headers #1892

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 5 commits into from
Jun 26, 2017
Merged

bpo-30532: email.policy.SMTP.fold() mangles long headers #1892

merged 5 commits into from
Jun 26, 2017

Conversation

jhillacre
Copy link
Contributor

@mention-bot
Copy link

@jhillacre, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bitdancer, @serhiy-storchaka and @ezio-melotti to be potential reviewers.

@@ -343,7 +343,7 @@ def _fold(self, folded):
if ws is not None:
# Peel off the leading whitespace and make it sticky, to
# avoid infinite recursion.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! You could remove the repeated comment from line 340 while you are here.

@jhillacre
Copy link
Contributor Author

@zvyn OK, got it in 3d07dc5.

@jhillacre
Copy link
Contributor Author

Rebased from master so the windows build passes and gives the PR the ✔️.

@jhillacre
Copy link
Contributor Author

Is there anything I need to do to keep the PR moving?

Copy link
Member

@bitdancer bitdancer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small change, and we need a NEWS entry (which should now use the 'blurb' mechanism, see the recent updates to the devguide).

# Peel off the leading whitespace and make it sticky, to
# avoid infinite recursion.
folded.stickyspace = str(part.pop(0))
folded.stickyspace = ws
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this was a case of me "fixing" the code, and missing one spot (for which there obviously wasn't a test). To make this code match other other instances of this logic, you should put an str call around ws.

@jhillacre
Copy link
Contributor Author

@bitdancer, Added your requested changes in ec63c3c and 29f92f0.

Should I be rebasing this pull request to a single commit with the message of bpo-30532: Fix email header value parser dropping folding white space in certain cases.?

@bitdancer bitdancer merged commit b350c22 into python:master Jun 26, 2017
@bitdancer
Copy link
Member

Nope, it gets squashed during merge. Now it needs backport to 3.6, and I think 3.5 is still open for backports. You can do that (see devguide), or someone else will, since it isn't (yet) automated.

@jhillacre
Copy link
Contributor Author

added backport PRs #2591 #2592

@serhiy-storchaka serhiy-storchaka added the type-bug An unexpected behavior, bug, or error label Jul 6, 2017
@jhillacre jhillacre deleted the bpo-30532 branch May 15, 2018 22:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants