diff options
22 files changed, 252 insertions, 127 deletions
diff --git a/src/mailman/app/inject.py b/src/mailman/app/inject.py index d22bf493d..74e569484 100644 --- a/src/mailman/app/inject.py +++ b/src/mailman/app/inject.py @@ -35,7 +35,7 @@ def inject_message(mlist, msg, recipients=None, switchboard=None, **kws): """Inject a message into a queue. If the message does not have a Message-ID header, one is added. An - X-Message-Id-Hash header is also always added. + Message-ID-Hash header is also always added. :param mlist: The mailing list this message is destined for. :type mlist: IMailingList @@ -78,7 +78,7 @@ def inject_text(mlist, text, recipients=None, switchboard=None, **kws): """Turn text into a message and inject that into a queue. If the text does not have a Message-ID header, one is added. An - X-Message-Id-Hash header is also always added. + Message-ID-Hash header is also always added. :param mlist: The mailing list this message is destined for. :type mlist: IMailingList diff --git a/src/mailman/app/tests/test_inject.py b/src/mailman/app/tests/test_inject.py index 614942f7e..c11b5cb59 100644 --- a/src/mailman/app/tests/test_inject.py +++ b/src/mailman/app/tests/test_inject.py @@ -90,21 +90,21 @@ Nothing. def test_inject_message_without_message_id(self): # inject_message() adds a Message-ID header if it's missing. del self.msg['message-id'] - self.assertFalse('message-id' in self.msg) + self.assertNotIn('message-id', self.msg) inject_message(self.mlist, self.msg) - self.assertTrue('message-id' in self.msg) + self.assertIn('message-id', self.msg) items = get_queue_messages('in') - self.assertTrue('message-id' in items[0].msg) + self.assertIn('message-id', items[0].msg) self.assertEqual(items[0].msg['message-id'], self.msg['message-id']) def test_inject_message_without_date(self): # inject_message() adds a Date header if it's missing. del self.msg['date'] - self.assertFalse('date' in self.msg) + self.assertNotIn('date', self.msg) inject_message(self.mlist, self.msg) - self.assertTrue('date' in self.msg) + self.assertIn('date', self.msg) items = get_queue_messages('in') - self.assertTrue('date' in items[0].msg) + self.assertIn('date', items[0].msg) self.assertEqual(items[0].msg['date'], self.msg['date']) def test_inject_message_with_keywords(self): @@ -116,23 +116,23 @@ Nothing. def test_inject_message_id_hash(self): # When the injected message has a Message-ID header, the injected - # message will also get an X-Message-ID-Hash header. + # message will also get an Message-ID-Hash header. inject_message(self.mlist, self.msg) items = get_queue_messages('in') - self.assertEqual(items[0].msg['x-message-id-hash'], + self.assertEqual(items[0].msg['message-id-hash'], '4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB') def test_inject_message_id_hash_without_message_id(self): # When the injected message does not have a Message-ID header, a # Message-ID header will be added, and the injected message will also - # get an X-Message-ID-Hash header. + # get an Message-ID-Hash header. del self.msg['message-id'] - self.assertFalse('message-id' in self.msg) - self.assertFalse('x-message-id-hash' in self.msg) + self.assertNotIn('message-id', self.msg) + self.assertNotIn('message-id-hash', self.msg) inject_message(self.mlist, self.msg) items = get_queue_messages('in') - self.assertTrue('message-id' in items[0].msg) - self.assertTrue('x-message-id-hash' in items[0].msg) + self.assertIn('message-id', items[0].msg) + self.assertIn('message-id-hash', items[0].msg) @@ -140,6 +140,7 @@ class TestInjectText(unittest.TestCase): """Test text injection.""" layer = ConfigLayer + maxDiff = None def setUp(self): self.mlist = create_list('test@example.com') @@ -152,8 +153,6 @@ Date: Tue, 14 Jun 2011 21:12:00 -0400 Nothing. """ - # Python 2.7 has a better equality tester for message texts. - self.maxDiff = None def _remove_line(self, header): return NL.join(line for line in self.text.splitlines() @@ -165,18 +164,19 @@ Nothing. items = get_queue_messages('in') self.assertEqual(len(items), 1) self.assertTrue(isinstance(items[0].msg, Message)) - self.assertEqual(items[0].msg['x-message-id-hash'], + self.assertEqual(items[0].msg['message-id-hash'], 'GUXXQKNCHBFQAHGBFMGCME6HKZCUUH3K') - # Delete that header because it is not in the original text. + # Delete these headers because they don't exist in the original text. + del items[0].msg['message-id-hash'] del items[0].msg['x-message-id-hash'] self.assertMultiLineEqual(items[0].msg.as_string(), self.text) self.assertEqual(items[0].msgdata['listid'], 'test.example.com') self.assertEqual(items[0].msgdata['original_size'], - # Add back the X-Message-ID-Header which was in the - # message contributing to the original_size, but - # wasn't in the original text. Don't forget the - # newline! - len(self.text) + 52) + # Add back the Message-ID-Hash and X-Message-ID-Hash + # headers which wer in the message contributing to the + # original_size, but weren't in the original text. + # Don't forget the space, delimeter, and newline! + len(self.text) + 50 + 52) def test_inject_text_with_recipients(self): # Explicit recipients end up in the metadata. @@ -192,32 +192,33 @@ Nothing. self.assertEqual(len(items), 0) items = get_queue_messages('virgin') self.assertEqual(len(items), 1) - # Remove the X-Message-ID-Hash header which isn't in the original text. + # Remove the Message-ID-Hash header which isn't in the original text. + del items[0].msg['message-id-hash'] del items[0].msg['x-message-id-hash'] self.assertMultiLineEqual(items[0].msg.as_string(), self.text) self.assertEqual(items[0].msgdata['listid'], 'test.example.com') self.assertEqual(items[0].msgdata['original_size'], - # Add back the X-Message-ID-Header which was in the - # message contributing to the original_size, but - # wasn't in the original text. Don't forget the - # newline! - len(self.text) + 52) + # Add back the Message-ID-Hash and X-Message-ID-Hash + # headers which wer in the message contributing to the + # original_size, but weren't in the original text. + # Don't forget the space, delimeter, and newline! + len(self.text) + 50 + 52) def test_inject_text_without_message_id(self): # inject_text() adds a Message-ID header if it's missing. filtered = self._remove_line('message-id') - self.assertFalse('Message-ID' in filtered) + self.assertNotIn('Message-ID', filtered) inject_text(self.mlist, filtered) items = get_queue_messages('in') - self.assertTrue('message-id' in items[0].msg) + self.assertIn('message-id', items[0].msg) def test_inject_text_without_date(self): # inject_text() adds a Date header if it's missing. filtered = self._remove_line('date') - self.assertFalse('date' in filtered) + self.assertNotIn('date', filtered) inject_text(self.mlist, self.text) items = get_queue_messages('in') - self.assertTrue('date' in items[0].msg) + self.assertIn('date', items[0].msg) def test_inject_text_adds_original_size(self): # The metadata gets an original_size attribute that is the length of @@ -225,11 +226,11 @@ Nothing. inject_text(self.mlist, self.text) items = get_queue_messages('in') self.assertEqual(items[0].msgdata['original_size'], - # Add back the X-Message-ID-Header which was in the - # message contributing to the original_size, but - # wasn't in the original text. Don't forget the - # newline! - len(self.text) + 52) + # Add back the Message-ID-Hash and X-Message-ID-Hash + # headers which wer in the message contributing to the + # original_size, but weren't in the original text. + # Don't forget the space, delimeter, and newline! + len(self.text) + 50 + 52) def test_inject_text_with_keywords(self): # Keyword arguments are copied into the metadata. @@ -240,20 +241,20 @@ Nothing. def test_inject_message_id_hash(self): # When the injected message has a Message-ID header, the injected - # message will also get an X-Message-ID-Hash header. + # message will also get an Message-ID-Hash header. inject_text(self.mlist, self.text) items = get_queue_messages('in') - self.assertEqual(items[0].msg['x-message-id-hash'], + self.assertEqual(items[0].msg['message-id-hash'], 'GUXXQKNCHBFQAHGBFMGCME6HKZCUUH3K') def test_inject_message_id_hash_without_message_id(self): # When the injected message does not have a Message-ID header, a # Message-ID header will be added, and the injected message will also - # get an X-Message-ID-Hash header. + # get an Message-ID-Hash header. filtered = self._remove_line('message-id') - self.assertFalse('Message-ID' in filtered) - self.assertFalse('X-Message-ID-Hash' in filtered) + self.assertNotIn('Message-ID', filtered) + self.assertNotIn('Message-ID-Hash', filtered) inject_text(self.mlist, filtered) items = get_queue_messages('in') - self.assertTrue('message-id' in items[0].msg) - self.assertTrue('x-message-id-hash' in items[0].msg) + self.assertIn('message-id', items[0].msg) + self.assertIn('message-id-hash', items[0].msg) diff --git a/src/mailman/archiving/docs/common.rst b/src/mailman/archiving/docs/common.rst index e44247bbd..92c994751 100644 --- a/src/mailman/archiving/docs/common.rst +++ b/src/mailman/archiving/docs/common.rst @@ -121,6 +121,7 @@ Additionally, this archiver can handle malformed ``Message-IDs``. >>> del msg['x-message-id-hash'] >>> msg['Message-ID'] = '12345>' >>> add_message_hash(msg) + 'YJIGBYRWZFG5LZEBQ7NR25B5HBR2BVD6' >>> print(archiver.permalink(mlist, msg)) http://go.mail-archive.dev/YJIGBYRWZFG5LZEBQ7NR25B5HBR2BVD6 @@ -128,6 +129,7 @@ Additionally, this archiver can handle malformed ``Message-IDs``. >>> del msg['x-message-id-hash'] >>> msg['Message-ID'] = '<12345' >>> add_message_hash(msg) + 'XUFFJNJ2P2WC4NDPQRZFDJMV24POP64B' >>> print(archiver.permalink(mlist, msg)) http://go.mail-archive.dev/XUFFJNJ2P2WC4NDPQRZFDJMV24POP64B @@ -135,6 +137,7 @@ Additionally, this archiver can handle malformed ``Message-IDs``. >>> del msg['x-message-id-hash'] >>> msg['Message-ID'] = '12345' >>> add_message_hash(msg) + 'RSZCG7IGPHFIRW3EMTVMMDNJMNCVCOLE' >>> print(archiver.permalink(mlist, msg)) http://go.mail-archive.dev/RSZCG7IGPHFIRW3EMTVMMDNJMNCVCOLE @@ -143,6 +146,7 @@ Additionally, this archiver can handle malformed ``Message-IDs``. >>> add_message_hash(msg) >>> msg['Message-ID'] = ' 12345 ' >>> add_message_hash(msg) + 'RSZCG7IGPHFIRW3EMTVMMDNJMNCVCOLE' >>> print(archiver.permalink(mlist, msg)) http://go.mail-archive.dev/RSZCG7IGPHFIRW3EMTVMMDNJMNCVCOLE diff --git a/src/mailman/archiving/mailarchive.py b/src/mailman/archiving/mailarchive.py index c5dd8c8e5..06b5c05c9 100644 --- a/src/mailman/archiving/mailarchive.py +++ b/src/mailman/archiving/mailarchive.py @@ -57,10 +57,13 @@ class MailArchive: """See `IArchiver`.""" if mlist.archive_policy is not ArchivePolicy.public: return None - # It is the LMTP server's responsibility to ensure that the message - # has a X-Message-ID-Hash header. If it doesn't then there's no - # permalink. - message_id_hash = msg.get('x-message-id-hash') + # It is the LMTP server's responsibility to ensure that the message has + # a Message-ID-Hash header. For backward compatibility, fallback to + # searching for X-Message-ID-Hash. If the message has neither, then + # there's no permalink. + message_id_hash = msg.get('message-id-hash') + if message_id_hash is None: + message_id_hash = msg.get('x-message-id-hash') if message_id_hash is None: return None if isinstance(message_id_hash, bytes): diff --git a/src/mailman/archiving/mhonarc.py b/src/mailman/archiving/mhonarc.py index 8d19c6f64..1c351748e 100644 --- a/src/mailman/archiving/mhonarc.py +++ b/src/mailman/archiving/mhonarc.py @@ -63,10 +63,14 @@ class MHonArc: def permalink(self, mlist, msg): """See `IArchiver`.""" # XXX What about private MHonArc archives? - # It is the LMTP server's responsibility to ensure that the message - # has a X-Message-ID-Hash header. If it doesn't then there's no + # + # It is the LMTP server's responsibility to ensure that the message has + # a Message-ID-Hash header. For backward compatibility, fall back to + # X-Message-ID-Hash. If the message has neither, then there's no # permalink. - message_id_hash = msg.get('x-message-id-hash') + message_id_hash = msg.get('message-id-hash') + if message_id_hash is None: + message_id_hash = msg.get('x-message-id-hash') if message_id_hash is None: return None if isinstance(message_id_hash, bytes): diff --git a/src/mailman/archiving/prototype.py b/src/mailman/archiving/prototype.py index b5df11f78..73c296b5d 100644 --- a/src/mailman/archiving/prototype.py +++ b/src/mailman/archiving/prototype.py @@ -58,10 +58,13 @@ class Prototype: @staticmethod def permalink(mlist, msg): """See `IArchiver`.""" - # It is the LMTP server's responsibility to ensure that the message - # has a X-Message-ID-Hash header. If it doesn't then there's no + # It is the LMTP server's responsibility to ensure that the message has + # a Message-ID-Hash header. For backward compatibility, fall back to + # X-Message-ID-Hash. If the message has neither, then there's no # permalink. - message_id_hash = msg.get('x-message-id-hash') + message_id_hash = msg.get('message-id-hash') + if message_id_hash is None: + message_id_hash = msg.get('x-message-id-hash') if message_id_hash is None: return None if isinstance(message_id_hash, bytes): diff --git a/src/mailman/archiving/tests/test_prototype.py b/src/mailman/archiving/tests/test_prototype.py index 1cc4f6ee2..797f65584 100644 --- a/src/mailman/archiving/tests/test_prototype.py +++ b/src/mailman/archiving/tests/test_prototype.py @@ -53,7 +53,7 @@ To: test@example.com From: anne@example.com Subject: Testing the test list Message-ID: <ant> -X-Message-ID-Hash: MS6QLWERIJLGCRF44J7USBFDELMNT2BW +Message-ID-Hash: MS6QLWERIJLGCRF44J7USBFDELMNT2BW Tests are better than no tests but the water deserves to be swum. @@ -125,7 +125,7 @@ but the water deserves to be swum. # Archive a second message. If an exception occurs, let it fail the # test. Afterward, two messages should be in the 'new' directory. del self._msg['message-id'] - del self._msg['x-message-id-hash'] + del self._msg['message-id-hash'] self._msg['Message-ID'] = '<bee>' add_message_hash(self._msg) Prototype.archive_message(self._mlist, self._msg) diff --git a/src/mailman/commands/docs/inject.rst b/src/mailman/commands/docs/inject.rst index 68a5d534d..163f62886 100644 --- a/src/mailman/commands/docs/inject.rst +++ b/src/mailman/commands/docs/inject.rst @@ -95,7 +95,7 @@ By default, the incoming queue is used. >>> dump_msgdata(items[0].msgdata) _parsemsg : False listid : test.example.com - original_size: 203 + original_size: 253 version : 3 But a different queue can be specified on the command line. @@ -123,7 +123,7 @@ But a different queue can be specified on the command line. >>> dump_msgdata(items[0].msgdata) _parsemsg : False listid : test.example.com - original_size: 203 + original_size: 253 version : 3 @@ -167,7 +167,7 @@ The message text can also be provided on standard input. >>> dump_msgdata(items[0].msgdata) _parsemsg : False listid : test.example.com - original_size: 211 + original_size: 261 version : 3 .. Clean up. @@ -195,7 +195,7 @@ injected. bar : two foo : one listid : test.example.com - original_size: 203 + original_size: 253 version : 3 diff --git a/src/mailman/core/docs/chains.rst b/src/mailman/core/docs/chains.rst index 34f3e5c4e..0769df7f6 100644 --- a/src/mailman/core/docs/chains.rst +++ b/src/mailman/core/docs/chains.rst @@ -147,7 +147,8 @@ This one is addressed to the list moderators. To: test@example.com Subject: My first post Message-ID: <first> - X-Message-ID-Hash: RXJU4JL6N2OUN3OYMXXPPSCR7P7JE2BW + Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB + X-Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB <BLANKLINE> An important message. <BLANKLINE> @@ -219,7 +220,8 @@ processed and sent on to the list membership. To: test@example.com Subject: My first post Message-ID: <first> - X-Message-ID-Hash: RXJU4JL6N2OUN3OYMXXPPSCR7P7JE2BW + Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB + X-Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB <BLANKLINE> An important message. <BLANKLINE> @@ -264,7 +266,8 @@ This message will end up in the `pipeline` queue. To: test@example.com Subject: My first post Message-ID: <first> - X-Message-ID-Hash: RXJU4JL6N2OUN3OYMXXPPSCR7P7JE2BW + Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB + X-Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB X-Mailman-Rule-Misses: approved; emergency; loop; member-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header; nonmember-moderation diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 0fd37ed00..5071e24bd 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -36,6 +36,10 @@ Interfaces Given by Aurélien Bompard, tweaked by Barry Warsaw. * The default `postauth.txt` and `postheld.txt` templates now no longer include the inaccurate admindb and confirmation urls. + * 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. (Closes #118) Internal API ------------ diff --git a/src/mailman/interfaces/messages.py b/src/mailman/interfaces/messages.py index 83418a692..9617a879a 100644 --- a/src/mailman/interfaces/messages.py +++ b/src/mailman/interfaces/messages.py @@ -30,42 +30,42 @@ from zope.interface import Interface, Attribute class IMessageStore(Interface): """The interface of the global message storage service. - All messages that are stored in the system live in the message storage - service. A message stored in this service must have a Message-ID header. - The store writes an X-Message-ID-Hash header which contains the Base32 - encoded SHA1 hash of the message's Message-ID header. Any existing - X-Message-ID-Hash header is overwritten. + All messages that are stored in the system live in the message + storage service. A message stored in this service must have a + Message-ID header. The store writes an Message-ID-Hash header which + contains the Base32 encoded SHA1 hash of the message's Message-ID + header. Any existing Message-ID-Hash header is overwritten. - Either the Message-ID or the X-Message-ID-Hash header can be used to + Either the Message-ID or the Message-ID-Hash header can be used to uniquely identify this message in the storage service. While it is possible to see duplicate Message-IDs, this is never correct and the - service is allowed to drop any subsequent colliding messages, or overwrite - earlier messages with later ones. + service is allowed to drop any subsequent colliding messages, or + overwrite earlier messages with later ones. - The combination of the List-Archive header and either the Message-ID or - X-Message-ID-Hash header can be used to retrieve the message from the - internet facing interface for the message store. This can be considered a - globally unique URI to the message. + The combination of the List-Archive header and either the Message-ID + or Message-ID-Hash header can be used to retrieve the message from + the internet facing interface for the message store. This can be + considered a globally unique URI to the message. For example, a message with the following headers: Message-ID: <87myycy5eh.fsf@uwakimon.sk.tsukuba.ac.jp> Date: Wed, 04 Jul 2007 16:49:58 +0900 List-Archive: http://archive.example.com/ - X-Message-ID-Hash: RXTJ357KFOTJP3NFJA6KMO65X7VQOHJI + Message-ID-Hash: RXTJ357KFOTJP3NFJA6KMO65X7VQOHJI the globally unique URI would be: http://archive.example.com/RXTJ357KFOTJP3NFJA6KMO65X7VQOHJI - """ +""" def add(message): """Add the message to the store. :param message: An email.message.Message instance containing at least a unique Message-ID header. The message will be given an - X-Message-ID-Hash header, overriding any existing such header. - :returns: The calculated X-Message-ID-Hash header. + Message-ID-Hash header, overriding any existing such header. + :returns: The calculated Message-ID-Hash header. :raises ValueError: if the message is missing a Message-ID header. The storage service is also allowed to raise this exception if it find, but disallows collisions. @@ -79,9 +79,9 @@ class IMessageStore(Interface): """ def get_message_by_hash(message_id_hash): - """Return the message with the matching X-Message-ID-Hash. + """Return the message with the matching Message-ID-Hash. - :param message_id_hash: The X-Message-ID-Hash header contents to + :param message_id_hash: The Message-ID-Hash header contents to search for. :returns: The message, or None if no matching message was found. """ diff --git a/src/mailman/model/docs/messagestore.rst b/src/mailman/model/docs/messagestore.rst index 933ca5619..94ca0c0dc 100644 --- a/src/mailman/model/docs/messagestore.rst +++ b/src/mailman/model/docs/messagestore.rst @@ -22,11 +22,12 @@ A message with a ``Message-ID`` header can be stored. ... """) >>> x_message_id_hash = message_store.add(msg) >>> print(x_message_id_hash) - AGDWSNXXKCWEILKKNYTBOHRDQGOX3Y35 + JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP >>> print(msg.as_string()) Subject: An important message Message-ID: <87myycy5eh.fsf@uwakimon.sk.tsukuba.ac.jp> - X-Message-ID-Hash: AGDWSNXXKCWEILKKNYTBOHRDQGOX3Y35 + Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP + X-Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP <BLANKLINE> This message is very important. <BLANKLINE> @@ -50,7 +51,8 @@ Given an existing ``Message-ID``, the message can be found. >>> print(message.as_string()) Subject: An important message Message-ID: <87myycy5eh.fsf@uwakimon.sk.tsukuba.ac.jp> - X-Message-ID-Hash: AGDWSNXXKCWEILKKNYTBOHRDQGOX3Y35 + Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP + X-Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP <BLANKLINE> This message is very important. <BLANKLINE> @@ -61,7 +63,8 @@ Similarly, we can find messages by the ``X-Message-ID-Hash``: >>> print(message.as_string()) Subject: An important message Message-ID: <87myycy5eh.fsf@uwakimon.sk.tsukuba.ac.jp> - X-Message-ID-Hash: AGDWSNXXKCWEILKKNYTBOHRDQGOX3Y35 + Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP + X-Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP <BLANKLINE> This message is very important. <BLANKLINE> @@ -79,7 +82,8 @@ contains. >>> print(messages[0].as_string()) Subject: An important message Message-ID: <87myycy5eh.fsf@uwakimon.sk.tsukuba.ac.jp> - X-Message-ID-Hash: AGDWSNXXKCWEILKKNYTBOHRDQGOX3Y35 + Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP + X-Message-ID-Hash: JJIGKPKB6CVDX6B2CUG4IHAJRIQIOUTP <BLANKLINE> This message is very important. <BLANKLINE> diff --git a/src/mailman/model/messagestore.py b/src/mailman/model/messagestore.py index 7c582406b..d5b6eb1e3 100644 --- a/src/mailman/model/messagestore.py +++ b/src/mailman/model/messagestore.py @@ -24,14 +24,13 @@ __all__ = [ import os import errno -import base64 import pickle -import hashlib from mailman.config import config from mailman.database.transaction import dbconnection from mailman.interfaces.messages import IMessageStore from mailman.model.message import Message +from mailman.utilities.email import add_message_hash from mailman.utilities.filesystem import makedirs from zope.interface import implementer @@ -53,7 +52,7 @@ class MessageStore: message_ids = message.get_all('message-id', []) if len(message_ids) != 1: raise ValueError('Exactly one Message-ID header required') - # Calculate and insert the X-Message-ID-Hash. + # Calculate and insert the Message-ID-Hash. message_id = message_ids[0] if isinstance(message_id, bytes): message_id = message_id.decode('ascii') @@ -64,10 +63,7 @@ class MessageStore: raise ValueError( 'Message ID already exists in message store: {0}'.format( message_id)) - shaobj = hashlib.sha1(message_id.encode('utf-8')) - hash32 = base64.b32encode(shaobj.digest()).decode('utf-8') - del message['X-Message-ID-Hash'] - message['X-Message-ID-Hash'] = hash32 + hash32 = add_message_hash(message) # Calculate the path on disk where we're going to store this message # object, in pickled format. parts = [] diff --git a/src/mailman/model/tests/test_messagestore.py b/src/mailman/model/tests/test_messagestore.py index e7e533e00..337822cf7 100644 --- a/src/mailman/model/tests/test_messagestore.py +++ b/src/mailman/model/tests/test_messagestore.py @@ -60,12 +60,49 @@ This message is very important. add_message_hash(message) self._store.add(message) self.assertEqual(message['x-message-id-hash'], - 'V3YEHAFKE2WVJNK63Z7RFP4JMHISI2RG') + 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') found = self._store.get_message_by_hash( - 'V3YEHAFKE2WVJNK63Z7RFP4JMHISI2RG') + 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') self.assertEqual(found['message-id'], '<ant>') self.assertEqual(found['x-message-id-hash'], - 'V3YEHAFKE2WVJNK63Z7RFP4JMHISI2RG') + 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') def test_cannot_delete_missing_message(self): self.assertRaises(LookupError, self._store.delete_message, 'missing') + + def test_message_id_hash(self): + # The new specification calls for a Message-ID-Hash header, + # specifically without the X- prefix. + 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(self): + # Any existing Message-ID-Hash header (or for backward compatibility + # X-Message-ID-Hash) gets replaced with its new value. + msg = mfs("""\ +Subject: Testing +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') diff --git a/src/mailman/rest/docs/post-moderation.rst b/src/mailman/rest/docs/post-moderation.rst index 6dd96e71a..671f36e73 100644 --- a/src/mailman/rest/docs/post-moderation.rst +++ b/src/mailman/rest/docs/post-moderation.rst @@ -45,7 +45,8 @@ When a message gets held for moderator approval, it shows up in this list. To: ant@example.com Subject: Something Message-ID: <alpha> - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + Message-ID-Hash: XZ3DGG4V37BZTTLXNUX4NABB4DNQHTCP + X-Message-ID-Hash: XZ3DGG4V37BZTTLXNUX4NABB4DNQHTCP <BLANKLINE> Something else. <BLANKLINE> @@ -74,7 +75,8 @@ message. This will include the text of the message. To: ant@example.com Subject: Something Message-ID: <alpha> - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + Message-ID-Hash: XZ3DGG4V37BZTTLXNUX4NABB4DNQHTCP + X-Message-ID-Hash: XZ3DGG4V37BZTTLXNUX4NABB4DNQHTCP <BLANKLINE> Something else. <BLANKLINE> @@ -117,7 +119,8 @@ The message is still in the moderation queue. To: ant@example.com Subject: Something Message-ID: <alpha> - X-Message-ID-Hash: GCSMSG43GYWWVUMO6F7FBUSSPNXQCJ6M + Message-ID-Hash: XZ3DGG4V37BZTTLXNUX4NABB4DNQHTCP + X-Message-ID-Hash: XZ3DGG4V37BZTTLXNUX4NABB4DNQHTCP <BLANKLINE> Something else. <BLANKLINE> diff --git a/src/mailman/rest/tests/test_queues.py b/src/mailman/rest/tests/test_queues.py index cc860112b..df0ecc58b 100644 --- a/src/mailman/rest/tests/test_queues.py +++ b/src/mailman/rest/tests/test_queues.py @@ -89,8 +89,9 @@ class TestQueues(unittest.TestCase): msg = items[0].msg # Remove some headers that get added by Mailman. del msg['date'] - self.assertEqual(msg['x-message-id-hash'], + self.assertEqual(msg['message-id-hash'], 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') + del msg['message-id-hash'] del msg['x-message-id-hash'] self.assertMultiLineEqual(msg.as_string(), TEXT) diff --git a/src/mailman/runners/docs/incoming.rst b/src/mailman/runners/docs/incoming.rst index fa425980b..03ed58df4 100644 --- a/src/mailman/runners/docs/incoming.rst +++ b/src/mailman/runners/docs/incoming.rst @@ -125,6 +125,7 @@ Now the message is in the pipeline queue. To: test@example.com Subject: My first post Message-ID: <first> + Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB X-Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB Date: ... X-Mailman-Rule-Misses: approved; emergency; loop; member-moderation; diff --git a/src/mailman/runners/docs/lmtp.rst b/src/mailman/runners/docs/lmtp.rst index 45e8a3453..6a21295b9 100644 --- a/src/mailman/runners/docs/lmtp.rst +++ b/src/mailman/runners/docs/lmtp.rst @@ -60,6 +60,7 @@ queue. To: mylist@example.com Subject: An interesting message Message-ID: <badger> + Message-ID-Hash: JYMZWSQ4IC2JPKK7ZUONRFRVC4ZYJGKJ X-Message-ID-Hash: JYMZWSQ4IC2JPKK7ZUONRFRVC4ZYJGKJ X-MailFrom: anne.person@example.com <BLANKLINE> @@ -107,6 +108,7 @@ command queue for processing. To: mylist-request@example.com Subject: Help Message-ID: <dog> + Message-ID-Hash: 4SKREUSPI62BHDMFBSOZ3BMXFETSQHNA X-Message-ID-Hash: 4SKREUSPI62BHDMFBSOZ3BMXFETSQHNA X-MailFrom: anne.person@example.com <BLANKLINE> diff --git a/src/mailman/runners/tests/test_archiver.py b/src/mailman/runners/tests/test_archiver.py index 79ea6d2ea..30111a6a3 100644 --- a/src/mailman/runners/tests/test_archiver.py +++ b/src/mailman/runners/tests/test_archiver.py @@ -50,12 +50,12 @@ class DummyArchiver: @staticmethod def permalink(mlist, msg): - filename = msg['x-message-id-hash'] + filename = msg['message-id-hash'] return 'http://archive.example.com/' + filename @staticmethod def archive_message(mlist, msg): - filename = msg['x-message-id-hash'] + filename = msg['message-id-hash'] path = os.path.join(config.MESSAGES_DIR, filename) with open(path, 'w') as fp: print(msg.as_string(), file=fp) @@ -90,7 +90,7 @@ From: aperson@example.com To: test@example.com Subject: My first post Message-ID: <first> -X-Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB +Message-ID-Hash: 4CMWUN6BHVCMHMDAOSJZ2Q72G5M32MWB First post! """) diff --git a/src/mailman/runners/tests/test_lmtp.py b/src/mailman/runners/tests/test_lmtp.py index 306b6d5e5..08db8aa8f 100644 --- a/src/mailman/runners/tests/test_lmtp.py +++ b/src/mailman/runners/tests/test_lmtp.py @@ -70,12 +70,12 @@ Subject: This has no Message-ID header From: anne@example.com To: test@example.com Message-ID: <ant> -Subject: This has a Message-ID but no X-Message-ID-Hash +Subject: This has a Message-ID but no Message-ID-Hash """) messages = get_queue_messages('in') self.assertEqual(len(messages), 1) - self.assertEqual(messages[0].msg['x-message-id-hash'], + self.assertEqual(messages[0].msg['message-id-hash'], 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') def test_original_message_id_hash_is_overwritten(self): @@ -83,15 +83,15 @@ Subject: This has a Message-ID but no X-Message-ID-Hash From: anne@example.com To: test@example.com Message-ID: <ant> -X-Message-ID-Hash: IGNOREME -Subject: This has a Message-ID but no X-Message-ID-Hash +Message-ID-Hash: IGNOREME +Subject: This has a Message-ID but no Message-ID-Hash """) messages = get_queue_messages('in') self.assertEqual(len(messages), 1) - all_headers = messages[0].msg.get_all('x-message-id-hash') + all_headers = messages[0].msg.get_all('message-id-hash') self.assertEqual(len(all_headers), 1) - self.assertEqual(messages[0].msg['x-message-id-hash'], + self.assertEqual(messages[0].msg['message-id-hash'], 'MS6QLWERIJLGCRF44J7USBFDELMNT2BW') def test_received_time(self): 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') |