[2.7] bpo-34155: Dont parse domains containing @ (GH-13079) by rcsanchez97 · Pull Request #16006 · python/cpython · GitHub
Skip to content

[2.7] bpo-34155: Dont parse domains containing @ (GH-13079) - #16006

Merged
miss-islington merged 1 commit into
python:2.7from
rcsanchez97:2.7_bpo-34155
Sep 14, 2019
Merged

[2.7] bpo-34155: Dont parse domains containing @ (GH-13079)#16006
miss-islington merged 1 commit into
python:2.7from
rcsanchez97:2.7_bpo-34155

Conversation

@rcsanchez97

@rcsanchez97 rcsanchez97 commented Sep 11, 2019

Copy link
Copy Markdown

This change skips parsing of email addresses where domains include a "@" character, which can be maliciously used since the local part is returned as a complete address.

(cherry picked from commit 8cb65d1)

Excludes changes to Lib/email/_header_value_parser.py, which did not
exist in 2.7.

Co-authored-by: jpic jpic@users.noreply.github.com

https://bugs.python.org/issue34155

Automerge-Triggered-By: @maxking

https://bugs.python.org/issue34155
(cherry picked from commit 8cb65d1)

Excludes changes to Lib/email/_header_value_parser.py, which did not
exist in 2.7.

Co-authored-by: jpic <jpic@users.noreply.github.com>
@the-knights-who-say-ni

Copy link
Copy Markdown

@vstinner

Copy link
Copy Markdown
Member

@rcsanchez97: Can you please sign the CLA? See the previous comment for the procedure.

@rcsanchez97

Copy link
Copy Markdown
Author

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. But I would prefer to have a double check from another core dev.

@maxking or @warsaw: Can you please review this PR?

I compared with 8cb65d1 : the code looks the same. The 2.7 backport lacks the get_domain() change which raises HeaderParseError: Python 2.7 doens't have the parser.get_addr_spec() function, so I understand that it doesn't need this change.

@maxking maxking left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

Thank @rcsanchez97 for the backport and @vstinner for helping review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type-security A security issue

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants