16 files changed, 577 insertions, 155 deletions

diff --git a/src/mailman/app/docs/pipelines.rst b/src/mailman/app/docs/pipelines.rst
index 56fdeb5c6..96d9d232a 100644
--- a/src/mailman/app/docs/pipelines.rst
+++ b/src/mailman/app/docs/pipelines.rst
@@ -60,7 +60,7 @@ However there are currently no recipients for this message.
 
     >>> dump_msgdata(msgdata)
     original_sender : aperson@example.com
-    origsubj        : My first post
+    original_subject: My first post
     recipients      : set([])
     stripped_subject: My first post
 
@@ -90,7 +90,7 @@ processing queues.
     >>> dump_msgdata(messages[0].msgdata)
     _parsemsg       : False
     original_sender : aperson@example.com
-    origsubj        : My first post
+    original_subject: My first post
     recipients      : set([])
     stripped_subject: My first post
     version         : 3
@@ -98,7 +98,7 @@ processing queues.
 This mailing list is not linked to an NNTP newsgroup, so there's nothing in
 the outgoing nntp queue.
 
-    >>> messages = get_queue_messages('news')
+    >>> messages = get_queue_messages('nntp')
     >>> len(messages)
     0
 
@@ -128,7 +128,7 @@ delivered to end recipients.
     _parsemsg       : False
     listname        : test@example.com
     original_sender : aperson@example.com
-    origsubj        : My first post
+    original_subject: My first post
     recipients      : set([])
     stripped_subject: My first post
     version         : 3
diff --git a/src/mailman/commands/docs/inject.rst b/src/mailman/commands/docs/inject.rst
index 7150beac7..e8987128c 100644
--- a/src/mailman/commands/docs/inject.rst
+++ b/src/mailman/commands/docs/inject.rst
@@ -35,7 +35,7 @@ It's easy to find out which queues are available.
         digest
         in
         lmtp
-        news
+        nntp
         out
         pipeline
         rest
diff --git a/src/mailman/config/mailman.cfg b/src/mailman/config/mailman.cfg
index 0d37ceed9..10ef0ba17 100644
--- a/src/mailman/config/mailman.cfg
+++ b/src/mailman/config/mailman.cfg
@@ -61,8 +61,8 @@ class: mailman.runners.incoming.IncomingRunner
 [runner.lmtp]
 class: mailman.runners.lmtp.LMTPRunner
 
-[runner.news]
-class: mailman.runners.news.NewsRunner
+[runner.nntp]
+class: mailman.runners.nntp.NNTPRunner
 
 [runner.out]
 class: mailman.runners.outgoing.OutgoingRunner
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index 88f378159..44cbf6f4e 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -601,13 +601,15 @@ plain_digest_keep_headers:
 
 [nntp]
 # Set these variables if you need to authenticate to your NNTP server for
-# Usenet posting or reading.  If no authentication is necessary, specify None
-# for both variables.
-username:
+# Usenet posting or reading.  Leave these blank if no authentication is
+# necessary.
+user:
 password:
 
-# Set this if you have an NNTP server you prefer gatewayed lists to use.
+# Host and port of the NNTP server to connect to.  Leave these blank to use
+# the default localhost:119.
 host:
+port:
 
 # This controls how headers must be cleansed in order to be accepted by your
 # NNTP server.  Some servers like INN reject messages containing prohibited
diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst
index 48736b518..13bb919c1 100644
--- a/src/mailman/docs/NEWS.rst
+++ b/src/mailman/docs/NEWS.rst
@@ -22,6 +22,9 @@ Architecture
  * The `ArchiveRunner` no longer acquires a lock before it calls the
    individual archiver implementations, since not all of them need a lock.  If
    they do, the implementations must acquire said lock themselves.
