gh-56698: Fix base64-url parsing errors in email headers#136830
gh-56698: Fix base64-url parsing errors in email headers#136830matthieucan wants to merge 4 commits intopython:mainfrom
Conversation
encukou
left a comment
encukou
left a comment
There was a problem hiding this comment.
Thank you! This looks good to me, but, I'm not the expert here.
@bitdancer, could you take a look whenever you have a bit of time?
|
What does the RFC says here? should we be strict or not? should it be added as a defect if it fails? |
The RFC states that I believe we should not be strict - as reported in the original bug issue, some mailers use a different alphabet. A defect is indeed added when that happens. |
There was a problem hiding this comment.
I believe we should not be strict - as reported in the original bug issue, some mailers use a different alphabet
To be clear: we would still reject the bad alphabet because the first base64.b64decode() with validate=True would fail, and then we try to indicate that there is an error, but at this point, we would still fail because we still can't decode right? (and your PR fixes this path)
| b'QW5tZWxkdW5nIE5ldHphbnNjaGx1c3MgU_xkcmluZzNwLmpwZw==', | ||
| b'Anmeldung Netzanschluss S\xfcdring3p.jpg', | ||
| [errors.InvalidBase64CharactersDefect]) | ||
| # Mix of 2 base64 alphabets |
There was a problem hiding this comment.
| # Mix of 2 base64 alphabets | |
| # mix of different base64 alphabets |
I was a bit confused with "2 base64" so let's be a bit more verbose in the comment (in this case the 2 and the 64 from base64 look odd but maybe I'm tired)
Yes, I believe that description is correct |
|
This PR is stale because it has been open for 30 days with no activity. |
4 participants
Accept urlsafe base64 in email headers, as those are sometimes created by email clients.