[FIX] base: avoid bpo-35805 corrupting Message-Id

odony · odony · commit 02b78770147e · 2020-08-09T15:21:41.000Z
Python 3 before 3.8 has a bug that causes the email.policy classes to incorrectly fold and RFC2047-encode "identification fields" in email messages. This mainly applies to Message-Id, References, and In-Reply-To fields. We are impacted by this bug since odoo#35929 where we switched to using the "modern" email.message API. RFC2047 section 5 clearly states that those headers/fields are not to be encoded, and that would violate RFC5322. Further, such a folded Message-Id is considered non-RFC-conformant by popular MTAs (GMail, Outlook), which will then generate *another* Message-Id field, causing the original threading information to be lost. Replies to such a modified message will reference the new, unknown Message-Id, and won't be attached to the original thread. The solution we adopt here is to monkey-patch the SMTP policies to special-case those identification fields and deactivate the automatic folding, until the bug is properly and fully fixed in the standard lib. Some considerations taken into account for this patch: - `email.policy.SMTP` is being monkey-patched globally to make sure we fix all possible places where Messages are being encoded/folded - the fix is **not** made version-specific, considering that even in Python 3.8 the official bugfix only applies to Message-Id, but still fails to protect other identification fields, like *References* and *In-Reply-To*. The author specifically noted that shortcoming [2]. The fix wouldn't break anything on Python 3.8 anyway. - the `noFoldPolicy` trick for preventing folding is done with no max line length at all. RFC5322, section 2.1.1 states [3] that the maximum length is 998 due to legacy implementations, but there is no provision to wrap identification fields that are longer than that. Wrapping at 998 chars would corrupt the header anyway. We'll just count on the fact that we don't usually need 1k+ chars in those headers. The invalid folding/encoding in action on Python 3.6 (in Python 3.8 only the second header gets folded): ```py >>> msg = email.message.EmailMessage(policy=email.policy.SMTP) >>> msg['Message-Id'] = '<929227342217024.1596730490.324691772460938-example-30661-some.reference@test-123.example.com>' >>> msg['In-Reply-To'] = '<92922734221723.1596730568.324691772460444-another-30661-parent.reference@test-123.example.com>' >>> print(msg.as_string()) Message-Id: =?utf-8?q?=3C929227342217024=2E1596730490=2E324691772460938-exam?= =?utf-8?q?ple-30661-some=2Ereference=40test-123=2Eexample=2Ecom=3E?= In-Reply-To: =?utf-8?q?=3C92922734221723=2E1596730568=2E324691772460444-anot?= =?utf-8?q?her-30661-parent=2Ereference=40test-123=2Eexample=2Ecom=3E?= ``` and the expected result after the fix: ```py >>> msg = email.message.EmailMessage(policy=email.policy.SMTP) >>> msg['Message-Id'] = '<929227342217024.1596730490.324691772460938-example-30661-some.reference@test-123.example.com>' >>> msg['In-Reply-To'] = '<92922734221723.1596730568.324691772460444-another-30661-parent.reference@test-123.example.com>' >>> print(msg.as_string()) Message-Id: <929227342217024.1596730490.324691772460938-example-30661-some.reference@test-123.example.com> In-Reply-To: <92922734221723.1596730568.324691772460444-another-30661-parent.reference@test-123.example.com> ``` [1] bpo-35805: https://bugs.python.org/issue35805 [2] python/cpython#13397 (comment) [3] https://tools.ietf.org/html/rfc5322#section-2.1.1 closes odoo#55655 X-original-commit: 6726e9a Signed-off-by: Olivier Dony (odo) <odo@openerp.com>
diff --git a/odoo/addons/base/models/ir_mail_server.py b/odoo/addons/base/models/ir_mail_server.py
@@ -33,6 +33,21 @@ def _print_debug(self, *args):
     _logger.debug(' '.join(str(a) for a in args))
 smtplib.SMTP._print_debug = _print_debug
 
+# Python 3: workaround for bpo-35805, only partially fixed in Python 3.8.
+RFC5322_IDENTIFICATION_HEADERS = {'message-id', 'in-reply-to', 'references', 'resent-msg-id'}
+_noFoldPolicy = email.policy.SMTP.clone(max_line_length=None)
+class IdentificationFieldsNoFoldPolicy(email.policy.EmailPolicy):
+    # Override _fold() to avoid folding identification fields, excluded by RFC2047 section 5
+    # These are particularly important to preserve, as MTAs will often rewrite non-conformant
+    # Message-ID headers, causing a loss of thread information (replies are lost)
+    def _fold(self, name, value, *args, **kwargs):
+        if name.lower() in RFC5322_IDENTIFICATION_HEADERS:
+            return _noFoldPolicy._fold(name, value, *args, **kwargs)
+        return super()._fold(name, value, *args, **kwargs)
+
+# Global monkey-patch for our preferred SMTP policy, preserving the non-default linesep
+email.policy.SMTP = IdentificationFieldsNoFoldPolicy(linesep=email.policy.SMTP.linesep)
+
 # Python 2: replace smtplib's stderr
 class WriteToLogger(object):
     def write(self, s):
diff --git a/odoo/addons/base/tests/test_mail.py b/odoo/addons/base/tests/test_mail.py
@@ -397,38 +397,42 @@ def test_default_email_from(self, *args):
 
 class TestEmailMessage(TransactionCase):
     def test_as_string(self):
-        """Ensure all email sent are bpo-34424 free"""
+        """Ensure all email sent are bpo-34424 and bpo-35805 free"""
+
+        message_truth = (
+            r'From: .+? <joe@example\.com>\r\n'
+            r'To: .+? <joe@example\.com>\r\n'
+            r'Message-Id: <[0-9a-z.-]+@[0-9a-z.-]+>\r\n'
+            r'References: (<[0-9a-z.-]+@[0-9a-z.-]+>\s*)+\r\n'
+            r'\r\n'
+        )
 
         class FakeSMTP:
             """SMTP stub"""
             def __init__(this):
                 this.email_sent = False
 
+            # Python 3 before 3.7.4
             def sendmail(this, smtp_from, smtp_to_list, message_str,
                          mail_options=(), rcpt_options=()):
                 this.email_sent = True
-                message_truth = (
-                    r'From: .+? <joe@example\.com>\r\n'
-                    r'To: .+? <joe@example\.com>\r\n'
-                    r'\r\n'
-                )
                 self.assertRegex(message_str, message_truth)
 
+            # Python 3.7.4+
             def send_message(this, message, smtp_from, smtp_to_list,
                              mail_options=(), rcpt_options=()):
                 message_str = message.as_string()
                 this.email_sent = True
-                message_truth = (
-                    r'From: .+? <joe@example\.com>\r\n'
-                    r'To: .+? <joe@example\.com>\r\n'
-                    r'\r\n'
-                )
                 self.assertRegex(message_str, message_truth)
 
         msg = email.message.EmailMessage(policy=email.policy.SMTP)
         msg['From'] = '"Jo&eacute; Doe" <joe@example.com>'
         msg['To'] = '"Jo&eacute; Doe" <joe@example.com>'
 
+        # Message-Id & References fields longer than 77 chars (bpo-35805)
+        msg['Message-Id'] = '<929227342217024.1596730490.324691772460938-example-30661-some.reference@test-123.example.com>'
+        msg['References'] = '<345227342212345.1596730777.324691772483620-example-30453-other.reference@test-123.example.com>'
+
         smtp = FakeSMTP()
         self.patch(threading.currentThread(), 'testing', False)
         self.env['ir.mail_server'].send_email(msg, smtp_session=smtp)