+ * The `news` runner and queue has been renamed to the more accurate `nntp`.
+   The runner has also been ported to Mailman 3 (LP: #967409).  Beta testers
+   can can safely remove `$var_dir/queue/news`.
 
 Configuration
 -------------
@@ -29,6 +32,9 @@ Configuration
    every `[archiver.<name>]` section.  These are used to determine under what
    circumstances a message destined for a specific archiver should have its
    `Date:` header clobbered.  (LP: #963612)
+ * Configuration schema variable changes:
+   [nntp]username -> [nntp]user
+   [nntp]port (added)
 
 Documentation
 -------------
diff --git a/src/mailman/handlers/cook_headers.py b/src/mailman/handlers/cook_headers.py
index 2d117429c..5d1e416a6 100644
--- a/src/mailman/handlers/cook_headers.py
+++ b/src/mailman/handlers/cook_headers.py
@@ -190,7 +190,7 @@ def prefix_subject(mlist, msg, msgdata):
     ws = '\t'
     if len(lines) > 1 and lines[1] and lines[1][0] in ' \t':
         ws = lines[1][0]
-    msgdata['origsubj'] = subject
+    msgdata['original_subject'] = subject
     # The subject may be multilingual but we take the first charset as major
     # one and try to decode.  If it is decodable, returned subject is in one
     # line and cset is properly set.  If fail, subject is mime-encoded and
diff --git a/src/mailman/handlers/docs/nntp.rst b/src/mailman/handlers/docs/nntp.rst
index 874712397..c298fcb14 100644
--- a/src/mailman/handlers/docs/nntp.rst
+++ b/src/mailman/handlers/docs/nntp.rst
@@ -3,15 +3,14 @@ NNTP Gateway
 ============
 
 Mailman has an NNTP gateway, whereby messages posted to the mailing list can
-be forwarded onto an NNTP newsgroup.  Typically this means Usenet, but since
-NNTP is to Usenet as IP is to the web, it's more general than that.
+be forwarded onto an NNTP newsgroup.
 
-    >>> mlist = create_list('_xtest@example.com')
+    >>> mlist = create_list('test@example.com')
 
 Gatewaying from the mailing list to the newsgroup happens through a separate
 ``nntp`` queue and happen immediately when the message is posted through to
 the list.  Note that gatewaying from the newsgroup to the list happens via a
-cronjob (currently not shown).
+separate process.
 
 There are several situations which prevent a message from being gatewayed to
 the newsgroup.  The feature could be disabled, as is the default.
@@ -26,43 +25,43 @@ the newsgroup.  The feature could be disabled, as is the default.
 
     >>> handler = config.handlers['to-usenet']
     >>> handler.process(mlist, msg, {})
-
-    >>> switchboard = config.switchboards['news']
-    >>> switchboard.files
+    >>> from mailman.testing.helpers import get_queue_messages
+    >>> get_queue_messages('nntp')
     []
 
 Even if enabled, messages that came from the newsgroup are never gated back to
 the newsgroup.
 
     >>> mlist.gateway_to_news = True
-    >>> handler.process(mlist, msg, {'fromusenet': True})
-    >>> switchboard.files
+    >>> handler.process(mlist, msg, dict(fromusenet=True))
+    >>> get_queue_messages('nntp')
     []
 
 Neither are digests ever gated to the newsgroup.
 
-    >>> handler.process(mlist, msg, {'isdigest': True})
-    >>> switchboard.files
+    >>> handler.process(mlist, msg, dict(isdigest=True))
+    >>> get_queue_messages('nntp')
     []
 
 However, other posted messages get gated to the newsgroup via the nntp queue.
 The list owner can set the linked newsgroup and the nntp host that its
 messages are gated to.
+::
 
     >>> mlist.linked_newsgroup = 'comp.lang.thing'
     >>> mlist.nntp_host = 'news.example.com'
     >>> handler.process(mlist, msg, {})
-    >>> len(switchboard.files)
+    >>> messages = get_queue_messages('nntp')
+    >>> len(messages)
     1
-    >>> filebase = switchboard.files[0]
-    >>> msg, msgdata = switchboard.dequeue(filebase)
-    >>> switchboard.finish(filebase)
-    >>> print msg.as_string()
+
+    >>> print messages[0].msg.as_string()
     Subject: An important message
     <BLANKLINE>
     Something of great import.
     <BLANKLINE>
-    >>> dump_msgdata(msgdata)
+
+    >>> dump_msgdata(messages[0].msgdata)
     _parsemsg: False
-    listname : _xtest@example.com
+    listname : test@example.com
     version  : 3
diff --git a/src/mailman/handlers/docs/subject-munging.rst b/src/mailman/handlers/docs/subject-munging.rst
index 48cee8e2b..f9e3b9abb 100644
--- a/src/mailman/handlers/docs/subject-munging.rst
+++ b/src/mailman/handlers/docs/subject-munging.rst
@@ -8,7 +8,7 @@ transformations.  Some headers get added, others get changed.  Some of these
 changes depend on mailing list settings and others depend on how the message
 is getting sent through the system.  We'll take things one-by-one.
 
-    >>> mlist = create_list('_xtest@example.com')
+    >>> mlist = create_list('test@example.com')
 
 
 Inserting a prefix
@@ -32,11 +32,9 @@ subject munging, a mailing list must have a preferred language.
     >>> from mailman.handlers.cook_headers import process
     >>> process(mlist, msg, msgdata)
 
-The original subject header is stored in the message metadata.  We must print
-the new ``Subject`` header because it gets converted from a string to an
-``email.header.Header`` instance which has an unhelpful ``repr``.
+The original subject header is stored in the message metadata.
 
-    >>> msgdata['origsubj']
+    >>> msgdata['original_subject']
     u''
     >>> print msg['subject']
     [XTest] (no subject)
@@ -52,7 +50,7 @@ at the beginning of the header's value.
     ... """)
     >>> msgdata = {}
     >>> process(mlist, msg, msgdata)
-    >>> print msgdata['origsubj']
+    >>> print msgdata['original_subject']
     Something important
     >>> print msg['subject']
     [XTest] Something important
diff --git a/src/mailman/handlers/to_usenet.py b/src/mailman/handlers/to_usenet.py
index 26a383c64..021f8f9e5 100644
--- a/src/mailman/handlers/to_usenet.py
+++ b/src/mailman/handlers/to_usenet.py
@@ -17,7 +17,7 @@
 
 """Move the message to the mail->news queue."""
 
-from __future__ import absolute_import, unicode_literals
+from __future__ import absolute_import, print_function, unicode_literals
 
 __metaclass__ = type
 __all__ = [
@@ -50,11 +50,12 @@ class ToUsenet:
     def process(self, mlist, msg, msgdata):
         """See `IHandler`."""
         # Short circuits.
-        if not mlist.gateway_to_news or \
-               msgdata.get('isdigest') or \
-               msgdata.get('fromusenet'):
+        if (not mlist.gateway_to_news or
+            msgdata.get('isdigest') or
+            msgdata.get('fromusenet')):
+            # Short-circuit.
             return
-        # sanity checks
+        # Sanity checks.
         error = []
         if not mlist.linked_newsgroup:
             error.append('no newsgroup')
@@ -65,5 +66,5 @@ class ToUsenet:
                       COMMASPACE.join(error))
             return
         # Put the message in the news runner's queue.
-        config.switchboards['news'].enqueue(
+        config.switchboards['nntp'].enqueue(
             msg, msgdata, listname=mlist.fqdn_listname)
diff --git a/src/mailman/interfaces/nntp.py b/src/mailman/interfaces/nntp.py
index 099e14a07..d5d08d3f0 100644
--- a/src/mailman/interfaces/nntp.py
+++ b/src/mailman/interfaces/nntp.py
@@ -26,9 +26,9 @@ from flufl.enum import Enum
 
 
 class NewsModeration(Enum):
-    # The newsgroup is not moderated
+    # The newsgroup is not moderated.
     none = 0
     # The newsgroup is moderated, but allows for an open posting policy.
     open_moderated = 1
-    # The newsgroup is moderated
+    # The newsgroup is moderated.
     moderated = 2
diff --git a/src/mailman/runners/docs/news.rst b/src/mailman/runners/docs/nntp.rst
index 71febf95c..c54dd3696 100644
--- a/src/mailman/runners/docs/news.rst
+++ b/src/mailman/runners/docs/nntp.rst
@@ -1,23 +1,20 @@
 ===============
-The news runner
+The NNTP runner
 ===============
 
-The news runner gateways mailing list messages to an NNTP newsgroup.  One of
-the most important things this runner does is prepare the message for Usenet
-(yes, I know that NNTP is not Usenet, but this runner was originally written
-to gate to Usenet, which has its own rules).
+The NNTP runner gateways mailing list messages to an NNTP newsgroup.
 
-    >>> mlist = create_list('_xtest@example.com')
+    >>> mlist = create_list('test@example.com')
     >>> mlist.linked_newsgroup = 'comp.lang.python'
 
 Some NNTP servers such as INN reject messages containing a set of prohibited
-headers, so one of the things that the news runner does is remove these
-prohibited headers.
+headers, so one of the things that this runner does is remove these prohibited
+headers.
 ::
 
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
-    ... To: _xtest@example.com
+    ... To: test@example.com
     ... NNTP-Posting-Host: news.example.com
     ... NNTP-Posting-Date: today
     ... X-Trace: blah blah
@@ -34,13 +31,13 @@ prohibited headers.
     ... """)
     >>> msgdata = {}
 
-    >>> from mailman.runners.news import prepare_message
+    >>> from mailman.runners.nntp import prepare_message
     >>> prepare_message(mlist, msg, msgdata)
     >>> msgdata['prepped']
     True
     >>> print msg.as_string()
     From: aperson@example.com
-    To: _xtest@example.com
+    To: test@example.com
     Newsgroups: comp.lang.python
     Message-ID: ...
     Lines: 1
@@ -54,7 +51,7 @@ so the news runner must collapse or move these duplicate headers to an
 
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
-    ... To: _xtest@example.com
+    ... To: test@example.com
     ... To: two@example.com
     ... Cc: three@example.com
     ... Cc: four@example.com
@@ -74,7 +71,7 @@ so the news runner must collapse or move these duplicate headers to an
     Newsgroups: comp.lang.python
     Message-ID: ...
     Lines: 1
-    To: _xtest@example.com
+    To: test@example.com
     X-Original-To: two@example.com
     CC: three@example.com
     X-Original-CC: four@example.com
@@ -91,7 +88,7 @@ the message.
 
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
-    ... To: _xtest@example.com
+    ... To: test@example.com
     ... Cc: someother@example.com
     ... Content-Transfer-Encoding: yes
     ...
@@ -103,7 +100,7 @@ the message.
     True
     >>> print msg.as_string()
     From: aperson@example.com
-    To: _xtest@example.com
+    To: test@example.com
     Cc: someother@example.com
     Content-Transfer-Encoding: yes
     Newsgroups: comp.lang.python
@@ -125,31 +122,31 @@ posting address is added for the benefit of the Usenet system.
     >>> mlist.news_moderation = NewsModeration.open_moderated
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
-    ... To: _xtest@example.com
+    ... To: test@example.com
     ... Approved: this gets deleted
     ...
     ... """)
     >>> prepare_message(mlist, msg, {})
     >>> print msg['approved']
-    _xtest@example.com
+    test@example.com
 
     >>> mlist.news_moderation = NewsModeration.moderated
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
-    ... To: _xtest@example.com
+    ... To: test@example.com
     ... Approved: this gets deleted
     ...
     ... """)
     >>> prepare_message(mlist, msg, {})
     >>> print msg['approved']
