classification
Title: Infinite loop with short maximum line lengths in EmailPolicy
Type: Stage: resolved
Components: Versions: Python 3.9, Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: barry, maxking, miss-islington, msapiro, ned.deily, p-ganssle, r.david.murray, xtreak
Priority: normal Keywords: patch, security_issue

Created on 2019-04-08 16:45 by p-ganssle, last changed 2019-07-21 14:03 by ned.deily. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12732 merged p-ganssle, 2019-04-08 16:55
PR 14797 merged miss-islington, 2019-07-16 17:50
PR 14798 merged miss-islington, 2019-07-16 17:50
PR 14799 merged miss-islington, 2019-07-16 17:50
Messages (12)
msg339660 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2019-04-08 16:45
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: #33529, #33524

I will submit a patch to fix this.
msg342686 - (view) Author: Abhilash Raj (maxking) * (Python committer) Date: 2019-05-17 02:32
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?
msg342715 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2019-05-17 13:06
Can you demonstrate the parsing error?  maxlen should have no effect during parsing.
msg342716 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2019-05-17 13:08
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.
msg342717 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2019-05-17 13:11
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!
msg342721 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2019-05-17 14:03
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 :)
msg342722 - (view) Author: Abhilash Raj (maxking) * (Python committer) Date: 2019-05-17 14:15
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.
msg342729 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2019-05-17 15:25
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*.
msg348036 - (view) Author: Barry A. Warsaw (barry) * (Python committer) Date: 2019-07-16 17:50
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
msg348037 - (view) Author: miss-islington (miss-islington) Date: 2019-07-16 18:08
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
msg348038 - (view) Author: miss-islington (miss-islington) Date: 2019-07-16 18:15
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
msg348244 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2019-07-21 14:01
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
History
Date User Action Args
2019-07-21 14:03:58ned.deilysetkeywords: + security_issue
status: open -> closed
stage: patch review -> resolved
resolution: fixed
versions: + Python 3.6, Python 3.9
2019-07-21 14:01:47ned.deilysetnosy: + ned.deily
messages: + msg348244
2019-07-16 18:15:46miss-islingtonsetmessages: + msg348038
2019-07-16 18:08:42miss-islingtonsetnosy: + miss-islington
messages: + msg348037
2019-07-16 17:50:33miss-islingtonsetpull_requests: + pull_request14595
2019-07-16 17:50:24barrysetmessages: + msg348036
2019-07-16 17:50:23miss-islingtonsetpull_requests: + pull_request14594
2019-07-16 17:50:16miss-islingtonsetpull_requests: + pull_request14593
2019-05-17 15:25:42r.david.murraysetmessages: + msg342729
2019-05-17 14:15:27maxkingsetmessages: + msg342722
2019-05-17 14:03:42r.david.murraysetmessages: + msg342721
2019-05-17 13:11:37p-gansslesetmessages: + msg342717
2019-05-17 13:08:35r.david.murraysetmessages: + msg342716
2019-05-17 13:06:12r.david.murraysetmessages: + msg342715
2019-05-17 02:32:36maxkingsetmessages: + msg342686
2019-05-16 02:08:13p-gansslesetnosy: + msapiro, maxking
2019-04-08 17:10:48xtreaksetnosy: + xtreak
2019-04-08 16:55:49p-gansslesetkeywords: + patch
stage: patch review
pull_requests: + pull_request12655
2019-04-08 16:45:34p-gansslecreate