-
Notifications
You must be signed in to change notification settings - Fork 563
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
olevba: prevent side effects on python lib "email" #604
Conversation
…patching when needed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thanks. Just a question: why do you use copy() instead of just storing the original email.feedparser.headerRE?
Copy isn't needed, it actually raise an exception as you can't copy a pattern object, but it was masking another exception from email.message_from_bytes() in python3. With copy the exception was soon enough to not patch the regex (and thus no problem to use email later), in the second case the regex was patched, the exception raised and not patched back (and using email lib failed). Using a try.. finally clause seems more reasonable, to keep the old regex in case of exception. Now... for the reason of the exception in python3. It seems that the fix for #32 only works on python2.7 as python3 added an additional assert in feedparser.py at line 524. This same exception is actually the reason why I want the regex to be patched back to what is originally was. Using the version 0.55.1 from pypi, without my fix, on python3 on the sample from issue #32 give me this error:
It's basically the same trace I get from issue #602 So at the moment this PR does exactly what is needed to not have side effects on the email lib, but it still doesn't work on the sample from issue #32 |
Indeed, I confirm that the current dev version (without this PR) manages to parse the sample from issue #32 correctly on Python 2.7, but fails on Python 3. I quickly checked other MHT samples on Python 3, it seems to work, so it's only an issue with the sample from #32 and the monkeypatch. |
Fix issue #602
(Sorry for previous pull request, I committed with the wrong email, tried to fix it and ended up messing up the whole repository. This PR should be alright.)