-    _xtest@example.com
+    test@example.com
 
 But if the newsgroup is not moderated, the ``Approved:`` header is not changed.
 
     >>> mlist.news_moderation = NewsModeration.none
     >>> msg = message_from_string("""\
     ... From: aperson@example.com
-    ... To: _xtest@example.com
+    ... To: test@example.com
     ... Approved: this doesn't get deleted
     ...
     ... """)
diff --git a/src/mailman/runners/news.py b/src/mailman/runners/nntp.py
index e3c6f060d..8339c735e 100644
--- a/src/mailman/runners/news.py
+++ b/src/mailman/runners/nntp.py
@@ -17,6 +17,14 @@
 
 """NNTP runner."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'NNTPRunner',
+    ]
+
+
 import re
 import email
 import socket
@@ -24,12 +32,12 @@ import logging
 import nntplib
 
 from cStringIO import StringIO
-from lazr.config import as_host_port
 
 from mailman.config import config
 from mailman.core.runner import Runner
 from mailman.interfaces.nntp import NewsModeration
 
+COMMA = ','
 COMMASPACE = ', '
 log = logging.getLogger('mailman.error')
 
@@ -49,39 +57,46 @@ mcre = re.compile(r"""
 
 
 
-class NewsRunner(Runner):
+class NNTPRunner(Runner):
     def _dispose(self, mlist, msg, msgdata):
+        # Get NNTP server connection information.
+        host = config.nntp.host.strip()
+        port = config.nntp.port.strip()
+        if len(port) == 0:
+            port = 119
+        else:
+            try:
+                port = int(port)
+            except (TypeError, ValueError):
+                log.exception('Bad [nntp]port value: {0}'.format(port))
+                port = 119
         # Make sure we have the most up-to-date state
-        mlist.Load()
         if not msgdata.get('prepped'):
             prepare_message(mlist, msg, msgdata)
+        # Flatten the message object, sticking it in a StringIO object
+        fp = StringIO(msg.as_string())
+        conn = None
         try:
-            # Flatten the message object, sticking it in a StringIO object
-            fp = StringIO(msg.as_string())
-            conn = None
-            try:
-                try:
-                    nntp_host, nntp_port = as_host_port(
-                        mlist.nntp_host, default_port=119)
-                    conn = nntplib.NNTP(nntp_host, nntp_port,
-                                        readermode=True,
-                                        user=config.nntp.username,
-                                        password=config.nntp.password)
-                    conn.post(fp)
-                except nntplib.error_temp, e:
-                    log.error('(NNTPDirect) NNTP error for list "%s": %s',
-                              mlist.internal_name(), e)
-                except socket.error, e:
-                    log.error('(NNTPDirect) socket error for list "%s": %s',
-                              mlist.internal_name(), e)
-            finally:
-                if conn:
-                    conn.quit()
-        except Exception, e:
+            conn = nntplib.NNTP(host, port,
+                                readermode=True,
+                                user=config.nntp.user,
+                                password=config.nntp.password)
+            conn.post(fp)
+        except nntplib.error_temp:
+            log.exception('{0} NNTP error for {1}'.format(
+                msg.get('message-id', 'n/a'), mlist.fqdn_listname))
+        except socket.error:
+            log.exception('{0} NNTP socket error for {1}'.format(
+                msg.get('message-id', 'n/a'), mlist.fqdn_listname))
+        except Exception:
             # Some other exception occurred, which we definitely did not
             # expect, so set this message up for requeuing.
-            self._log(e)
+            log.exception('{0} NNTP unexpected exception for {1}'.format(
+                msg.get('message-id', 'n/a'), mlist.fqdn_listname))
             return True
+        finally:
+            if conn:
+                conn.quit()
         return False
 
 
@@ -99,35 +114,41 @@ def prepare_message(mlist, msg, msgdata):
     # messages? TK: We use stripped_subject (prefix stripped) which was
     # crafted in CookHeaders.py to ensure prefix was stripped from the subject
     # came from mailing list user.
-    stripped_subject = msgdata.get('stripped_subject') \
-                       or msgdata.get('origsubj')
+    stripped_subject = msgdata.get('stripped_subject',
+                                   msgdata.get('original_subject'))
+    # XXX 2012-03-31 BAW: rename news_prefix_subject_too to nntp_.  This
+    # requires a schema change.
     if not mlist.news_prefix_subject_too and stripped_subject is not None:
         del msg['subject']
         msg['subject'] = stripped_subject
-    # Add the appropriate Newsgroups: header
-    ngheader = msg['newsgroups']
-    if ngheader is not None:
+    # Add the appropriate Newsgroups header.  Multiple Newsgroups headers are
+    # generally not allowed so we're not testing for them.
+    header = msg.get('newsgroups')
+    if header is None:
+        msg['Newsgroups'] = mlist.linked_newsgroup
+    else:
         # See if the Newsgroups: header already contains our linked_newsgroup.
         # If so, don't add it again.  If not, append our linked_newsgroup to
         # the end of the header list
-        ngroups = [s.strip() for s in ngheader.split(',')]
-        if mlist.linked_newsgroup not in ngroups:
-            ngroups.append(mlist.linked_newsgroup)
+        newsgroups = [value.strip() for value in header.split(COMMA)]
+        if mlist.linked_newsgroup not in newsgroups:
+            newsgroups.append(mlist.linked_newsgroup)
             # Subtitute our new header for the old one.
             del msg['newsgroups']
-            msg['Newsgroups'] = COMMASPACE.join(ngroups)
-    else:
-        # Newsgroups: isn't in the message
-        msg['Newsgroups'] = mlist.linked_newsgroup
+            msg['Newsgroups'] = COMMASPACE.join(newsgroups)
     # Note: We need to be sure two messages aren't ever sent to the same list
     # in the same process, since message ids need to be unique.  Further, if
-    # messages are crossposted to two Usenet-gated mailing lists, they each
-    # need to have unique message ids or the nntpd will only accept one of
-    # them.  The solution here is to substitute any existing message-id that
-    # isn't ours with one of ours, so we need to parse it to be sure we're not
-    # looping.
+    # messages are crossposted to two gated mailing lists, they must each have
+    # unique message ids or the nntpd will only accept one of them.  The
+    # solution here is to substitute any existing message-id that isn't ours
+    # with one of ours, so we need to parse it to be sure we're not looping.
     #
     # Our Message-ID format is <mailman.secs.pid.listname@hostname>
