#12537: in mailbox avoid depending on knowledge of email package internals

Previously mailbox was copying a list of attributes from one message object to
another in order to "copy the message data".  This means that any time new
attributes were added to email.message.Message, mailbox broke.  Now instead it
copies all attributes from the source object to the target object, skipping
any mailbox-object-specific attributes to produce the same clean initial
state it was previously getting by copying only the "known" attributes.

David Lam assisted in the development of this patch.

diff --git a/Lib/mailbox.py b/Lib/mailbox.py
index 325b9c9ac0d82d01e2aa388c5fbc1ee6b837cf39..7a29555e4a8fbb427ba33248dab3e0e0776c6571 100644 (file)
--- a/Lib/mailbox.py
+++ b/Lib/mailbox.py
@@ -1462,9 +1462,10 @@ class Message(email.message.Message):
 
     def _become_message(self, message):
         """Assume the non-format-specific state of message."""
-        for name in ('_headers', '_unixfrom', '_payload', '_charset',
-                     'preamble', 'epilogue', 'defects', '_default_type'):
-            self.__dict__[name] = message.__dict__[name]
+        type_specific = getattr(message, '_type_specific_attributes', [])
+        for name in message.__dict__:
+            if name not in type_specific:
+                self.__dict__[name] = message.__dict__[name]
 
     def _explain_to(self, message):
         """Copy format-specific state to message insofar as possible."""
@@ -1477,6 +1478,8 @@ class Message(email.message.Message):
 class MaildirMessage(Message):
     """Message with Maildir-specific properties."""
 
+    _type_specific_attributes = ['_subdir', '_info', '_date']
+
     def __init__(self, message=None):
         """Initialize a MaildirMessage instance."""
         self._subdir = 'new'
@@ -1584,6 +1587,8 @@ class MaildirMessage(Message):
 class _mboxMMDFMessage(Message):
     """Message with mbox- or MMDF-specific properties."""
 
+    _type_specific_attributes = ['_from']
+
     def __init__(self, message=None):
         """Initialize an mboxMMDFMessage instance."""
         self.set_from('MAILER-DAEMON', True)
@@ -1699,6 +1704,8 @@ class mboxMessage(_mboxMMDFMessage):
 class MHMessage(Message):
     """Message with MH-specific properties."""
 
+    _type_specific_attributes = ['_sequences']
+
     def __init__(self, message=None):
         """Initialize an MHMessage instance."""
         self._sequences = []
@@ -1769,6 +1776,8 @@ class MHMessage(Message):
 class BabylMessage(Message):
     """Message with Babyl-specific properties."""
 
+    _type_specific_attributes = ['_labels', '_visible']
+
     def __init__(self, message=None):
         """Initialize an BabylMessage instance."""
         self._labels = []

diff --git a/Lib/test/test_mailbox.py b/Lib/test/test_mailbox.py
index 95f8e0d2a2b16b0b201f47e98cecd78cf46de026..0fe4bd9dd051a95dea8323c7c59257a98508d993 100644 (file)
--- a/Lib/test/test_mailbox.py
+++ b/Lib/test/test_mailbox.py
@@ -1330,6 +1330,14 @@ class TestMessage(TestBase, unittest.TestCase):
         # Initialize with invalid argument
         self.assertRaises(TypeError, lambda: self._factory(object()))
 
+    def test_all_eMM_attribues_exist(self):
+        # Issue 12537
+        eMM = email.message_from_string(_sample_message)
+        msg = self._factory(_sample_message)
+        for attr in eMM.__dict__:
+            self.assertTrue(attr in msg.__dict__,
+                '{} attribute does not exist'.format(attr))
+
     def test_become_message(self):
         # Take on the state of another message
         eMM = email.message_from_string(_sample_message)
@@ -1596,6 +1604,21 @@ class TestMessageConversion(TestBase, unittest.TestCase):
         for class_ in self.all_mailbox_types:
             self.assertRaises(TypeError, lambda: class_(False))
 
+    def test_type_specific_attributes_removed_on_conversion(self):
+        reference = {class_: class_(_sample_message).__dict__
+                        for class_ in self.all_mailbox_types}
+        for class1 in self.all_mailbox_types:
+            for class2 in self.all_mailbox_types:
+                if class1 is class2:
+                    continue
+                source = class1(_sample_message)
+                target = class2(source)
+                type_specific = [a for a in reference[class1]
+                                   if a not in reference[class2]]
+                for attr in type_specific:
+                    self.assertNotIn(attr, target.__dict__,
+                        "while converting {} to {}".format(class1, class2))
+
     def test_maildir_to_maildir(self):
         # Convert MaildirMessage to MaildirMessage
         msg_maildir = mailbox.MaildirMessage(_sample_message)

diff --git a/Misc/NEWS b/Misc/NEWS
index 6256e02575e7cef5b2e872aa9c6aac21b7bc87e3..4c3758d915c867b11e724d7295a2a94c9deb8e4e 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -19,6 +19,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #12537: The mailbox module no longer depends on knowledge of internal
+  implementation details of the email package Message object.
+
 - Issue #7978: socketserver now restarts the select() call when EINTR is
   returned.  This avoids crashing the server loop when a signal is received.
   Patch by Jerzy Kozera.