1 files changed, 27 insertions, 20 deletions

diff --git a/src/mailman/runners/nntp.py b/src/mailman/runners/nntp.py
index 32f0165da..411fbb6f2 100644
--- a/src/mailman/runners/nntp.py
+++ b/src/mailman/runners/nntp.py
@@ -30,6 +30,7 @@ from mailman.config import config
 from mailman.core.runner import Runner
 from mailman.interfaces.nntp import NewsModeration
 
+COMMA = ','
 COMMASPACE = ', '
 log = logging.getLogger('mailman.error')
 
@@ -101,33 +102,39 @@ def prepare_message(mlist, msg, msgdata):
     # came from mailing list user.
     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,14 +146,14 @@ 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]
@@ -163,5 +170,5 @@ def prepare_message(mlist, msg, msgdata):
         msg[header] = values[0]
         for v in values[1:]:
             msg[rewrite] = v
-    # Mark this message as prepared in case it has to be requeued
+    # Mark this message as prepared in case it has to be requeued.
     msgdata['prepped'] = True