Skip to content

gh-75171: Fix parsing invalid email address headers starting or ending with a dot #15600

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Apr 17, 2024

Conversation

tsufeki
Copy link
Contributor

@tsufeki tsufeki commented Aug 29, 2019

This PR is based on #2811, with additional fixes for the case where display-name starts with a dot.

Added checks whether or not given token is a list before inserting/removing cfws from it.

https://bugs.python.org/issue30988

@tsufeki tsufeki requested a review from a team as a code owner August 29, 2019 18:40
@the-knights-who-say-ni
Copy link

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).

Our records indicate we have not received your CLA. For legal reasons we need you to sign this before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

@bedevere-bot
Copy link

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.

@tsufeki
Copy link
Contributor Author

tsufeki commented Sep 1, 2019

I have made the requested changes; please review again

@bedevere-bot
Copy link

Thanks for making the requested changes!

@maxking: please review the changes made to this pull request.

Co-Authored-By: Abhilash Raj <maxking@users.noreply.github.com>
@malvidin
Copy link

malvidin commented Mar 4, 2020

@maxking and @csabella, is there any additional review that is needed for this pull request from @tsufeki?
We would appreciate it if we could remove the workaround in the eml_parser for bpo-30988.
https://github.com/GOVCERT-LU/eml_parser/blob/f98980a77d9c7d914d97525a62294075c0ce42d9/tests/test_emlparser.py#L131

@spacefreak86
Copy link

Hi @maxking

Could you please review this PL and approve it? This issue currently affects my productive systems and it would be great if it could be fixed upstream.

FYI: python/email-team was requested to review this PL, but it does not seem to exist anymore. Maybe that's the reason, why this PL was not approved yet?

@spacefreak86
Copy link

Hi @tsufeki
Could you please change the the two lines of the PR according to PEP 8?
It should be a quick fix and after that the PR will may get merged.

@tsufeki
Copy link
Contributor Author

tsufeki commented Mar 10, 2021

@ZackerySpytz @spacefreak86 I fixed those long lines.

@malvidin
Copy link

Is there anyone from the python/email-team to review this?
@ZackerySpytz, @maxking, and @csabella, do you know who can review this?

@spacefreak86
Copy link

@maxking could please review the changed PR again? It would be really great to have this fixed upstream.

@BoboTiG
Copy link
Contributor

BoboTiG commented Apr 4, 2023

Gentle ping for this one since #2811 was closed :)

@arhadthedev arhadthedev changed the title bpo-30988: Fix parsing invalid email address headers starting or ending with a dot gh-75171: Fix parsing invalid email address headers starting or ending with a dot Apr 5, 2023
@nielsbox
Copy link

Any updates on this? @maxking?

@arhadthedev
Copy link
Member

@warsaw (as a email module expert)

@emerrf
Copy link

emerrf commented Sep 12, 2023

Hi @warsaw, @bitdancer, @maxking is anything missing in order to have the PR merge into main? (remove the label "awaiting change review").
Why is this PR important? We identified phishing emails with dots in the Display Name that cannot be parsed

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

There was also the same bug in DisplayName.value, which I have fixed.

@miss-islington-app
Copy link

Thanks @tsufeki for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Apr 17, 2024
… ending with a dot (pythonGH-15600)

(cherry picked from commit 8cc9adb)

Co-authored-by: tsufeki <tsufeki@ymail.com>
Co-authored-by: Tim Bell <timothybell@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@bedevere-app
Copy link

bedevere-app bot commented Apr 17, 2024

GH-117964 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Apr 17, 2024
serhiy-storchaka added a commit that referenced this pull request Apr 17, 2024
…r ending with a dot (GH-15600) (GH-117964)

(cherry picked from commit 8cc9adb)

Co-authored-by: tsufeki <tsufeki@ymail.com>
Co-authored-by: Tim Bell <timothybell@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
… ending with a dot (pythonGH-15600)

Co-authored-by: Tim Bell <timothybell@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
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.