4 files changed, 128 insertions, 42 deletions

diff --git a/src/mailman/runners/nntp.py b/src/mailman/runners/nntp.py
index 411fbb6f2..11c3942f0 100644
--- a/src/mailman/runners/nntp.py
+++ b/src/mailman/runners/nntp.py
@@ -157,18 +157,26 @@ def prepare_message(mlist, msg, msgdata):
     # reject the message because of other problems.
     for header in config.nntp.remove_headers.split():
         del msg[header]
-    for rewrite_pairs in config.nntp.rewrite_duplicate_headers.splitlines():
-        if len(rewrite_pairs.strip()) == 0:
-            continue
-        header, rewrite = rewrite_pairs.split()
-        values = msg.get_all(header, [])
+    dup_headers = config.nntp.rewrite_duplicate_headers.split()
+    if len(dup_headers) % 2 != 0:
+        # There are an odd number of headers; ignore the last one.
+        bad_header = dup_headers.pop()
+        log.error('Ignoring odd [nntp]rewrite_duplicate_headers: {0}'.format(
+            bad_header))
+    dup_headers.reverse()
+    while dup_headers:
+        source = dup_headers.pop()
+        target = dup_headers.pop()
+        values = msg.get_all(source, [])
         if len(values) < 2:
-            # We only care about duplicates
+            # We only care about duplicates.
             continue
-        del msg[header]
-        # But keep the first one...
-        msg[header] = values[0]
-        for v in values[1:]:
-            msg[rewrite] = v
+        # Delete all the original headers.
+        del msg[source]
+        # Put the first value back on the original header.
+        msg[source] = values[0]
+        # And put all the subsequent values on the destination header.
+        for value in values[1:]:
+            msg[target] = value
     # Mark this message as prepared in case it has to be requeued.
     msgdata['prepped'] = True
diff --git a/src/mailman/runners/tests/test_archiver.py b/src/mailman/runners/tests/test_archiver.py
index ca09de9fa..6f5804cae 100644
--- a/src/mailman/runners/tests/test_archiver.py
+++ b/src/mailman/runners/tests/test_archiver.py
@@ -36,6 +36,7 @@ from mailman.config import config
 from mailman.interfaces.archiver import IArchiver
 from mailman.runners.archive import ArchiveRunner
 from mailman.testing.helpers import (
+    configuration,
     make_testable_runner,
     specialized_message_from_string as mfs)
 from mailman.testing.layers import ConfigLayer
@@ -43,30 +44,6 @@ from mailman.utilities.datetime import RFC822_DATE_FMT, factory, now
 
 
 
-# This helper will set up a specific archiver as appropriate for a specific
-# test.  It assumes the setUp() will just disable all archivers.
-def archiver(name, enable=False, clobber=None, skew=None):
-    def decorator(func):
-        def wrapper(*args, **kws):
-            config_name = 'archiver {0}'.format(name)
-            section = """
-            [archiver.{0}]
-            enable: {1}
-            clobber_date: {2}
-            clobber_skew: {3}
-            """.format(name,
-                       'yes' if enable else 'no',
-                       clobber, skew)
-            config.push(config_name, section)
-            try:
-                return func(*args, **kws)
-            finally:
-                config.pop(config_name)
-        return wrapper
-    return decorator
-
-
-
 class DummyArchiver:
     implements(IArchiver)
     name = 'dummy'
@@ -126,7 +103,7 @@ First post!
     def tearDown(self):
         config.pop('dummy')
 
