X Tutup
The Wayback Machine - https://web.archive.org/web/20260401064511/https://github.com/python/cpython/pull/142
Skip to content

bpo-1025395: Fix email.utils.parseaddr to handle multiple hops#142

Closed
ioanatia wants to merge 1 commit intopython:masterfrom
ioanatia:fix-email-utils-parseaddr
Closed

bpo-1025395: Fix email.utils.parseaddr to handle multiple hops#142
ioanatia wants to merge 1 commit intopython:masterfrom
ioanatia:fix-email-utils-parseaddr

Conversation

@ioanatia
Copy link
Copy Markdown

@ioanatia ioanatia commented Feb 17, 2017

This fixes a very old issue with email.utils.parseaddr.
Right now email.utils.parseaddr will fail to parse the input correctly when the address contains a route with multiple hops. However it does not have an issue with single hop routes.

RFC5322 states that including route hops is obsolete and that the route part should be ignored (Section 4.4). It also states that this syntax is valid and it must be accepted and parsed (Section 4).

https://bugs.python.org/issue1025395

@the-knights-who-say-ni
Copy link
Copy Markdown

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA. This is necessary for legal reasons before we can look at your contribution. Please follow these steps to help rectify the issue:

  1. If you don't have an account on b.p.o, please create one
  2. Make sure your GitHub username is listed in "Your Details" at b.p.o
  3. If you have not already done so, please sign the PSF contributor agreement
  4. If you just signed the CLA, please wait at least one US business day and then check "Your Details" on bugs.python.org to see if your account has been marked as having signed the CLA (the delay is due to a person having to manually check your signed CLA)
  5. Reply here saying you have completed the above steps

Thanks again to your contribution and we look forward to looking at it!

@ioanatia ioanatia force-pushed the fix-email-utils-parseaddr branch from 350ccc1 to 9804620 Compare October 27, 2017 15:50
@ioanatia ioanatia requested a review from a team as a code owner October 27, 2017 15:50
@brettcannon brettcannon reopened this Oct 27, 2017
akruis pushed a commit to akruis/cpython that referenced this pull request Nov 7, 2017
Update the URLs for the documentation (http://stackless.readthedocs.io)
and source code browsing (https://github.com/stackless-dev/stackless).
(cherry picked from commit b4a5725)
@ioanatia
Copy link
Copy Markdown
Author

ioanatia commented Nov 7, 2017

Hey folks! I was wondering why my PR keeps failing, even though I did a rebase a couple of days ago! Can someone elucidate what might be going on here, please? From what I'm noticing, the Travis job has a different config than other jobs, so I'm kinda confused about why is that. Thanks a lot!

@brettcannon
Copy link
Copy Markdown
Member

It's totally fine to just do a merge if that's easier since we will do a squash commit in the end. I've restarted the Travis job just in case it was a weird fluke, though.

@ioanatia ioanatia force-pushed the fix-email-utils-parseaddr branch from 9804620 to 6d17d16 Compare December 8, 2017 14:32
@ioanatia
Copy link
Copy Markdown
Author

ioanatia commented Dec 8, 2017

Rebasing the branch on latest master does not fix it.
It seems to be that the travis build is not even run my latest commit, but on a random one de1d4fc .
On the other hand, the appveyor build seems fine.

@brettcannon
Copy link
Copy Markdown
Member

Maybe try doing a merge instead of a rebase? We've had issues in the past of Travis only grabbing the last 50 commits so that can lead to issues with older PRs.

akruis pushed a commit to akruis/cpython that referenced this pull request Mar 25, 2018
@Mariatta
Copy link
Copy Markdown
Member

Thanks for the PR, @ioanatia.

  • Travis build failed:
/home/travis/build/python/cpython/Doc/library/pyexpat.rst:872:Footnote [#] is not referenced.

The change seems unrelated to your change, but unless that got fixed we can't accept the PR.

I realize this PR is quite old, so it's possible the fix for the failed travis build has already been done.
Will it be possible to rebase it off the latest master branch? If it's easier feel free to open a new PR.

  • Please add yourself to Misc/ACKs file (there's a brief info about that here

  • Please add a news entry describing this change. It can be added using blurb add.

The instruction for blurb add is here
There are additional details about how to write the news entry here
blurb is available in python >= 3.5

Copy link
Copy Markdown
Member

@Mariatta Mariatta left a comment

Choose a reason for hiding this comment

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

Please see my review: #142 (comment)

@bedevere-bot
Copy link
Copy Markdown

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.

@ioanatia
Copy link
Copy Markdown
Author

thanks @Mariatta !
I rebased this branch on latest master multiple times, but somehow the Travis build seems to pick up the same unrelated commit de1d4fc .

Not sure what else I can do than to close this one and reopen it in #6917.

@ioanatia ioanatia closed this May 16, 2018
akruis pushed a commit to akruis/cpython that referenced this pull request Jun 19, 2018
Update the URLs for the documentation (http://stackless.readthedocs.io)
and source code browsing (https://github.com/stackless-dev/stackless).
(cherry picked from commit b4a5725)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

X Tutup