Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32k
gh-81074: Allow non-ASCII addr_spec in email.headerregistry.Address#122477
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
The email.headerregistry.Address constructor raised an error ifaddr_spec contained a non-ASCII character. (But it fully supportsnon-ASCII in the separate username and domain args.) This changeremoves the error for a non-ASCII addr_spec.
ghost commentedJul 30, 2024 • edited by ghost
Loading Uh oh!
There was an error while loading.Please reload this page.
edited by ghost
Uh oh!
There was an error while loading.Please reload this page.
All commit authors signed the Contributor License Agreement. |
Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
@bitdancer, as theemail expert, do you want to review this? |
Yes, I'll add it to my list. It will take me quite some time to get through the backlog, unfortunately. |
| if not isinstance(defect, errors.NonASCIILocalPartDefect) | ||
| ] | ||
| if relevant_defects: | ||
| raise relevant_defects[0] |
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 started to write that this should have a comment because the context was lacking, but when I wrote it I realized what this really means is that NonASCIILocalPartDefect is not a true defect. Especially when you consider that the comments say it can only happen when parsing unicode, not when parsing bytes. So instead of doing this, now that we're doing the serialization check (which is where it becomes a true defect) we should just remove the check for this defect in _header_value_parser.
We can't remove it from errors.py for backward compatibility reasons, but it should be marked as deprecated.
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 |
Uh oh!
There was an error while loading.Please reload this page.
The email.headerregistry.Address constructor raised an error if addr_spec contained a non-ASCII character. (But it fully supports non-ASCII in the separate username and domain args.) This change removes the error for a non-ASCII addr_spec.
This PR implements@bitdancer's suggested fix from#81074 (comment):
(The other bugs discussed in that comment are reported separately as#83938 and#122476.)
Fixesgh-81074