* Flesh out the BounceRunner, complete with tests.

* Clean up the DSN bounce processor.

2 files changed, 195 insertions, 28 deletions

diff --git a/src/mailman/queue/bounce.py b/src/mailman/queue/bounce.py
index 819f0e368..a714f2669 100644
--- a/src/mailman/queue/bounce.py
+++ b/src/mailman/queue/bounce.py
@@ -19,9 +19,11 @@
 
 import logging
 
-from mailman.app.bounces import StandardVERP
-from mailman.config import config
-from mailman.interfaces.bounce import Stop
+from zope.component import getUtility
+
+from mailman.app.bounces import (
+    ProbeVERP, StandardVERP, maybe_forward, scan_message)
+from mailman.interfaces.bounce import BounceContext, IBounceProcessor, Stop
 from mailman.queue import Runner
 
 
@@ -35,37 +37,44 @@ elog = logging.getLogger('mailman.error')
 class BounceRunner(Runner):
     """The bounce runner."""
 
+    def __init__(self, name, slice=None):
+        super(BounceRunner, self).__init__(name, slice)
+        self._processor = getUtility(IBounceProcessor)
+
     def _dispose(self, mlist, msg, msgdata):
         # List isn't doing bounce processing?
         if not mlist.bounce_processing:
             return False
         # Try VERP detection first, since it's quick and easy
-        addrs = StandardVERP().get_verp(mlist, msg)
-        if addrs:
-            # We have an address, but check if the message is non-fatal.
-            if scan_messages(mlist, msg) is Stop:
-                return
+        context = BounceContext.normal
+        addresses = StandardVERP().get_verp(mlist, msg)
+        if addresses:
+            # We have an address, but check if the message is non-fatal.  It
+            # will be non-fatal if the bounce scanner returns Stop.  It will
+            # return a set of addresses when the bounce is fatal, but we don't
+            # care about those addresses, since we got it out of the VERP.
+            if scan_message(mlist, msg) is Stop:
+                return False
         else:
             # See if this was a probe message.
-            token = verp_probe(mlist, msg)
-            if token:
-                self._probe_bounce(mlist, token)
-                return
-            # That didn't give us anything useful, so try the old fashion
-            # bounce matching modules.
-            addrs = scan_messages(mlist, msg)
-            if addrs is Stop:
-                # This is a recognized, non-fatal notice. Ignore it.
-                return
+            addresses = ProbeVERP().get_verp(mlist, msg)
+            if addresses:
+                context = BounceContext.probe
+            else:
+                # That didn't give us anything useful, so try the old fashion
+                # bounce matching modules.
+                addresses = scan_message(mlist, msg)
+                if addresses is Stop:
+                    # This is a recognized, non-fatal notice. Ignore it.
+                    return False
         # If that still didn't return us any useful addresses, then send it on
         # or discard it.
-        if not addrs:
-            log.info('bounce message w/no discernable addresses: %s',
-                     msg.get('message-id'))
+        if len(addresses) > 0:
+            for address in addresses:
+                self._processor.register(mlist, address, msg, context)
+        else:
+            log.info('Bounce message w/no discernable addresses: %s',
+                     msg.get('message-id', 'n/a'))
             maybe_forward(mlist, msg)
-            return
-        # BAW: It's possible that there are None's in the list of addresses,
-        # although I'm unsure how that could happen.  Possibly scan_messages()
-        # can let None's sneak through.  In any event, this will kill them.
-        addrs = filter(None, addrs)
-        self._queue_bounces(mlist.fqdn_listname, addrs, msg)
+        # Dequeue this message.
+        return False
diff --git a/src/mailman/queue/tests/test_bounce.py b/src/mailman/queue/tests/test_bounce.py
index ba2476e79..1946df50c 100644
--- a/src/mailman/queue/tests/test_bounce.py
+++ b/src/mailman/queue/tests/test_bounce.py
@@ -29,12 +29,16 @@ import unittest
 
 from zope.component import getUtility
 
+from mailman.app.bounces import send_probe
 from mailman.app.lifecycle import create_list
 from mailman.config import config
+from mailman.interfaces.bounce import (
+    BounceContext, IBounceProcessor, UnrecognizedBounceDisposition)
 from mailman.interfaces.member import MemberRole
 from mailman.interfaces.usermanager import IUserManager
 from mailman.queue.bounce import BounceRunner
 from mailman.testing.helpers import (
+    LogFileMark,
     get_queue_messages,
     make_testable_runner,
     specialized_message_from_string as message_from_string)
@@ -56,11 +60,19 @@ class TestBounceQueue(unittest.TestCase):
         self._member = self._mlist.subscribe(self._anne, MemberRole.member)
         self._msg = message_from_string("""\
 From: mail-daemon@example.com
-To: test-bounce+anne=example.com@example.com
+To: test-bounces+anne=example.com@example.com
 Message-Id: <first>
 
 """)
         self._msgdata = dict(listname='test@example.com')
+        self._processor = getUtility(IBounceProcessor)
+        config.push('site owner', """
+        [mailman]
+        site_owner: postmaster@example.com
+        """)
+
+    def tearDown(self):
+        config.pop('site owner')
 
     def test_does_no_processing(self):
         # If the mailing list does no bounce processing, the messages are
@@ -69,6 +81,152 @@ Message-Id: <first>
         self._bounceq.enqueue(self._msg, self._msgdata)
         self._runner.run()
         self.assertEqual(len(get_queue_messages('bounces')), 0)
