Infinite loop with short maximum line lengths in EmailPolicy

[pganssle]

BPO	36564
Nosy	@warsaw, @msapiro, @ned-deily, @bitdancer, @maxking, @pganssle, @miss-islington, @tirkarthi
PRs	python/cpython#12732 python/cpython#14797 python/cpython#14798 python/cpython#14799

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields: ```python assignee = None closed_at = created_at = labels = ['3.7', '3.8', '3.9'] title = 'Infinite loop with short maximum line lengths in EmailPolicy' updated_at = user = 'https://github.com/pganssle' ``` bugs.python.org fields: ```python activity = actor = 'ned.deily' assignee = 'none' closed = True closed_date = closer = 'ned.deily' components = [] creation = creator = 'p-ganssle' dependencies = [] files = [] hgrepos = [] issue_num = 36564 keywords = ['patch', 'security_issue'] message_count = 12.0 messages = ['339660', '342686', '342715', '342716', '342717', '342721', '342722', '342729', '348036', '348037', '348038', '348244'] nosy_count = 8.0 nosy_names = ['barry', 'msapiro', 'ned.deily', 'r.david.murray', 'maxking', 'p-ganssle', 'miss-islington', 'xtreak'] pr_nums = ['12732', '14797', '14798', '14799'] priority = 'normal' resolution = 'fixed' stage = 'resolved' status = 'closed' superseder = None type = None url = 'https://bugs.python.org/issue36564' versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9'] ```

[pganssle]

When reviewing PR 12020 fixing an infinite loop in the e-mail module, I noticed that a *different* infinite loop is documented with a "# XXX" comment on line 2724:

https://github.com/python/cpython/blob/58721a903074d28151d008d8990c98fc31d1e798/Lib/email/_header_value_parser.py#L2724

This is triggered when the policy's max_line_length is set to be shorter than minimum line length required by the "RFC 2047 chrome". It can be reproduced with:

    from email.policy import default

    policy = default.clone(max_line_length=7) # max_line_length = 78
    policy.fold("Subject", "12345678")

I could not find an entry on the tracker for this bug, but it is documented in the source code itself, so maybe I just didn't try hard enough.

Related but distinct bugs: bpo-33529, bpo-33524

I will submit a patch to fix this.

[maxking]

Moving the conversation here from https://github.com/python/cpython/pull/12732 for David.

I previously suggested HeaderParseError because of the fact that we could fail to parse a value into a Header Object in the above scenario.

@r.david.murray Would it be more appropriate to have something like a "PolicyError" in case where the policy's max_line_length isn't long enough to fit the shortest encoded word?

[bitdancer]

Can you demonstrate the parsing error? maxlen should have no effect during parsing.

[bitdancer]

As for the other, I don't see the need for a custom error. It's a ValueError in my view. I wouldn't object to it strongly, but note that this error is content dependent. If there's nothing to encode, you can "get away with" a shorter maxlen. Though why you would want to is beyond me, and that's another reason I don't think this warrants a custom error class.

[pganssle]

Responding to a comment on the PR:

Now, that said you might want to consider the fact that in _fold_mime_parameters I deal with this issue by bumping maxlen to 78 rather than raising an error. I'm not sure that was the right choice, but whatever we do, it should probably be made consistent between the two cases.

So I think in an ideal world this would be consistent between the two, but I *also* think that the right solution is to raise an exception rather than silently coercing, and changing this in _fold_mime_parameters would be a backwards-incompatible change.

I think that if you agree that an exception is better, maybe the path forward is to raise an exception in this case and switch the _fold_mime_parameters case over to raising a warning, which will be turned into an exception in a later release (3.10 maybe).

It is so sad that I never came back to fix that XXX comment I left myself :(

I'm glad that the XXX comment was at least there! Otherwise I wouldn't have notice that there was anything to fix!

[bitdancer]

Good point about the backward compatibility. Yes I agree, I think raising the error is probably better. A deprecation warning seems like a good path forward...I will be very surprised if anyone encounters it, though :)

[maxking]

I was wrong about the parsing error, it looks like length from the policy isn't used when parsing.

>>> from email.policy import default
>>> from email import message_from_string
>>> p = default.clone(max_line_length=10)
>>> msg = message_from_string("""\
... From: Hello@example.com
... To: Hello@example.com
... Subject: WelcomeToThisLongSubject
... 
... Thanks""", policy=p)
>>> msg
<email.message.EmailMessage object at 0x7f448f70d860>
>>> msg['Subject']
'WelcomeToThisLongSubject'

This works just fine. Thanks David. +1 for ValueError then.

[bitdancer]

Right, one of the fundamental principles of the email library is that when parsing input we do not ever raise an error. We may note defects, but whatever we get we *must parse and turn in to *something.

[warsaw]

New changeset f69d5c61981ea97d251db515c7ff280fcc17182d by Barry Warsaw (Paul Ganssle) in branch 'master': Fix infinite loop in email folding logic (GH-12732) https://github.com/python/cpython/commit/f69d5c61981ea97d251db515c7ff280fcc17182d

[miss-islington]

New changeset 6a2aec0ff587032beb8aac8cbebb09e7a52f6694 by Miss Islington (bot) in branch '3.8': Fix infinite loop in email folding logic (GH-12732) https://github.com/python/cpython/commit/6a2aec0ff587032beb8aac8cbebb09e7a52f6694

[miss-islington]

New changeset e7bec26937ce602ca21cc1f78a391adcf5eafdf1 by Miss Islington (bot) in branch '3.7': Fix infinite loop in email folding logic (GH-12732) https://github.com/python/cpython/commit/e7bec26937ce602ca21cc1f78a391adcf5eafdf1

[ned-deily]

New changeset 79a47e2b9cff6c9facdbc022a752177ab89dc533 by Ned Deily (Miss Islington (bot)) in branch '3.6': Fix infinite loop in email folding logic (GH-12732) (GH-14799) https://github.com/python/cpython/commit/79a47e2b9cff6c9facdbc022a752177ab89dc533