-    @archiver('dummy', enable=True)
+    @configuration('archiver.dummy', enable='yes')
     def test_archive_runner(self):
         # Ensure that the archive runner ends up archiving the message.
         self._archiveq.enqueue(
@@ -141,7 +118,7 @@ First post!
             archived = message_from_file(fp)
         self.assertEqual(archived['message-id'], '<first>')
 
-    @archiver('dummy', enable=True)
+    @configuration('archiver.dummy', enable='yes')
     def test_archive_runner_with_dated_message(self):
         # Date headers don't throw off the archiver runner.
         self._msg['Date'] = now(strip_tzinfo=False).strftime(RFC822_DATE_FMT)
@@ -158,7 +135,7 @@ First post!
         self.assertEqual(archived['message-id'], '<first>')
         self.assertEqual(archived['date'], 'Mon, 01 Aug 2005 07:49:23 +0000')
 
-    @archiver('dummy', enable=True, clobber='never')
+    @configuration('archiver.dummy', enable='yes', clobber_date='never')
     def test_clobber_date_never(self):
         # Even if the Date header is insanely off from the received time of
         # the message, if clobber_date is 'never', the header is not clobbered.
@@ -176,7 +153,7 @@ First post!
         self.assertEqual(archived['message-id'], '<first>')
         self.assertEqual(archived['date'], 'Mon, 01 Aug 2005 07:49:23 +0000')
 
-    @archiver('dummy', enable=True)
+    @configuration('archiver.dummy', enable='yes')
     def test_clobber_dateless(self):
         # A message with no Date header will always get clobbered.
         self.assertEqual(self._msg['date'], None)
@@ -195,7 +172,7 @@ First post!
         self.assertEqual(archived['message-id'], '<first>')
         self.assertEqual(archived['date'], 'Mon, 01 Aug 2005 07:49:23 +0000')
 
-    @archiver('dummy', enable=True, clobber='always')
+    @configuration('archiver.dummy', enable='yes', clobber_date='always')
     def test_clobber_date_always(self):
         # The date always gets clobbered with the current received time.
         self._msg['Date'] = now(strip_tzinfo=False).strftime(RFC822_DATE_FMT)
@@ -216,7 +193,8 @@ First post!
         self.assertEqual(archived['x-original-date'],
                          'Mon, 01 Aug 2005 07:49:23 +0000')
 
-    @archiver('dummy', enable=True, clobber='maybe', skew='1d')
+    @configuration('archiver.dummy', 
+                   enable='yes', clobber_date='maybe', clobber_skew='1d')
     def test_clobber_date_maybe_when_insane(self):
         # The date is clobbered if it's farther off from now than its skew
         # period.
@@ -238,7 +216,8 @@ First post!
         self.assertEqual(archived['x-original-date'],
                          'Mon, 01 Aug 2005 07:49:23 +0000')
 
-    @archiver('dummy', enable=True, clobber='maybe', skew='10d')
+    @configuration('archiver.dummy',
+                   enable='yes', clobber_date='maybe', clobber_skew='10d')
     def test_clobber_date_maybe_when_sane(self):
         # The date is not clobbered if it's nearer to now than its skew
         # period.
diff --git a/src/mailman/runners/tests/test_nntp.py b/src/mailman/runners/tests/test_nntp.py
index 33a2f9a7c..dcd3cfac4 100644
--- a/src/mailman/runners/tests/test_nntp.py
+++ b/src/mailman/runners/tests/test_nntp.py
@@ -31,6 +31,7 @@ from mailman.app.lifecycle import create_list
 from mailman.interfaces.nntp import NewsModeration
 from mailman.runners import nntp
 from mailman.testing.helpers import (
+    configuration,
     specialized_message_from_string as mfs)
 from mailman.testing.layers import ConfigLayer
 
@@ -163,6 +164,63 @@ Testing
         self.assertEqual(self._msg['lines'], '1')
 
     def test_the_message_has_been_prepared(self):
+        # A key gets added to the metadata so that a retry won't try to
+        # re-apply all the preparations.
         msgdata = {}
         nntp.prepare_message(self._mlist, self._msg, msgdata)
         self.assertTrue(msgdata.get('prepped'))
+
+    @configuration('nntp', remove_headers='x-complaints-to')
+    def test_remove_headers(self):
+        # During preparation, headers which cause problems with certain NNTP
+        # servers such as INN get removed.
+        self._msg['X-Complaints-To'] = 'arguments@example.com'
+        nntp.prepare_message(self._mlist, self._msg, {})
+        self.assertEqual(self._msg['x-complaints-to'], None)
+
+    @configuration('nntp', rewrite_duplicate_headers="""
+        To X-Original-To
+        X-Fake X-Original-Fake
+        """)
+    def test_rewrite_headers(self):
+        # Some NNTP servers are very strict about duplicate headers.  What we
+        # can do is look at some headers and if they is more than one of that
+        # header in the message, all the headers are deleted except the first
+        # one, and then the other values are moved to the destination header.
+        #
+        # In this example, we'll create multiple To headers, which will all
+        # get moved to X-Original-To.  However, because there will only be one
+        # X-Fake header, it doesn't get rewritten.
+        self._msg['To'] = 'test@example.org'
+        self._msg['To'] = 'test@example.net'
+        self._msg['X-Fake'] = 'ignore me'
+        self.assertEqual(len(self._msg.get_all('to')), 3)
+        self.assertEqual(len(self._msg.get_all('x-fake')), 1)
+        nntp.prepare_message(self._mlist, self._msg, {})
+        tos = self._msg.get_all('to')
+        self.assertEqual(len(tos), 1)
+        self.assertEqual(tos[0], 'test@example.com')
+        original_tos = self._msg.get_all('x-original-to')
+        self.assertEqual(len(original_tos), 2)
+        self.assertEqual(original_tos, 
+                         ['test@example.org', 'test@example.net'])
+        fakes = self._msg.get_all('x-fake')
+        self.assertEqual(len(fakes), 1)
+        self.assertEqual(fakes[0], 'ignore me')
+        self.assertEqual(self._msg.get_all('x-original-fake'), None)
+
+    @configuration('nntp', rewrite_duplicate_headers="""
+        To X-Original-To
+        X-Fake
+        """)
+    def test_odd_duplicates(self):
+        # This is just a corner case, where there is an odd number of rewrite
+        # headers.  In that case, the odd-one-out does not get rewritten.
+        self._msg['x-fake'] = 'one'
+        self._msg['x-fake'] = 'two'
+        self._msg['x-fake'] = 'three'
+        self.assertEqual(len(self._msg.get_all('x-fake')), 3)
+        nntp.prepare_message(self._mlist, self._msg, {})
+        fakes = self._msg.get_all('x-fake')
+        self.assertEqual(len(fakes), 3)
+        self.assertEqual(fakes, ['one', 'two', 'three'])
diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py
index 032b028a9..ca0b14b18 100644
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -25,6 +25,7 @@ __all__ = [
     'TestableMaster',
     'body_line_iterator',
     'call_api',
+    'configuration',
     'digest_mbox',
     'event_subscribers',
     'get_lmtp_client',
@@ -40,6 +41,7 @@ __all__ = [
 import os
 import json
 import time
+import uuid
 import errno
 import signal
 import socket
@@ -68,6 +70,9 @@ from mailman.interfaces.usermanager import IUserManager
 from mailman.utilities.mailbox import Mailbox
 
 
+NL = '\n'
+
+
 
 def make_testable_runner(runner_class, name=None, predicate=None):
     """Create a runner that runs until its queue is empty.
@@ -331,6 +336,42 @@ def event_subscribers(*subscribers):
 
 
 
+class configuration:
+    """A decorator/context manager for temporarily setting configurations."""
+
+    def __init__(self, section, **kws):
+        self._section = section
+        self._values = kws.copy()
+        self._uuid = uuid.uuid4().hex
+
+    def _apply(self):
+        lines = ['[{0}]'.format(self._section)]
+        for key, value in self._values.items():
+            lines.append('{0}: {1}'.format(key, value))
+        config.push(self._uuid, NL.join(lines))
+
+    def _remove(self):
+        config.pop(self._uuid)
+
+    def __enter__(self):
+        self._apply()
+
+    def __exit__(self, *exc_info):
+        self._remove()
+        # Do not suppress exceptions.
+        return False
+
+    def __call__(self, func):
+        def wrapper(*args, **kws):
+            self._apply()
+            try:
+                return func(*args, **kws)
+            finally:
+                self._remove()
+        return wrapper
+
+
+
 def subscribe(mlist, first_name, role=MemberRole.member):
     """Helper for subscribing a sample person to a mailing list."""
     user_manager = getUtility(IUserManager)