Skip to content

Conversation

@n0npax
Copy link
Contributor

@n0npax n0npax commented Jul 2, 2019

Copy link
Member

@tirkarthi tirkarthi left a comment

Choose a reason for hiding this comment

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

Please move the tests to Lib/test/test_email/test_parser.py and remove the Lib/email/test/__init__.py file.

Edit : There is also Lib/test/test_email/test__header_value_parser.py with similar tests that might be a better location.

@n0npax n0npax changed the title bpo-37461: Fix email.parser.Parse hang [WIP] bpo-37461: Fix email.parser.Parse hang Jul 2, 2019
@n0npax
Copy link
Contributor Author

n0npax commented Jul 2, 2019

I've got 2 broken tests and I would appreciate some guidance.
Is changing function _get_ptext_to_endchars to return _get_ptext_to_endchars('"', '"') -> '"', "", False bad approach?

@tirkarthi
Copy link
Member

Test case failures usually mean there is a regression where some behavior that was guaranteed previously is now being changed and there should be a good reason to change the tests. Sorry, email is not my expertise and maybe @maxking could help.

@guidovranken
Copy link

This is a slightly different PoC, and this still hangs after applying the PR

from email.parser import BytesParser, Parser
from email.policy import default
payload = "\x43\x6f\x6e\x74\x65\x6e\x74\x2d\x54\x79\x70\x65\x3a\x78\x3b\x61\x72\x1b\x2a\x3d\x22\x73\x4f\x27\x23\x61\xff\xff\x27\x5c\x22\x3d\x72\x2d\x54\x79\x8e\xc4\x9e\x8d\xe4\xd5\xc2\x83\x7c\x7c\x7c"
Parser(policy=default).parsestr(payload)

@maxking
Copy link
Contributor

maxking commented Jul 2, 2019

Thanks @n0npax for this patch!

+1 on what @guidovranken says, I was going to comment but you beat me to it.

I probably won't be able help too much for next 15 days, since I am travelling, but this PR doesn't fix the real problem. I was able to reproduce the problem with:

from email.parser import BytesParser, Parser
from email.policy import default


payload = 'Content-Type:x;\x1b*="\'G\'\\"g'
msg = Parser(policy=default).parsestr(payload)

Note the last "g in the end of the string, which caused the fix of value == '"' to fail. I am not sure about where the end " of the quoted-printable is supposed to be parsed.

Please make sure we don't delete any tests without proper reasoning about why we need to delete them :)

@n0npax
Copy link
Contributor Author

n0npax commented Jul 4, 2019

I've got no idea how to solve this in a proper way. Closing PR.

@n0npax n0npax closed this Jul 4, 2019
@n0npax
Copy link
Contributor Author

n0npax commented Jul 4, 2019

Sorry for fuss guys. Thanks for suggestions.

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.

6 participants