bpo-39100: _header_value_parser: do not treat a Group as invalid-mailbox (#24872)

When an address in an address-list has garbage at the end, the code will
currently:

1. change the mailbox in the last parsed address into invalid-mailbox by
   overriding its token_type;
2. wrap the trailing garbage into another invalid-mailbox and append it
   to the last parsed address.

However, that does not take into account that an address may
also contain a Group instead of a single mailbox. In that case,
overwriting token_type leads to undesirable results, e.g. parsing an
email with the following 'To' header:

unlisted-recipients:; (no To-header on input)

raises an AttributeError from trying to treat the Group as a Mailbox.

Moreover it is questionable whether the previously parsed mailbox should
be treated as invalid in addition to the trailing garbage.

Address both of the above by wrapping the trailing garbage in a new
Address with a single invalid-mailbox, and append it to the AddressList
directly.

Changes the results of the
test_get_address_list_mailboxes_invalid_addresses test, where the
address list is now parsed into 4 mailboxes instead of 3 (all but the
first one are invalid).

diff --git a/Lib/email/_header_value_parser.py b/Lib/email/_header_value_parser.py
index 4c5394ab6353ac492c7aa8de6d50d6dfbaecba10..f6b45e13271d7a0dae968812cd563273c42c5c26 100644 (file)
--- a/Lib/email/_header_value_parser.py
+++ b/Lib/email/_header_value_parser.py
@@ -2063,12 +2063,10 @@ def get_address_list(value):
                 address_list.defects.append(errors.InvalidHeaderDefect(
                     "invalid address in address-list"))
         if value and value[0] != ',':
-            # Crap after address; treat it as an invalid mailbox.
-            # The mailbox info will still be available.
-            mailbox = address_list[-1][0]
-            mailbox.token_type = 'invalid-mailbox'
+            # Crap after address: add it to the address list
+            # as an invalid mailbox
             token, value = get_invalid_mailbox(value, ',')
-            mailbox.extend(token)
+            address_list.append(Address([token]))
             address_list.defects.append(errors.InvalidHeaderDefect(
                 "invalid address in address-list"))
         if value:  # Must be a , at this point.

diff --git a/Lib/test/test_email/test__header_value_parser.py b/Lib/test/test_email/test__header_value_parser.py
index e28fe3892015b921541eabcbc2e198e61c9c1ec9..f3c03062572ba5e96d0c26b134562dd51734742d 100644 (file)
--- a/Lib/test/test_email/test__header_value_parser.py
+++ b/Lib/test/test_email/test__header_value_parser.py
@@ -2617,7 +2617,7 @@ class TestParser(TestParserMixin, TestEmailBase):
             '')
         self.assertEqual(address_list.token_type, 'address-list')
         self.assertEqual(len(address_list.mailboxes), 1)
-        self.assertEqual(len(address_list.all_mailboxes), 3)
+        self.assertEqual(len(address_list.all_mailboxes), 4)
         self.assertEqual([str(x) for x in address_list.all_mailboxes],
                          [str(x) for x in address_list.addresses])
         self.assertEqual(address_list.mailboxes[0].domain, 'example.com')
@@ -2626,11 +2626,13 @@ class TestParser(TestParserMixin, TestEmailBase):
         self.assertEqual(address_list.addresses[1].token_type, 'address')
         self.assertEqual(len(address_list.addresses[0].mailboxes), 1)
         self.assertEqual(len(address_list.addresses[1].mailboxes), 0)
-        self.assertEqual(len(address_list.addresses[1].mailboxes), 0)
+        self.assertEqual(len(address_list.addresses[2].mailboxes), 0)
+        self.assertEqual(len(address_list.addresses[3].mailboxes), 0)
         self.assertEqual(
             address_list.addresses[1].all_mailboxes[0].local_part, 'Foo x')
+        self.assertEqual(address_list.addresses[2].all_mailboxes[0].value, '[]')
         self.assertEqual(
-            address_list.addresses[2].all_mailboxes[0].display_name,
+            address_list.addresses[3].all_mailboxes[0].display_name,
                 "Nobody Is. Special")
 
     def test_get_address_list_group_empty(self):
@@ -2695,6 +2697,14 @@ class TestParser(TestParserMixin, TestEmailBase):
         self.assertEqual(str(address_list.addresses[1]),
                          str(address_list.mailboxes[2]))
 
+    def test_get_address_list_trailing_garbage(self):
+        address_list = self._test_get_x(parser.get_address_list,
+            'unlisted-recipients:; (no To-header on input)',
+            'unlisted-recipients:; (no To-header on input)',
+            'unlisted-recipients:; ',
+            [errors.InvalidHeaderDefect]*2 + [errors.ObsoleteHeaderDefect],
+            '')
+
     def test_invalid_content_disposition(self):
         content_disp = self._test_parse_x(
             parser.parse_content_disposition_header,

diff --git a/Misc/NEWS.d/next/Library/2023-09-08-13-10-32.gh-issue-83281.2Plpcj.rst b/Misc/NEWS.d/next/Library/2023-09-08-13-10-32.gh-issue-83281.2Plpcj.rst
new file mode 100644 (file)
index 0000000..cf2ae77
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2023-09-08-13-10-32.gh-issue-83281.2Plpcj.rst
@@ -0,0 +1,2 @@
+:mod:`email`: improve handling trailing garbage in address lists to avoid throwing
+AttributeError in certain edge cases