Skip to content
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

bpo-44637: Fix DBQuote mail header refold #29881

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Julien00859
Copy link

@Julien00859 Julien00859 commented Dec 1, 2021

When a header content is too long, the RFC demands to fold it over
multiple lines. Each line starting with a space to denote folded-lines
from regular ones.

Folding a line requires splitint it, there are only a few sweet spots
where it is possible to do so (e.g. between two words). Words are pretty
deep in the parse-tree thus multiple parts must be unwrap to reveal
them. One of those parts can be a quoted-string, printing a
quoted-string as a whole correctly wraps its content with double-quotes
but printing every child never quotes them.

When a quoted-string must be unwrap to find a sweet-splot to split the
line, we now inject double-quotes literals before and after its
children.

https://bugs.python.org/issue44637

When a header content is too long, the RFC demands to fold it over
multiple lines. Each line starting with a space to denote folded-lines
from regular ones.

Folding a line requires splitint it, there are only a few sweet spots
where it is possible to do so (e.g. between two words). Words are pretty
deep in the parse-tree thus multiple parts must be unwrap to reveal
them. One of those parts can be a quoted-string, printing a
quoted-string as a whole correctly wraps its content with double-quotes
but printing every child never quotes them.

When a quoted-string must be unwrap to find a sweet-splot to split the
line, we now inject double-quotes literals before and after its
children.
@Julien00859
Copy link
Author

The email refolding algorithm was wrongly removing double-quotes, we now make sure they are correctly inserted when a BareQuotedString part is unwrap. I tried several other approaches in order not to bloat the _refold_parse_tree more but I couldn't make them to work.

@Julien00859
Copy link
Author

Not sure it deserves a message in the news, you tell me.

@Julien00859
Copy link
Author

I tested the changes by sending myself an email with a forged message. Server is postfix, client is Thunderbird.

image
issue44637-eml.txt
issue44637-py.txt

@github-actions
Copy link

github-actions bot commented Jan 2, 2022

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 2, 2022
Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Choose a reason for hiding this comment

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

All email test ok apart from
test_utf8_input_no_charset
Looks ok to me.

@arhadthedev
Copy link
Member

@warsaw could you review (and merge if possible) this PR? The fixed problem has a real-world impact (#88803 (comment)):

Because the proposed PR have been staled for over three months and that we continue to get angry emails from our customers complaining about this issue [...]

@AlexWaygood
Copy link
Member

I'm not familiar with the email module, but if this is a bugfix, a short NEWS entry is probably warranted. You can add one using the web app here: https://blurb-it.herokuapp.com

Copy link

@gazfaris gazfaris left a comment

Choose a reason for hiding this comment

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

Compiles, without errors

@Julien00859
Copy link
Author

up, it has been over a year, the issue is still affecting us, it has a clear commit message, tests and all checks are successful

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants