gh-91400: make sure email parsing dont unquote realnames with spaces by carlbordum · Pull Request #92638 · python/cpython (original) (raw)

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

This way it describes the observable behavior change. Rather than the more broad wording that'd make a guarantee we don't actually test exhaustively for.

running it a second time when a round tripped identical value has already been guaranteed doesn't do anything. Though it was meaningful during the original bug report, in correct code it is not.

any objections from folks on email-team?

there is a caveat to this bug fix: anyone who had been working around it by doing their own test for characters and adding ""s when it wouldn't already be quoted by our existing code will now wind up with extra properly escaped " characters within their result.

that doesn't seem like a bad problem though, it is a valid address. Attempting to detect and handle that as a special case would be messy logic and lead to potentially worse outcomes of intended ""s that should be escaped not being preserved.

Choose a reason for hiding this comment

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

any objections from folks on email-team?

~Two years later and no objections, let's merge this.

Thanks for working on this, but see my analysis on the issue. The problem isn't with formataddr, which does the right thing, it's with parseaddr.

Choose a reason for hiding this comment

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

formataddr does the right thing. It's parseaddr that would need a fix, as well as _headervalueparser.

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.