From a8f5bf8bf86c58ee3196bea61b865fb2a7273c15 Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sat, 17 Jul 2021 00:41:17 -0400 Subject: [PATCH 1/7] Initial version of fix. Added message/global Content-Transfer-Encoding section to _parsegen function, three supporting classes, and a supporting function. --- Lib/email/feedparser.py | 107 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 107 insertions(+) diff --git a/Lib/email/feedparser.py b/Lib/email/feedparser.py index 97d3f5144d606f..788275e1abac41 100644 --- a/Lib/email/feedparser.py +++ b/Lib/email/feedparser.py @@ -21,6 +21,9 @@ __all__ = ['FeedParser', 'BytesFeedParser'] +import abc +import base64 +import quopri import re from email import errors @@ -292,6 +295,28 @@ def _parsegen(self): # Not at EOF so this is a line we're going to need. self._input.unreadline(line) return + if self._cur.get_content_type() == 'message/global': + # Support for message/global parts that have non-identity + # content-transfer-encodings as outlined in RFC 6532 + # (s. 1, p. 3; s 3.5; "Encoding considerations," s. 3.7). + transfer_decoding_parser = _new_transfer_decoding_parser( + self._cur['Content-Transfer-Encoding'], + policy=self.policy, + _factory=self._factory + ) + if isinstance(transfer_decoding_parser, + NonIdentityTransferDecodingFeedParser): + for line in self._input: # Consume all of the content. + if line is NeedMoreData: + yield NeedMoreData + continue + if line == '': + break + transfer_decoding_parser.feed(line) + # Retrieve and attach subpart: + subpart = transfer_decoding_parser.close() + self._cur.attach(subpart) + return if self._cur.get_content_maintype() == 'message': # The message claims to be a message/* type, then what follows is # another RFC 2822 message. @@ -534,3 +559,85 @@ class BytesFeedParser(FeedParser): def feed(self, data): super().feed(data.decode('ascii', 'surrogateescape')) + + +class NonIdentityTransferDecodingFeedParser(abc.ABC): + """ + This is an abstract base class; only its subclasses should be instantiated + directly. + + Instances of this class work like FeedParser except that the feed() method, + prior to parsing the input, transparently decodes the input consistent with + RFC 2045, s. 6.2. Each concrete subclass reverses one of the non-identity + Content-Transfer-Encoding transformations described there. + """ + + def __init__(self, *args, **kwargs): + self._bytes_feed_parser = BytesFeedParser(*args, **kwargs) + + @abc.abstractmethod + def feed(self, text): + pass + + def close(self): + return self._bytes_feed_parser.close() + + +class Base64TransferDecodingFeedParser(NonIdentityTransferDecodingFeedParser): + """ + Concrete, "base64" implementation of TransferDecodingFeedParser. + """ + + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self.encoded_buffer = bytearray() + + def feed(self, line): + self.encoded_buffer.extend(line.rstrip().encode('ascii')) + total_decodable = int(len(self.encoded_buffer) / 4) * 4 + if total_decodable >= 1: + decodable_bytes = self.encoded_buffer[:total_decodable] + self.encoded_buffer = self.encoded_buffer[total_decodable:] + decoded_bytes = base64.standard_b64decode(decodable_bytes) + self._bytes_feed_parser.feed(decoded_bytes) + + def close(self): + if len(self.encoded_buffer) >= 1: + decoded_bytes = base64.standard_b64decode(self.encoded_buffer) + self._bytes_feed_parser.feed(decoded_bytes) + self.encoded_buffer.clear() + return self._bytes_feed_parser.close() + + +class QuotedPrintableTransferDecodingFeedParser( + NonIdentityTransferDecodingFeedParser +): + """ + Concrete, "quoted-printable" implementation of TransferDecodingFeedParser. + """ + + def feed(self, line): + if line == '': + return + if line[-1] != '\n': + line += '\r\n' + encoded_bytes = line.encode('ascii') + decoded_bytes = quopri.decodestring(encoded_bytes) + self._bytes_feed_parser.feed(decoded_bytes) + + +def _new_transfer_decoding_parser(content_transfer_encoding, **kwargs): + """ + 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 + content_transfer_encoding specifies an identity transformation (i.e. no + decoding is required), is None or invalid; + NonIdentityTransferDecodingFeedParser, otherwise. + """ + + if content_transfer_encoding == 'quoted-printable': + return QuotedPrintableTransferDecodingFeedParser(**kwargs) + if content_transfer_encoding == 'base64': + return Base64TransferDecodingFeedParser(**kwargs) From c91bde45f33ccdf08bd88657a93b4f51dd18768d Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sat, 17 Jul 2021 03:21:09 -0400 Subject: [PATCH 2/7] Added parsing unit test coverage. Added parsing coverage for base64 and quoted-printable Content-Transfer-Encodings. --- Lib/test/test_email/test_email.py | 166 ++++++++++++++++++++++++++++++ 1 file changed, 166 insertions(+) diff --git a/Lib/test/test_email/test_email.py b/Lib/test/test_email/test_email.py index eed60142b19e3b..832a6806400c61 100644 --- a/Lib/test/test_email/test_email.py +++ b/Lib/test/test_email/test_email.py @@ -3793,6 +3793,172 @@ def test_CRLFLF_at_end_of_part(self): msg = email.message_from_string(m) self.assertTrue(msg.get_payload(0).get_payload().endswith('\r\n')) + def test_rfc6532_s3_5_quoted_printable_subpart(self): + # Issue 44660: email.feedparser Module Lacks Support for Section 3.5 of + # RFC 6532: message/global Emails with non-identity + # Content-Transfer-Encodings. + + m = ( + # Top-level part header: + 'Content-Type: message/global\r\n' + 'Content-Transfer-Encoding: quoted-printable\r\n' + '\r\n' + # Top-level part body: + # Quoted-printable-encoded subpart header: + 'Content-Type: text/plain; charset=3Dutf-8\r\n' + 'X-Really-Long-Header-Field-Name: This header field value is intentionally l=\r\n' + 'ong because soft-wrapped, quoted-printable header fields are an edge case.\r\n' + 'X-Header-Field-with-Unicode-Value: =E2=93=89=E2=93=97=E2=93=98=E2=93=A2 =\r\n' + '=E2=93=98=E2=93=A2 =E2=93=8A=E2=93=9D=E2=93=98=E2=93=92=E2=93=9E=E2=93=\r\n' + '=93=E2=93=94 =E2=93=A3=E2=93=94=E2=93=A7=E2=93=A3.\r\n' + '\r\n' + 'This is ASCII text.\r\n' + '=E2=93=89=E2=93=97=E2=93=98=E2=93=A2 =E2=93=98=E2=93=A2 =E2=93=8A=E2=93=\r\n' + '=9D=E2=93=98=E2=93=92=E2=93=9E=E2=93=93=E2=93=94 =E2=93=A3=E2=93=94=\r\n' + '=E2=93=A7=E2=93=A3.\r\n' + ) + actual_email_object = email.message_from_string( + m, + policy=email.policy.default + ) + self.assertIsInstance( + actual_email_object, + email.message.EmailMessage, + 'Parsed value should be a message part object.' + ) + + self.assertEqual( + dict(actual_email_object), + {'Content-Transfer-Encoding': 'quoted-printable', + 'Content-Type': 'message/global'}, + 'Top-level message headers incorrect.' + ) + + actual_payload = actual_email_object.get_payload() + self.assertIsInstance( + actual_payload, + list, + 'Top-level message body should be a list.' + ) + self.assertFalse( + [subpart for subpart in actual_payload if not isinstance(subpart, Message)], + 'Top-level message body (a list) should contain only subpart object(s).' + ) + self.assertEqual( + len(actual_payload), + 1, + 'Top-level message should have a single subpart.' + ) + actual_subpart: Message = actual_payload[0] + + self.assertEqual( + actual_subpart.get_content_type(), + 'text/plain' + ) + self.assertEqual( + actual_subpart.get_content_charset(), + 'utf-8' + ) + self.assertEqual( + { key: list(actual_subpart.get_all(key)) + for key in actual_subpart.keys() + if key not in ['Content-Type']}, + {'X-Header-Field-with-Unicode-Value': ['Ⓣⓗⓘⓢ ⓘⓢ Ⓤⓝⓘⓒⓞⓓⓔ ⓣⓔⓧⓣ.'], + 'X-Really-Long-Header-Field-Name': [('This header field value is intentionally ' + 'long because soft-wrapped, ' + 'quoted-printable header fields are an ' + 'edge case.')]}, + 'Subpart headers incorrect.' + ) + actual_subpart_payload = actual_subpart.get_payload() + self.assertIsInstance( + actual_subpart_payload, + str, + 'Subpart body is of type "text/plain" so should be a string.' + ) + self.assertEqual( + actual_subpart_payload, + ('This is ASCII text.\r\n' + 'Ⓣⓗⓘⓢ ⓘⓢ Ⓤⓝⓘⓒⓞⓓⓔ ⓣⓔⓧⓣ.\r\n'), + 'Parsed subpart body text might be corrupt.' + ) + + + def test_rfc6532_s3_5_base64_subpart(self): + # Issue 44660: email.feedparser Module Lacks Support for Section 3.5 of + # RFC 6532: message/global Emails with non-identity + # Content-Transfer-Encodings. + + m = ( + # Top-level part header: + 'Content-Type: message/global\r\n' + 'Content-Transfer-Encoding: base64\r\n' + '\r\n' + # Top-level part body: + # Base-64-encoded subpart header: + 'Q29udGVudC1UeXBlOiB0ZXh0L3BsYWluDQpYLUhlYWRlci1GaWVsZC13aXRoLVVuaWNvZGUtVmFs\r\n' + 'dWU6IOKTieKTl+KTmOKToiDik5jik6Ig4pOK4pOd4pOY4pOS4pOe4pOT4pOUIOKTo+KTlOKTp+KT\r\n' + 'oy4NCg0KVGhpcyBpcyBBU0NJSSB0ZXh0Lg==\r\n' + ) + actual_email_object = email.message_from_string( + m, + policy=email.policy.default + ) + self.assertIsInstance( + actual_email_object, + email.message.EmailMessage, + 'Parsed value should be a message part object.' + ) + + self.assertEqual( + dict(actual_email_object), + {'Content-Transfer-Encoding': 'base64', + 'Content-Type': 'message/global'}, + 'Top-level message headers incorrect.' + ) + + actual_payload = actual_email_object.get_payload() + self.assertIsInstance( + actual_payload, + list, + 'Top-level message body should be a list.' + ) + self.assertFalse( + [subpart for subpart in actual_payload if not isinstance(subpart, Message)], + 'Top-level message body (a list) should contain only subpart object(s).' + ) + self.assertEqual( + len(actual_payload), + 1, + 'Top-level message should have a single subpart.' + ) + actual_subpart: Message = actual_payload[0] + + self.assertEqual( + actual_subpart.get_content_type(), + 'text/plain' + ) + self.assertEqual( + {key: list(actual_subpart.get_all(key)) + for key in actual_subpart.keys() + if key not in ['Content-Type']}, + {'X-Header-Field-with-Unicode-Value': [ + 'Ⓣⓗⓘⓢ ⓘⓢ Ⓤⓝⓘⓒⓞⓓⓔ ⓣⓔⓧⓣ.' + ]}, + 'Subpart headers incorrect.' + ) + actual_subpart_payload = actual_subpart.get_payload() + self.assertIsInstance( + actual_subpart_payload, + str, + 'Subpart body is of type "text/plain" so should be a string.' + ) + self.assertEqual( + actual_subpart_payload, + 'This is ASCII text.', + 'Parsed subpart body text might be corrupt.' + ) + class Test8BitBytesHandling(TestEmailBase): # In Python3 all input is string, but that doesn't work if the actual input From 0e60b7697dfba34dc144c8ad2a5a6465d20db691 Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sat, 17 Jul 2021 03:32:02 -0400 Subject: [PATCH 3/7] Corrected comment. --- Lib/test/test_email/test_email.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_email/test_email.py b/Lib/test/test_email/test_email.py index 832a6806400c61..4e14b58083b9dc 100644 --- a/Lib/test/test_email/test_email.py +++ b/Lib/test/test_email/test_email.py @@ -3812,6 +3812,7 @@ def test_rfc6532_s3_5_quoted_printable_subpart(self): '=E2=93=98=E2=93=A2 =E2=93=8A=E2=93=9D=E2=93=98=E2=93=92=E2=93=9E=E2=93=\r\n' '=93=E2=93=94 =E2=93=A3=E2=93=94=E2=93=A7=E2=93=A3.\r\n' '\r\n' + # Quoted-printable-encoded subpart body: 'This is ASCII text.\r\n' '=E2=93=89=E2=93=97=E2=93=98=E2=93=A2 =E2=93=98=E2=93=A2 =E2=93=8A=E2=93=\r\n' '=9D=E2=93=98=E2=93=92=E2=93=9E=E2=93=93=E2=93=94 =E2=93=A3=E2=93=94=\r\n' @@ -3895,7 +3896,7 @@ def test_rfc6532_s3_5_base64_subpart(self): 'Content-Transfer-Encoding: base64\r\n' '\r\n' # Top-level part body: - # Base-64-encoded subpart header: + # Base-64-encoded subpart (header and body): 'Q29udGVudC1UeXBlOiB0ZXh0L3BsYWluDQpYLUhlYWRlci1GaWVsZC13aXRoLVVuaWNvZGUtVmFs\r\n' 'dWU6IOKTieKTl+KTmOKToiDik5jik6Ig4pOK4pOd4pOY4pOS4pOe4pOT4pOUIOKTo+KTlOKTp+KT\r\n' 'oy4NCg0KVGhpcyBpcyBBU0NJSSB0ZXh0Lg==\r\n' From ecc8b1fd747fe565a5be9aedfaa1f0f27a681483 Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sun, 18 Jul 2021 16:12:33 -0400 Subject: [PATCH 4/7] Productionization and attempt to address jdevries3133's recommendations. 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. --- Lib/email/base64mime.py | 52 ++++++++++++++ Lib/email/feedparser.py | 146 ++++++++++++++++++++++++---------------- 2 files changed, 140 insertions(+), 58 deletions(-) diff --git a/Lib/email/base64mime.py b/Lib/email/base64mime.py index a7cc37365c6f9a..bd07a46f8bf48e 100644 --- a/Lib/email/base64mime.py +++ b/Lib/email/base64mime.py @@ -36,6 +36,7 @@ from base64 import b64encode from binascii import b2a_base64, a2b_base64 +from typing import ByteString, Callable, Optional CRLF = '\r\n' NL = '\n' @@ -114,6 +115,57 @@ def decode(string): return a2b_base64(string) +class Base64FeedDecoder: + """ + Adaptation of RFC 2045, s. 6.8 that performs incremental decoding for + FeedParser API. + + Note that there is no parsing-related functionality in this class. + Therefore, this class could be generalized, by making the _feed and _decode + variables optional; and refactored/moved to the top-level, base64 package. + """ + + def __init__(self, feed: Callable[[ByteString], None]): + """ + :param feed: function that, when specified, consumes the decoded data. + """ + self._decode = a2b_base64 # Underlying decoder implementation. + self._feed = feed # Consumes the decoded data. + self._encoded_buffer = bytearray() + + def feed(self, data): + """ + Feed the parser some more base-64-encoded data. data should be a + bytes-like object representing one or more decoded octets. The octets + can be partial and the decoder will stitch such partial octets together + properly. + :param data: bytes-like object of arbitrary-length. + """ + # Decode and parse the largest group of contiguous "strings of 4 + # encoded characters" (described in RFC 2045, s. 6.8, p. 4) available. + decodable_length = int(len(self._encoded_buffer) / 4) * 4 + if decodable_length >= 1: + decodable_bytes = self._encoded_buffer[:decodable_length] + self._encoded_buffer = self._encoded_buffer[decodable_length:] + decoded_bytes = base64.standard_b64decode(decodable_bytes) + self._feed(decoded_bytes) + + def close(self) -> Optional[bytes]: + """ + Complete the decoding of all previously fed data and return the decoded + data. It is undefined what happens if feed() is called after this + method has been called. + :return: Optional[bytes] decoded data. + """ + if len(self._encoded_buffer) >= 1: + # TODO: Raise a different error. + raise Exception('More base64 data expected.') + if self._feed is None: + return bytes(self._decoded_buffer) + # Nothing to return because the decoded data were subsequently fed to + # another function. + + # For convenience and backwards compatibility w/ standard base64 module body_decode = decode decodestring = decode diff --git a/Lib/email/feedparser.py b/Lib/email/feedparser.py index 788275e1abac41..1d133c27d7fc6f 100644 --- a/Lib/email/feedparser.py +++ b/Lib/email/feedparser.py @@ -30,6 +30,7 @@ from email._policybase import compat32 from collections import deque from io import StringIO +from email.base64mime import Base64FeedDecoder NLCRE = re.compile(r'\r\n|\r|\n') NLCRE_bol = re.compile(r'(\r\n|\r|\n)') @@ -296,25 +297,31 @@ def _parsegen(self): self._input.unreadline(line) return if self._cur.get_content_type() == 'message/global': - # Support for message/global parts that have non-identity + # Support for message/global parts that can have non-identity # content-transfer-encodings as outlined in RFC 6532 # (s. 1, p. 3; s 3.5; "Encoding considerations," s. 3.7). - transfer_decoding_parser = _new_transfer_decoding_parser( - self._cur['Content-Transfer-Encoding'], - policy=self.policy, - _factory=self._factory + decoding_parser_factory = _decoding_parser_factory_map.get( + self._cur['Content-Transfer-Encoding'] ) - if isinstance(transfer_decoding_parser, - NonIdentityTransferDecodingFeedParser): - for line in self._input: # Consume all of the content. + if decoding_parser_factory is not None: + # This block only executes if the subpart needs to be decoded as + # it's parsed. Unspecified and identity + # Content-Transfer-Encodings are implicitly handled in a + # subsequent block. + decoding_parser = decoding_parser_factory( + policy=self.policy, + _factory=self._factory + ) + # Decode current part's body and parse as another part: + for line in self._input: if line is NeedMoreData: yield NeedMoreData continue if line == '': break - transfer_decoding_parser.feed(line) - # Retrieve and attach subpart: - subpart = transfer_decoding_parser.close() + decoding_parser.feed(line) + # Retrieve new part and attach (i.e. make a subpart): + subpart = decoding_parser.close() self._cur.attach(subpart) return if self._cur.get_content_maintype() == 'message': @@ -561,15 +568,15 @@ def feed(self, data): super().feed(data.decode('ascii', 'surrogateescape')) -class NonIdentityTransferDecodingFeedParser(abc.ABC): +class EncodedFeedParser(abc.ABC): """ This is an abstract base class; only its subclasses should be instantiated directly. - Instances of this class work like FeedParser except that the feed() method, - prior to parsing the input, transparently decodes the input consistent with - RFC 2045, s. 6.2. Each concrete subclass reverses one of the non-identity - Content-Transfer-Encoding transformations described there. + Instances of this class work like FeedParser except that the concrete + implementations of feed(), prior to parsing the input, transparently decode + the input consistent with RFC 2045, s. 6.2. Each subclass reverses one of + the non-identity Content-Transfer-Encoding transformations described there. """ def __init__(self, *args, **kwargs): @@ -583,61 +590,84 @@ def close(self): return self._bytes_feed_parser.close() -class Base64TransferDecodingFeedParser(NonIdentityTransferDecodingFeedParser): +class Base64EncodedFeedParser(EncodedFeedParser): """ - Concrete, "base64" implementation of TransferDecodingFeedParser. + FeedParser that supports base64-encoded message parts (i.e. the combination + of RFC 2045, s. 6.8; and RFC 6532, particularly s. 3.5). """ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) - self.encoded_buffer = bytearray() - - def feed(self, line): - self.encoded_buffer.extend(line.rstrip().encode('ascii')) - total_decodable = int(len(self.encoded_buffer) / 4) * 4 - if total_decodable >= 1: - decodable_bytes = self.encoded_buffer[:total_decodable] - self.encoded_buffer = self.encoded_buffer[total_decodable:] - decoded_bytes = base64.standard_b64decode(decodable_bytes) - self._bytes_feed_parser.feed(decoded_bytes) + # This buffer, when non-empty between calls to feed(), represents an + # incomplete base64 block that can't be decoded or parsed yet: + self._encoded_buffer = bytearray() + self._decoder = Base64FeedDecoder(self._bytes_feed_parser.feed) + + def feed(self, text): + self._decoder.feed(text.encode('ascii')) + # self._encoded_buffer.extend(text.rstrip().encode('ascii')) + # # Decode and parse the largest group of contiguous "strings of 4 + # # encoded characters" (described in RFC 2045, s. 6.8, p. 4) available. + # decodable_length = int(len(self._encoded_buffer) / 4) * 4 + # if decodable_length >= 1: + # decodable_bytes = self._encoded_buffer[:decodable_length] + # self._encoded_buffer = self._encoded_buffer[decodable_length:] + # decoded_bytes = base64.standard_b64decode(decodable_bytes) + # self._bytes_feed_parser.feed(decoded_bytes) def close(self): - if len(self.encoded_buffer) >= 1: - decoded_bytes = base64.standard_b64decode(self.encoded_buffer) - self._bytes_feed_parser.feed(decoded_bytes) - self.encoded_buffer.clear() + try: + self._decoder.close() + except Exception as e: + # TODO: Add a defect to the message object. + pass return self._bytes_feed_parser.close() -class QuotedPrintableTransferDecodingFeedParser( - NonIdentityTransferDecodingFeedParser -): +class QuotedPrintableFeedParser(EncodedFeedParser): """ - Concrete, "quoted-printable" implementation of TransferDecodingFeedParser. + FeedParser that supports quoted-printable message parts (i.e. the + combination of RFC 2045, s. 6.7; and RFC 6532, particularly s. 3.5). """ - def feed(self, line): - if line == '': - return - if line[-1] != '\n': - line += '\r\n' - encoded_bytes = line.encode('ascii') - decoded_bytes = quopri.decodestring(encoded_bytes) - self._bytes_feed_parser.feed(decoded_bytes) + def __init__(self, *args, **kwargs): + super().__init__(*args, **kwargs) + self._encoded_buffer = bytearray() + def feed(self, text): + self._encoded_buffer.extend(text.encode('ascii')) + if len(text) < 1 or len(self._encoded_buffer) < 1: + # Buffer either hasn't changed since last call or still has nothing + # that can be parsed. + return + index_of_last_equal_sign = self._encoded_buffer.rfind(b'=') + if (index_of_last_equal_sign < 0 + or index_of_last_equal_sign < len(self._encoded_buffer) - 2): + # The buffer either contains no 3-char-sequence, octets/soft line + # breaks; or it contains all three chars of its last octet/soft line + # break; so the whole buffer can be decoded and parsed. + last_decodable_index = len(self._encoded_buffer) - 1 + else: + # The buffer doesn't yet have all three chars of its last octet/soft + # line break, so only the chars leading up to its last equal sign + # can be decoded. + last_decodable_index = index_of_last_equal_sign - 1 + encoded_bytes = self._encoded_buffer[:last_decodable_index + 1] + self._encoded_buffer = self._encoded_buffer[last_decodable_index + 1:] + if len(encoded_bytes) >= 1: + decoded_bytes = quopri.decodestring(encoded_bytes) + self._bytes_feed_parser.feed(decoded_bytes) -def _new_transfer_decoding_parser(content_transfer_encoding, **kwargs): - """ - 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 - content_transfer_encoding specifies an identity transformation (i.e. no - decoding is required), is None or invalid; - NonIdentityTransferDecodingFeedParser, otherwise. - """ + def close(self): + if len(self._encoded_buffer) >= 1: + # TODO: Add a defect to the message object. + pass + return self._bytes_feed_parser.close() - if content_transfer_encoding == 'quoted-printable': - return QuotedPrintableTransferDecodingFeedParser(**kwargs) - if content_transfer_encoding == 'base64': - return Base64TransferDecodingFeedParser(**kwargs) +# Map of EncodedFeedParser "factory" functions keyed by +# Content-Transfer-Encodings. Note that the semantics of "decoding" in this +# context exclude identity transformations (i.e. where no decoding is required): +_decoding_parser_factory_map = { + 'quoted-printable': QuotedPrintableFeedParser, + 'base64': Base64EncodedFeedParser +} From f5fc6f05113e7491ea341ce9573693360413a6bb Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sun, 18 Jul 2021 17:12:18 -0400 Subject: [PATCH 5/7] Added and refactored the error handling. --- Lib/email/base64mime.py | 39 +++++++++++++++++++++------------------ Lib/email/feedparser.py | 18 +++++++++++++----- 2 files changed, 34 insertions(+), 23 deletions(-) diff --git a/Lib/email/base64mime.py b/Lib/email/base64mime.py index bd07a46f8bf48e..f0cee045d2207b 100644 --- a/Lib/email/base64mime.py +++ b/Lib/email/base64mime.py @@ -35,8 +35,9 @@ from base64 import b64encode +from typing import ByteString, Callable + from binascii import b2a_base64, a2b_base64 -from typing import ByteString, Callable, Optional CRLF = '\r\n' NL = '\n' @@ -121,8 +122,10 @@ class Base64FeedDecoder: FeedParser API. Note that there is no parsing-related functionality in this class. - Therefore, this class could be generalized, by making the _feed and _decode - variables optional; and refactored/moved to the top-level, base64 package. + Therefore, this class could be generalized, by making the _feed variable + optional, a new _decode_buffer variable that is returned by close(), + and _decode a constructor kwarg, for example; and refactored/moved to the + top-level, base64 package. """ def __init__(self, feed: Callable[[ByteString], None]): @@ -131,9 +134,11 @@ def __init__(self, feed: Callable[[ByteString], None]): """ self._decode = a2b_base64 # Underlying decoder implementation. self._feed = feed # Consumes the decoded data. + # This buffers an incomplete base-64 block that can't be decoded or + # parsed yet: self._encoded_buffer = bytearray() - def feed(self, data): + def feed(self, data: ByteString): """ Feed the parser some more base-64-encoded data. data should be a bytes-like object representing one or more decoded octets. The octets @@ -141,29 +146,27 @@ def feed(self, data): properly. :param data: bytes-like object of arbitrary-length. """ - # Decode and parse the largest group of contiguous "strings of 4 - # encoded characters" (described in RFC 2045, s. 6.8, p. 4) available. + data = bytes(encoded_byte + for encoded_byte in data + if encoded_byte not in b'\r\n') + self._encoded_buffer.extend(data) decodable_length = int(len(self._encoded_buffer) / 4) * 4 if decodable_length >= 1: decodable_bytes = self._encoded_buffer[:decodable_length] self._encoded_buffer = self._encoded_buffer[decodable_length:] - decoded_bytes = base64.standard_b64decode(decodable_bytes) + decoded_bytes = self._decode(decodable_bytes) self._feed(decoded_bytes) - def close(self) -> Optional[bytes]: + def close(self): """ - Complete the decoding of all previously fed data and return the decoded - data. It is undefined what happens if feed() is called after this - method has been called. - :return: Optional[bytes] decoded data. + Ensure the decoding of all previously fed data; and validate the input + length. If this class had a buffer with the decoded data, normally + this function would also return the buffer. It is undefined what + happens if feed() is called after this method has been called. + :raises: ValueError if the input fails length validation. """ if len(self._encoded_buffer) >= 1: - # TODO: Raise a different error. - raise Exception('More base64 data expected.') - if self._feed is None: - return bytes(self._decoded_buffer) - # Nothing to return because the decoded data were subsequently fed to - # another function. + raise ValueError('The base-64 input has invalid length.') # For convenience and backwards compatibility w/ standard base64 module diff --git a/Lib/email/feedparser.py b/Lib/email/feedparser.py index 1d133c27d7fc6f..d63c9bf3ecd08d 100644 --- a/Lib/email/feedparser.py +++ b/Lib/email/feedparser.py @@ -600,11 +600,15 @@ def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) # This buffer, when non-empty between calls to feed(), represents an # incomplete base64 block that can't be decoded or parsed yet: - self._encoded_buffer = bytearray() self._decoder = Base64FeedDecoder(self._bytes_feed_parser.feed) + self._errors = [] def feed(self, text): - self._decoder.feed(text.encode('ascii')) + encoded_bytes = text.encode('ascii') + try: + self._decoder.feed(encoded_bytes) + except Exception as e: + self._errors.append(e) # self._encoded_buffer.extend(text.rstrip().encode('ascii')) # # Decode and parse the largest group of contiguous "strings of 4 # # encoded characters" (described in RFC 2045, s. 6.8, p. 4) available. @@ -616,12 +620,16 @@ def feed(self, text): # self._bytes_feed_parser.feed(decoded_bytes) def close(self): + message_part = self._bytes_feed_parser.close() + # Attempt to close the decoder in case any further errors occur: try: self._decoder.close() except Exception as e: - # TODO: Add a defect to the message object. - pass - return self._bytes_feed_parser.close() + self._errors.append(e) + # Include the decoding-related errors in the message: + for error in self._errors: + self.policy.handle_defect(message_part, error) + return message_part class QuotedPrintableFeedParser(EncodedFeedParser): From 6155863fa3693066bb74b022e022c7830fcac9e8 Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sun, 18 Jul 2021 17:21:51 -0400 Subject: [PATCH 6/7] Added/updated the comments/docstrings. --- Lib/email/base64mime.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Lib/email/base64mime.py b/Lib/email/base64mime.py index f0cee045d2207b..7235b849f27a47 100644 --- a/Lib/email/base64mime.py +++ b/Lib/email/base64mime.py @@ -146,27 +146,33 @@ def feed(self, data: ByteString): properly. :param data: bytes-like object of arbitrary-length. """ + # Remove whitespace to ensure accurate length calculation: data = bytes(encoded_byte for encoded_byte in data if encoded_byte not in b'\r\n') + # Update buffer and decode any complete base-64 blocks: self._encoded_buffer.extend(data) decodable_length = int(len(self._encoded_buffer) / 4) * 4 if decodable_length >= 1: decodable_bytes = self._encoded_buffer[:decodable_length] self._encoded_buffer = self._encoded_buffer[decodable_length:] decoded_bytes = self._decode(decodable_bytes) + # If _feed were made optional, then the decoded bytes could be + # appended to a new self._decoded_buffer variable when _feed is + # None: self._feed(decoded_bytes) def close(self): """ Ensure the decoding of all previously fed data; and validate the input - length. If this class had a buffer with the decoded data, normally - this function would also return the buffer. It is undefined what - happens if feed() is called after this method has been called. + length. It is undefined what happens if feed() is called after this + method has been called. :raises: ValueError if the input fails length validation. """ if len(self._encoded_buffer) >= 1: raise ValueError('The base-64 input has invalid length.') + # If _feed were made optional, then a new self._decoded_buffer variable + # could be returned when _feed is None. # For convenience and backwards compatibility w/ standard base64 module From b1222f441702432c3e362db164f6d6d0cdf359ff Mon Sep 17 00:00:00 2001 From: f18a14c09s <35183457+f18a14c09s@users.noreply.github.com> Date: Sun, 18 Jul 2021 17:22:57 -0400 Subject: [PATCH 7/7] Removed unused, commented-out code. --- Lib/email/feedparser.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/Lib/email/feedparser.py b/Lib/email/feedparser.py index d63c9bf3ecd08d..2a3ed89e369849 100644 --- a/Lib/email/feedparser.py +++ b/Lib/email/feedparser.py @@ -609,15 +609,6 @@ def feed(self, text): self._decoder.feed(encoded_bytes) except Exception as e: self._errors.append(e) - # self._encoded_buffer.extend(text.rstrip().encode('ascii')) - # # Decode and parse the largest group of contiguous "strings of 4 - # # encoded characters" (described in RFC 2045, s. 6.8, p. 4) available. - # decodable_length = int(len(self._encoded_buffer) / 4) * 4 - # if decodable_length >= 1: - # decodable_bytes = self._encoded_buffer[:decodable_length] - # self._encoded_buffer = self._encoded_buffer[decodable_length:] - # decoded_bytes = base64.standard_b64decode(decodable_bytes) - # self._bytes_feed_parser.feed(decoded_bytes) def close(self): message_part = self._bytes_feed_parser.close() @@ -672,6 +663,7 @@ def close(self): pass return self._bytes_feed_parser.close() + # Map of EncodedFeedParser "factory" functions keyed by # Content-Transfer-Encodings. Note that the semantics of "decoding" in this # context exclude identity transformations (i.e. where no decoding is required):