* Messages now include a `Message-ID-Hash` as the replacement for

   `X-Message-ID-Hash` although the latter is still included for backward
   compatibility.  Also be sure that all places which add the header use the
   same algorithm.

2 files changed, 75 insertions, 16 deletions

diff --git a/src/mailman/utilities/email.py b/src/mailman/utilities/email.py
index 2243686d1..7d3f8b7b0 100644
--- a/src/mailman/utilities/email.py
+++ b/src/mailman/utilities/email.py
@@ -46,15 +46,17 @@ def split_email(address):
 
 
 def add_message_hash(msg):
-    """Add a X-Message-ID-Hash header derived from Message-ID.
+    """Add a Message-ID-Hash header derived from Message-ID.
 
-    This function works by side-effect; the original message is mutated.  Any
-    existing X-Message-ID-Headers are deleted if a Message-ID header is
-    found.  If no Message-ID header is found, the original message is not
-    modified.
+    This function works by side-effect; the original message is mutated.
+    Any existing Message-ID-Headers are deleted if a Message-ID header
+    is found.  If no Message-ID header is found, the original message is
+    not modified.
 
     :param msg: An email message
     :type msg: `email.message.Message` or derived
+    :return: The Message-ID-Hash contents.
+    :rtype: str
     """
     message_id = msg.get('message-id')
     if message_id is None:
@@ -71,6 +73,10 @@ def add_message_hash(msg):
     # we need a string for the header value.  We know the b32encoded byte
     # string must be ascii-only.
     digest = sha1(message_id.encode('utf-8')).digest()
-    message_id_hash = b32encode(digest)
+    hash32 = b32encode(digest).decode('ascii')
+    del msg['message-id-hash']
+    msg['Message-ID-Hash'] = hash32
+    # For backward compatibility with previous versions of the spec.
     del msg['x-message-id-hash']
-    msg['X-Message-ID-Hash'] = message_id_hash.decode('ascii')
+    msg['X-Message-ID-Hash'] = hash32
+    return hash32
diff --git a/src/mailman/utilities/tests/test_email.py b/src/mailman/utilities/tests/test_email.py
index 580a49805..44fa7d492 100644
--- a/src/mailman/utilities/tests/test_email.py
+++ b/src/mailman/utilities/tests/test_email.py
@@ -19,14 +19,18 @@
 
 __all__ = [
     'TestEmail',
+    'TestMessageIDHash',
     ]
 
 
 import unittest
 
+from mailman.interfaces.messages import IMessageStore
 from mailman.testing.helpers import (
     specialized_message_from_string as mfs)
+from mailman.testing.layers import ConfigLayer
 from mailman.utilities.email import add_message_hash, split_email
+from zope.component import getUtility
 
 
 
@@ -50,30 +54,30 @@ Message-ID: <aardvark>
 
 """)
         add_message_hash(msg)
-        self.assertEqual(msg['x-message-id-hash'],
+        self.assertEqual(msg['message-id-hash'],
                          '75E2XSUXAFQGWANWEROVQ7JGYMNWHJBT')
 
     def test_remove_hash_headers_first(self):
         # Any existing X-Mailman-Hash-ID header is removed first.
         msg = mfs("""\
 Message-ID: <aardvark>
-X-Message-ID-Hash: abc
+Message-ID-Hash: abc
 
 """)
         add_message_hash(msg)
-        headers = msg.get_all('x-message-id-hash')
+        headers = msg.get_all('message-id-hash')
         self.assertEqual(len(headers), 1)
         self.assertEqual(headers[0], '75E2XSUXAFQGWANWEROVQ7JGYMNWHJBT')
 
     def test_hash_header_left_alone_if_no_message_id(self):
         # If the original message has no Message-ID header, then any existing
-        # X-Message-ID-Hash headers are left intact.
+        # Message-ID-Hash headers are left intact.
         msg = mfs("""\
-X-Message-ID-Hash: abc
+Message-ID-Hash: abc
 
 """)
         add_message_hash(msg)
-        headers = msg.get_all('x-message-id-hash')
+        headers = msg.get_all('message-id-hash')
         self.assertEqual(len(headers), 1)
         self.assertEqual(headers[0], 'abc')
 
@@ -85,7 +89,7 @@ Message-ID: aardvark
 
 """)
         add_message_hash(msg)
-        self.assertEqual(msg['x-message-id-hash'],
+        self.assertEqual(msg['message-id-hash'],
                          '75E2XSUXAFQGWANWEROVQ7JGYMNWHJBT')
 
     def test_mismatched_angle_brackets_do_contribute_to_hash(self):
@@ -96,12 +100,61 @@ Message-ID: <aardvark
 
 """)
         add_message_hash(msg)
-        self.assertEqual(msg['x-message-id-hash'],
+        self.assertEqual(msg['message-id-hash'],
                          'AOJ545GHRYD2Y3RUFG2EWMPHUABTG4SM')
         msg = mfs("""\
 Message-ID: aardvark>
 
 """)
         add_message_hash(msg)
-        self.assertEqual(msg['x-message-id-hash'],
+        self.assertEqual(msg['message-id-hash'],
                          '5KH3RA7ZM4VM6XOZXA7AST2XN2X4S3WY')
+
+    def test_return_value(self):
+        msg = mfs("""\
+Message-ID: aardvark>
+
+""")
+        hash32 = add_message_hash(msg)
+        self.assertEqual(hash32, '5KH3RA7ZM4VM6XOZXA7AST2XN2X4S3WY')
+
+
+
+class TestMessageIDHash(unittest.TestCase):
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._store = getUtility(IMessageStore)
+
+    def test_message_id_hash_backward_compatibility(self):
+        msg = mfs("""\
+Message-ID: <ant>
+
+""")
+        self._store.add(msg)
+        stored_msg = self._store.get_message_by_id('<ant>')
+        self.assertEqual(stored_msg['message-id-hash'],
+                         'MS6QLWERIJLGCRF44J7USBFDELMNT2BW')
+        # For backward compatibility with the old spec.
+        self.assertEqual(stored_msg['x-message-id-hash'],
+                         'MS6QLWERIJLGCRF44J7USBFDELMNT2BW')
+
+    def test_message_id_hash_gets_replaced_backward_compatibility(self):
+        msg = mfs("""\
+Message-ID: <ant>
+Message-ID-Hash: abc
+X-Message-ID-Hash: abc
+
+""")
+        self._store.add(msg)
+        stored_msg = self._store.get_message_by_id('<ant>')
+        message_id_hashes = stored_msg.get_all('message-id-hash')
+        self.assertEqual(len(message_id_hashes), 1)
+        self.assertEqual(message_id_hashes[0],
+                         'MS6QLWERIJLGCRF44J7USBFDELMNT2BW')
+        # For backward compatibility with the old spec.
+        x_message_id_hashes = stored_msg.get_all('x-message-id-hash')
+        self.assertEqual(len(x_message_id_hashes), 1)
+        self.assertEqual(x_message_id_hashes[0],
+                         'MS6QLWERIJLGCRF44J7USBFDELMNT2BW')