+    #
+    # XXX 2012-03-31 BAW: What we really want to do is try posting the message
+    # to the nntpd first, and only if that fails substitute a unique
+    # Message-ID.  The following should get moved out of prepare_message() and
+    # into _dispose() above.
     msgid = msg['message-id']
     hackmsgid = True
     if msgid:
@@ -139,29 +160,37 @@ def prepare_message(mlist, msg, msgdata):
     if hackmsgid:
         del msg['message-id']
         msg['Message-ID'] = email.utils.make_msgid()
-    # Lines: is useful
+    # Lines: is useful.
     if msg['Lines'] is None:
         # BAW: is there a better way?
-        count = len(list(email.Iterators.body_line_iterator(msg)))
+        count = len(list(email.iterators.body_line_iterator(msg)))
         msg['Lines'] = str(count)
     # Massage the message headers by remove some and rewriting others.  This
-    # woon't completely sanitize the message, but it will eliminate the bulk
-    # of the rejections based on message headers.  The NNTP server may still
+    # won't completely sanitize the message, but it will eliminate the bulk of
+    # the rejections based on message headers.  The NNTP server may still
     # 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
-    # Mark this message as prepared in case it has to be requeued
+        # 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
new file mode 100644
index 000000000..426e829d8
--- /dev/null
+++ b/src/mailman/runners/tests/test_nntp.py
@@ -0,0 +1,370 @@
+# Copyright (C) 2012 by the Free Software Foundation, Inc.
+#
+# This file is part of GNU Mailman.
+#
+# GNU Mailman is free software: you can redistribute it and/or modify it under
+# the terms of the GNU General Public License as published by the Free
+# Software Foundation, either version 3 of the License, or (at your option)
+# any later version.
+#
+# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT
+# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+# FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+# more details.
+#
+# You should have received a copy of the GNU General Public License along with
+# GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test the NNTP runner and related utilities."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'TestPrepareMessage',
+    'TestNNTPRunner',
+    ]
+
+
+import mock
+import socket
+import nntplib
+import unittest
+
+from mailman.app.lifecycle import create_list
+from mailman.config import config
+from mailman.interfaces.nntp import NewsModeration
+from mailman.runners import nntp
+from mailman.testing.helpers import (
+    LogFileMark,
+    configuration,
+    get_queue_messages,
+    make_testable_runner,
+    specialized_message_from_string as mfs)
+from mailman.testing.layers import ConfigLayer
+
+
+
+class TestPrepareMessage(unittest.TestCase):
+    """Test message preparation."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        self._mlist.linked_newsgroup = 'example.test'
+        self._msg = mfs("""\
+From: anne@example.com
+To: test@example.com
+Subject: A newsgroup posting
+Message-ID: <ant>
+
+Testing
+""")
+
+    def test_moderated_approved_header(self):
+        # When the mailing list is moderated , the message will get an
+        # Approved header, which NNTP software uses to forward to the
+        # newsgroup.  The message would not have gotten to the mailing list if
+        # it wasn't already approved.
+        self._mlist.news_moderation = NewsModeration.moderated
+        nntp.prepare_message(self._mlist, self._msg, {})
+        self.assertEqual(self._msg['approved'], 'test@example.com')
+
+    def test_open_moderated_approved_header(self):
+        # When the mailing list is moderated using an open posting policy, the
+        # message will get an Approved header, which NNTP software uses to
+        # forward to the newsgroup.  The message would not have gotten to the
+        # mailing list if it wasn't already approved.
+        self._mlist.news_moderation = NewsModeration.open_moderated
+        nntp.prepare_message(self._mlist, self._msg, {})
+        self.assertEqual(self._msg['approved'], 'test@example.com')
+
+    def test_moderation_removes_previous_approved_header(self):
+        # Any existing Approved header is removed from moderated messages.
+        self._msg['Approved'] = 'a bogus approval'
+        self._mlist.news_moderation = NewsModeration.moderated
+        nntp.prepare_message(self._mlist, self._msg, {})
+        headers = self._msg.get_all('approved')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'test@example.com')
+
+    def test_open_moderation_removes_previous_approved_header(self):
+        # Any existing Approved header is removed from moderated messages.
+        self._msg['Approved'] = 'a bogus approval'
+        self._mlist.news_moderation = NewsModeration.open_moderated
+        nntp.prepare_message(self._mlist, self._msg, {})
+        headers = self._msg.get_all('approved')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'test@example.com')
+
+    def test_stripped_subject(self):
+        # The cook-headers handler adds the original and/or stripped (of the
+        # prefix) subject to the metadata.  Assume that handler's been run;
+        # check the Subject header.
+        self._mlist.news_prefix_subject_too = False
+        del self._msg['subject']
+        self._msg['subject'] = 'Re: Your test'
+        msgdata = dict(stripped_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        headers = self._msg.get_all('subject')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'Your test')
+
+    def test_original_subject(self):
+        # The cook-headers handler adds the original and/or stripped (of the
+        # prefix) subject to the metadata.  Assume that handler's been run;
+        # check the Subject header.
+        self._mlist.news_prefix_subject_too = False
+        del self._msg['subject']
+        self._msg['subject'] = 'Re: Your test'
+        msgdata = dict(original_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        headers = self._msg.get_all('subject')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'Your test')
+
+    def test_stripped_subject_prefix_okay(self):
+        # The cook-headers handler adds the original and/or stripped (of the
+        # prefix) subject to the metadata.  Assume that handler's been run;
+        # check the Subject header.
+        self._mlist.news_prefix_subject_too = True
+        del self._msg['subject']
+        self._msg['subject'] = 'Re: Your test'
+        msgdata = dict(stripped_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        headers = self._msg.get_all('subject')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'Re: Your test')
+
+    def test_original_subject_prefix_okay(self):
+        # The cook-headers handler adds the original and/or stripped (of the
+        # prefix) subject to the metadata.  Assume that handler's been run;
+        # check the Subject header.
+        self._mlist.news_prefix_subject_too = True
+        del self._msg['subject']
+        self._msg['subject'] = 'Re: Your test'
+        msgdata = dict(original_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        headers = self._msg.get_all('subject')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'Re: Your test')
+
+    def test_add_newsgroups_header(self):
+        # Prepared messages get a Newsgroups header.
+        msgdata = dict(original_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        self.assertEqual(self._msg['newsgroups'], 'example.test')
+
+    def test_add_newsgroups_header_to_existing(self):
+        # If the message already has a Newsgroups header, the linked newsgroup
+        # gets appended to that value, using comma-space separated lists.
+        self._msg['Newsgroups'] = 'foo.test, bar.test'
+        msgdata = dict(original_subject='Your test')
+        nntp.prepare_message(self._mlist, self._msg, msgdata)
+        headers = self._msg.get_all('newsgroups')
+        self.assertEqual(len(headers), 1)
+        self.assertEqual(headers[0], 'foo.test, bar.test, example.test')
+
+    def test_add_lines_header(self):
+        # A Lines: header seems useful.
+        nntp.prepare_message(self._mlist, self._msg, {})
+        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'])
+
+
+
+class TestNNTPRunner(unittest.TestCase):
+    """The NNTP runner hands messages off to the NNTP server."""
+
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+        self._mlist.linked_newsgroup = 'example.test'
+        self._msg = mfs("""\
+From: anne@example.com
+To: test@example.com
+Subject: A newsgroup posting
+Message-ID: <ant>
+
+Testing
+""")
+        self._runner = make_testable_runner(nntp.NNTPRunner, 'nntp')
+        self._nntpq = config.switchboards['nntp']
+
+    @mock.patch('nntplib.NNTP')
+    def test_connect(self, class_mock):
+        # Test connection to the NNTP server with default values.
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        self._runner.run()
+        class_mock.assert_called_once_with(
+            '', 119, user='', password='', readermode=True)
+
+    @configuration('nntp', user='alpha', password='beta',
+                   host='nntp.example.com', port='2112')
+    @mock.patch('nntplib.NNTP')
+    def test_connect_with_configuration(self, class_mock):
+        # Test connection to the NNTP server with specific values.
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        self._runner.run()
+        class_mock.assert_called_once_with(
+            'nntp.example.com', 2112,
+            user='alpha', password='beta', readermode=True)
+
+    @mock.patch('nntplib.NNTP')
+    def test_post(self, class_mock):
+        # Test that the message is posted to the NNTP server.
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        self._runner.run()
+        # Get the mocked instance, which was used in the runner.
+        conn_mock = class_mock()
+        # The connection object's post() method was called once with a
+        # file-like object containing the message's bytes.  Read those bytes
+        # and make some simple checks that the message is what we expected.
+        args = conn_mock.post.call_args
+        # One positional argument.
+        self.assertEqual(len(args[0]), 1)
+        # No keyword arguments.
+        self.assertEqual(len(args[1]), 0)
+        msg = mfs(args[0][0].read())
+        self.assertEqual(msg['subject'], 'A newsgroup posting')
+
+    @mock.patch('nntplib.NNTP')
+    def test_connection_got_quit(self, class_mock):
+        # The NNTP connection gets closed after a successful post.
+        # Test that the message is posted to the NNTP server.
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        self._runner.run()
+        # Get the mocked instance, which was used in the runner.
+        conn_mock = class_mock()
+        # The connection object's post() method was called once with a
+        # file-like object containing the message's bytes.  Read those bytes
+        # and make some simple checks that the message is what we expected.
+        conn_mock.quit.assert_called_once_with()
+
+    @mock.patch('nntplib.NNTP', side_effect=nntplib.error_temp)
+    def test_connect_with_nntplib_failure(self, class_mock):
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        mark = LogFileMark('mailman.error')
+        self._runner.run()
+        log_message = mark.readline()[:-1]
+        self.assertTrue(log_message.endswith(
+            'NNTP error for test@example.com'))
+
+    @mock.patch('nntplib.NNTP', side_effect=socket.error)
+    def test_connect_with_socket_failure(self, class_mock):
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        mark = LogFileMark('mailman.error')
+        self._runner.run()
+        log_message = mark.readline()[:-1]
+        self.assertTrue(log_message.endswith(
+            'NNTP socket error for test@example.com'))
+
+    @mock.patch('nntplib.NNTP', side_effect=RuntimeError)
+    def test_connect_with_other_failure(self, class_mock):
+        # In this failure mode, the message stays queued, so we can only run
+        # the nntp runner once.
+        def once(runner):
+            # I.e. stop immediately, since the queue will not be empty.
+            return True
+        runner = make_testable_runner(nntp.NNTPRunner, 'nntp', predicate=once)
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        mark = LogFileMark('mailman.error')
+        runner.run()
+        log_message = mark.readline()[:-1]
+        self.assertTrue(log_message.endswith(
+            'NNTP unexpected exception for test@example.com'))
+        messages = get_queue_messages('nntp')
+        self.assertEqual(len(messages), 1)
+        self.assertEqual(messages[0].msgdata['listname'], 'test@example.com')
+        self.assertEqual(messages[0].msg['subject'], 'A newsgroup posting')
+
+    @mock.patch('nntplib.NNTP', side_effect=nntplib.error_temp)
+    def test_connection_never_gets_quit_after_failures(self, class_mock):
+        # The NNTP connection doesn't get closed after a unsuccessful
+        # connection, since there's nothing to close.
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        self._runner.run()
+        # Get the mocked instance, which was used in the runner.  Turn off the
+        # exception raising side effect first though!
+        class_mock.side_effect = None
+        conn_mock = class_mock()
+        # The connection object's post() method was called once with a
+        # file-like object containing the message's bytes.  Read those bytes
+        # and make some simple checks that the message is what we expected.
+        self.assertEqual(conn_mock.quit.call_count, 0)
+
+    @mock.patch('nntplib.NNTP')
+    def test_connection_got_quit_after_post_failure(self, class_mock):
+        # The NNTP connection does get closed after a unsuccessful post.
+        # Add a side-effect to the instance mock's .post() method.
+        conn_mock = class_mock()
+        conn_mock.post.side_effect = nntplib.error_temp
+        self._nntpq.enqueue(self._msg, {}, listname='test@example.com')
+        self._runner.run()
+        # The connection object's post() method was called once with a
+        # file-like object containing the message's bytes.  Read those bytes
+        # and make some simple checks that the message is what we expected.
+        conn_mock.quit.assert_called_once_with()
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)
diff --git a/src/mailman/testing/testing.cfg b/src/mailman/testing/testing.cfg
index b7e80ff02..91613cc8d 100644
--- a/src/mailman/testing/testing.cfg
+++ b/src/mailman/testing/testing.cfg
@@ -48,7 +48,7 @@ max_restarts: 1
 [runner.lmtp]
 max_restarts: 1
 
-[runner.news]
+[runner.nntp]
 max_restarts: 1
 
 [runner.out]