+        self.assertEqual(len(list(self._processor.events)), 0)
+
+    def test_verp_detection(self):
+        # When we get a VERPd bounce, and we're doing processing, a bounce
+        # event will be registered.
+        self._bounceq.enqueue(self._msg, self._msgdata)
+        self._runner.run()
+        self.assertEqual(len(get_queue_messages('bounces')), 0)
+        events = list(self._processor.events)
+        self.assertEqual(len(events), 1)
+        self.assertEqual(events[0].email, 'anne@example.com')
+        self.assertEqual(events[0].list_name, 'test@example.com')
+        self.assertEqual(events[0].message_id, '<first>')
+        self.assertEqual(events[0].context, BounceContext.normal)
+        self.assertEqual(events[0].processed, False)
+
+    def test_nonfatal_verp_detection(self):
+        # A VERPd bounce was received, but the error was nonfatal.
+        nonfatal = message_from_string("""\
+From: mail-daemon@example.com
+To: test-bounces+anne=example.com@example.com
+Message-Id: <first>
+Content-Type: multipart/report; report-type=delivery-status; boundary=AAA
+MIME-Version: 1.0
+
+--AAA
+Content-Type: message/delivery-status
+
+Action: delayed
+Original-Recipient: rfc822; somebody@example.com
+
+--AAA--
+""")
+        self._bounceq.enqueue(nonfatal, self._msgdata)
+        self._runner.run()
+        self.assertEqual(len(get_queue_messages('bounces')), 0)
+        events = list(self._processor.events)
+        self.assertEqual(len(events), 0)
+
+    def test_verp_probe_bounce(self):
+        # A VERP probe bounced.  The primary difference here is that the
+        # registered bounce event will have a different context.  The
+        # Message-Id will be different too, because of the way we're
+        # simulating the probe bounce.
+        #
+        # Start be simulating a probe bounce.
+        send_probe(self._member, self._msg)
+        message = get_queue_messages('virgin')[0].msg
+        bounce = message_from_string("""\
+To: {0}
+From: mail-daemon@example.com
+Message-Id: <second>
+
+""".format(message['From']))
+        self._bounceq.enqueue(bounce, self._msgdata)
+        self._runner.run()
+        self.assertEqual(len(get_queue_messages('bounces')), 0)
+        events = list(self._processor.events)
+        self.assertEqual(len(events), 1)
+        self.assertEqual(events[0].email, 'anne@example.com')
+        self.assertEqual(events[0].list_name, 'test@example.com')
+        self.assertEqual(events[0].message_id, '<second>')
+        self.assertEqual(events[0].context, BounceContext.probe)
+        self.assertEqual(events[0].processed, False)
+
+    def test_nonverp_detectable_fatal_bounce(self):
+        # Here's a bounce that is not VERPd, but which has a bouncing address
+        # that can be parsed from a known bounce format.  DSN is as good as
+        # any, but we'll make the parsed address different for the fun of it.
+        dsn = message_from_string("""\
+From: mail-daemon@example.com
+To: test-bounces@example.com
+Message-Id: <first>
+Content-Type: multipart/report; report-type=delivery-status; boundary=AAA
+MIME-Version: 1.0
+
+--AAA
+Content-Type: message/delivery-status
+
+Action: fail
+Original-Recipient: rfc822; bart@example.com
+
+--AAA--
+""")
+        self._bounceq.enqueue(dsn, self._msgdata)
+        self._runner.run()
+        self.assertEqual(len(get_queue_messages('bounces')), 0)
+        events = list(self._processor.events)
+        self.assertEqual(len(events), 1)
+        self.assertEqual(events[0].email, 'bart@example.com')
+        self.assertEqual(events[0].list_name, 'test@example.com')
+        self.assertEqual(events[0].message_id, '<first>')
+        self.assertEqual(events[0].context, BounceContext.normal)
+        self.assertEqual(events[0].processed, False)
+
+    def test_nonverp_detectable_nonfatal_bounce(self):
+        # Here's a bounce that is not VERPd, but which has a bouncing address
+        # that can be parsed from a known bounce format.  The bounce is
+        # non-fatal so no bounce event is registered.
+        dsn = message_from_string("""\
+From: mail-daemon@example.com
+To: test-bounces@example.com
+Message-Id: <first>
+Content-Type: multipart/report; report-type=delivery-status; boundary=AAA
+MIME-Version: 1.0
+
+--AAA
+Content-Type: message/delivery-status
+
+Action: delayed
+Original-Recipient: rfc822; bart@example.com
+
+--AAA--
+""")
+        self._bounceq.enqueue(dsn, self._msgdata)
+        self._runner.run()
+        self.assertEqual(len(get_queue_messages('bounces')), 0)
+        events = list(self._processor.events)
+        self.assertEqual(len(events), 0)
+
+    def test_no_detectable_bounce_addresses(self):
+        # A bounce message was received, but no addresses could be detected.
+        # A message will be logged in the bounce log though, and the message
+        # can be forwarded to someone who can do something about it.
+        self._mlist.forward_unrecognized_bounces_to = (
+            UnrecognizedBounceDisposition.site_owner)
+        bogus = message_from_string("""\
+From: mail-daemon@example.com
+To: test-bounces@example.com
+Message-Id: <third>
+
+""")
+        self._bounceq.enqueue(bogus, self._msgdata)
+        mark = LogFileMark('mailman.bounce')
+        self._runner.run()
+        self.assertEqual(len(get_queue_messages('bounces')), 0)
+        events = list(self._processor.events)
+        self.assertEqual(len(events), 0)
+        line = mark.readline()
+        self.assertEqual(
+            line[-51:-1],
+            'Bounce message w/no discernable addresses: <third>')
+        # Here's the forwarded message to the site owners.
+        forwards = get_queue_messages('virgin')
+        self.assertEqual(len(forwards), 1)
+        self.assertEqual(forwards[0].msg['to'], 'postmaster@example.com')