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-29412: Fix indexError when parsing a header value ending unexpectedly #14387

Merged
merged 4 commits into from
Jun 26, 2019

Conversation

maxking
Copy link
Contributor

@maxking maxking commented Jun 26, 2019

When parsing a word, if the value is None, we should raise HeaderParseError instead of continuing silently. This helps fix the IndexError.

Part of the patch is from GH-6907, so thanks to @TyrannosourceExe.

https://bugs.python.org/issue29412

…er_value_parser.py and created tests in test__header_value_parser.py for CFWS.
@maxking
Copy link
Contributor Author

maxking commented Jun 26, 2019

To mention this borrows a test case from GH-6907 (which seeems to be abandoned, which is why I am proposing this new PR to override that).

The fix however is different than GH-6907, which is why I haven't added a Co-Authored-By: in the PR message since I don't know if the original author agrees with my implementation.

@maxking maxking force-pushed the gh-6907-2 branch 2 times, most recently from 2941cb3 to e28cbc9 Compare June 26, 2019 05:28
Copy link
Contributor

@mangrisano mangrisano left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you for providing the test. It is always useful. :)

@maxking maxking changed the title bpo-29412: Fix indexError when parsing a header value with only comments bpo-29412: Fix indexError when parsing a header value ending unexpectedly Jun 26, 2019
Lib/email/_header_value_parser.py Outdated Show resolved Hide resolved
@@ -1360,6 +1360,9 @@ def get_word(value):
leader, value = get_cfws(value)
else:
leader = None
if not value:
Copy link
Member

Choose a reason for hiding this comment

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

Do you want to catch all false-y values or just None?

Copy link
Contributor Author

@maxking maxking Jun 26, 2019

Choose a reason for hiding this comment

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

I want to catch None and empty string '' values, both of which could cause IndexError when running value[0]. It could also be caused by empty list, empty tuple etc etc, but I don't think value can be of those datatypes.

Remove implicit concatenation of strings and move the string
 to next line so that it can fit properly under 79 chars.
@warsaw
Copy link
Member

warsaw commented Jun 26, 2019

I'm assuming this needs backporting to 3.7 and 3.8.

@warsaw warsaw merged commit 7213df7 into python:master Jun 26, 2019
@miss-islington
Copy link
Contributor

Thanks @maxking for the PR, and @warsaw for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 26, 2019
…edly (pythonGH-14387)

* patched string index out of range error in get_word function of _header_value_parser.py and created tests in test__header_value_parser.py for CFWS.
* Raise HeaderParseError instead of continuing when parsing a word.
(cherry picked from commit 7213df7)

Co-authored-by: Abhilash Raj <[email protected]>
@bedevere-bot
Copy link

GH-14411 is a backport of this pull request to the 3.8 branch.

@bedevere-bot
Copy link

GH-14412 is a backport of this pull request to the 3.7 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 26, 2019
…edly (pythonGH-14387)

* patched string index out of range error in get_word function of _header_value_parser.py and created tests in test__header_value_parser.py for CFWS.
* Raise HeaderParseError instead of continuing when parsing a word.
(cherry picked from commit 7213df7)

Co-authored-by: Abhilash Raj <[email protected]>
warsaw pushed a commit that referenced this pull request Jun 26, 2019
…edly (GH-14387) (GH-14412)

* patched string index out of range error in get_word function of _header_value_parser.py and created tests in test__header_value_parser.py for CFWS.
* Raise HeaderParseError instead of continuing when parsing a word.
(cherry picked from commit 7213df7)

Co-authored-by: Abhilash Raj <[email protected]>
warsaw pushed a commit that referenced this pull request Jun 26, 2019
…edly (GH-14387) (GH-14411)

* patched string index out of range error in get_word function of _header_value_parser.py and created tests in test__header_value_parser.py for CFWS.
* Raise HeaderParseError instead of continuing when parsing a word.
(cherry picked from commit 7213df7)

Co-authored-by: Abhilash Raj <[email protected]>
lisroach pushed a commit to lisroach/cpython that referenced this pull request Sep 10, 2019
…edly (pythonGH-14387)

* patched string index out of range error in get_word function of _header_value_parser.py and created tests in test__header_value_parser.py for CFWS.
* Raise HeaderParseError instead of continuing when parsing a word.
DinoV pushed a commit to DinoV/cpython that referenced this pull request Jan 14, 2020
…edly (pythonGH-14387)

* patched string index out of range error in get_word function of _header_value_parser.py and created tests in test__header_value_parser.py for CFWS.
* Raise HeaderParseError instead of continuing when parsing a word.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants