bpo-44660: Updated email.feedparser with support for message/global emails with quoted-printable and base64 Content-Transfer-Encodings. #27208
Conversation
Added message/global Content-Transfer-Encoding section to _parsegen function, three supporting classes, and a supporting function.
Added parsing coverage for base64 and quoted-printable Content-Transfer-Encodings.
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
|
||
|
|
||
| class NonIdentityTransferDecodingFeedParser(abc.ABC): | ||
| """ |
There was a problem hiding this comment.
hanging indent on the docstring looks strange to me; consult PEP 257
Lib/email/feedparser.py
Outdated
|
|
||
| class Base64TransferDecodingFeedParser(NonIdentityTransferDecodingFeedParser): | ||
| """ | ||
| Concrete, "base64" implementation of TransferDecodingFeedParser. |
There was a problem hiding this comment.
This docstring, and the docstring for QuotedPrintableTransferDecodingFeedParser seem redundant. Seeing as the class and base class names are quite descriptive, maybe just provide an RFC (or other) citation in the docstrings?
There was a problem hiding this comment.
Thanks, I incorporated the recommendation in a massive update I'm about to apply.
Lib/email/feedparser.py
Outdated
| Creates a new FeedParser object that transparently reverses the specified | ||
| content_transfer_encoding transformation. | ||
| :param content_transfer_encoding: str | ||
| :return: Union[NoneType,NonIdentityTransferDecodingFeedParser] None if |
There was a problem hiding this comment.
If you're going to provide this information, use type hints:
def _new_transfer_decoding_parser(content_transfer_encoding: str, **kwargs
) -> Union[NoneType, NonIdentityTransferDecodingFeedParser]: Formatting is tricky because the bass class name is so long. Maybe consider shortening it a bit: NoIdTransDecodingFP. I don't really love that abbreviation myself and I like the current descriptive name, but it is quite a mouthful.
There was a problem hiding this comment.
Thanks, I incorporated the recommendation in a massive update I'm about to apply.
Replaced decoding-parser factory method with Content-Transfer-Encoding (str) to factory (function) map. Removed the word "transfer" from the TransferDecoding-related classes, functions, variables, etc. to be more concise and because there is already sufficient context. Removed the word "nonidentity" from class NonIdentityTransferDecodingFeedParser's name and updated the docstrings/comments explaining why the word is redundant. Moved base-64-only functionality to new class in base64mime sub-package. Accounted for edge cases in QuotedPrintableFeedParser so that it could be made a standalone feature if desired. Added/updated code comments.
Issue 44660 code review changes 1
|
This PR is stale because it has been open for 30 days with no activity. |
|
The following commit authors need to sign the Contributor License Agreement: |
Fixed issue 44660: email.feedparser Module Lacks Support for Section 3.5 of RFC 6532: message/global Emails with non-identity Content-Transfer-Encodings as follows:
Added message/global Content-Transfer-Encoding section to _parsegen function; three supporting classes; and a supporting function.
Added unit test coverage for the subject scenarios.
https://bugs.python.org/issue44660