9 files changed, 302 insertions, 28 deletions

diff --git a/src/mailman/app/bounces.py b/src/mailman/app/bounces.py
index 7f6507a14..8d2526a66 100644
--- a/src/mailman/app/bounces.py
+++ b/src/mailman/app/bounces.py
@@ -22,21 +22,28 @@ from __future__ import absolute_import, unicode_literals
 __metaclass__ = type
 __all__ = [
     'bounce_message',
+    'get_verp',
     'scan_message',
     ]
 
+
+import re
 import logging
 
 from email.mime.message import MIMEMessage
 from email.mime.text import MIMEText
+from email.utils import parseaddr
 
 from mailman.app.finder import find_components
+from mailman.config import config
 from mailman.core.i18n import _
 from mailman.email.message import UserNotification
 from mailman.interfaces.bounce import IBounceDetector
+from mailman.utilities.email import split_email
 from mailman.utilities.string import oneline
 
 log = logging.getLogger('mailman.config')
+elog = logging.getLogger('mailman.error')
 
 
 
@@ -93,3 +100,49 @@ def scan_message(mlist, msg):
         if addresses:
             return set(addresses)
     return set()
+
+
+
+def get_verp(mlist, msg):
+    """Extract a set of VERP bounce addresses.
+
+    Sadly not every MTA bounces VERP messages correctly, or consistently.
+    First, the To: header is checked, then Delivered-To: (Postfix),
+    Envelope-To: (Exim) and Apparently-To:.  Note that there can be multiple
+    Delivered-To: headers so we need to search them all (and we don't worry
+    about false positives for forwarded email, because only one should match
+    `verp_regexp`).
+
+    :param mlist: The mailing list being checked.
+    :type mlist: `IMailingList`
+    :param msg: The message being parsed.
+    :type msg: `email.message.Message`
+    :return: The set of addresses extracted from the VERP headers.
+    :rtype: set of strings
+    """
+    cre = re.compile(config.mta.verp_regexp, re.IGNORECASE)
+    blocal, bdomain = split_email(mlist.bounces_address)
+    values = set()
+    verp_matches = set()
+    for header in ('to', 'delivered-to', 'envelope-to', 'apparently-to'):
+        values.update(msg.get_all(header, []))
+    for field in values:
+        address = parseaddr(field)[1]
+        if not address:
+            # This header was empty.
+            continue
+        mo = cre.search(address)
+        if not mo:
+            # This did not match the VERP regexp.
+            continue
+        try:
+            if blocal != mo.group('bounces'):
+                # This was not a bounce to our mailing list.
+                continue
+            original_address = '{0}@{1}'.format(*mo.group('local', 'domain'))
+        except IndexError:
+            elog.error("verp_regexp doesn't yield the right match groups")
+            return set()
+        else:
+            verp_matches.add(original_address)
+    return verp_matches
diff --git a/src/mailman/app/tests/test_bounces.py b/src/mailman/app/tests/test_bounces.py
new file mode 100644
index 000000000..c6a9394ad
--- /dev/null
+++ b/src/mailman/app/tests/test_bounces.py
@@ -0,0 +1,166 @@
+# Copyright (C) 2011 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/>.
+
+"""Testing app.bounces functions."""
+
+from __future__ import absolute_import, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'test_suite',
+    ]
+
+
+import unittest
+
+from mailman.app.bounces import get_verp
+from mailman.app.lifecycle import create_list
+from mailman.testing.helpers import (
+    specialized_message_from_string as message_from_string)
+from mailman.testing.layers import ConfigLayer
+
+
+
+class TestVERP(unittest.TestCase):
+    layer = ConfigLayer
+
+    def setUp(self):
+        self._mlist = create_list('test@example.com')
+
+    def test_no_verp(self):
+        # The empty set is returned when there is no VERP headers.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: mailman-bounces@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set())
+
+    def test_verp_in_to(self):
+        # A VERP address is found in the To header.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces+anne=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_verp_in_delivered_to(self):
+        # A VERP address is found in the Delivered-To header.
+        msg = message_from_string("""\
+From: postmaster@example.com
+Delivered-To: test-bounces+anne=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_verp_in_envelope_to(self):
+        # A VERP address is found in the Envelope-To header.
+        msg = message_from_string("""\
+From: postmaster@example.com
+Envelope-To: test-bounces+anne=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_verp_in_apparently_to(self):
+        # A VERP address is found in the Apparently-To header.
+        msg = message_from_string("""\
+From: postmaster@example.com
+Apparently-To: test-bounces+anne=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_verp_with_empty_header(self):
+        # A VERP address is found, but there's an empty header.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces+anne=example.org@example.com
+To:
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_no_verp_with_empty_header(self):
+        # There's an empty header, and no VERP address is found.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To:
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set())
+
+    def test_verp_with_non_match(self):
+        # A VERP address is found, but a header had a non-matching pattern.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces+anne=example.org@example.com
+To: test-bounces@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_no_verp_with_non_match(self):
+        # No VERP address is found, and a header had a non-matching pattern.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set())
+
+    def test_multiple_verps(self):
+        # More than one VERP address was found in the same header.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces+anne=example.org@example.com
+To: test-bounces+anne=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg), set(['anne@example.org']))
+
+    def test_multiple_verps_different_values(self):
+        # More than one VERP address was found in the same header with
+        # different values.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces+anne=example.org@example.com
+To: test-bounces+bart=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg),
+                         set(['anne@example.org', 'bart@example.org']))
+
+    def test_multiple_verps_different_values_different_headers(self):
+        # More than one VERP address was found in different headers with
+        # different values.
+        msg = message_from_string("""\
+From: postmaster@example.com
+To: test-bounces+anne=example.org@example.com
+Apparently-To: test-bounces+bart=example.org@example.com
+
+""")
+        self.assertEqual(get_verp(self._mlist, msg),
+                         set(['anne@example.org', 'bart@example.org']))
+
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestVERP))
+    return suite
diff --git a/src/mailman/config/schema.cfg b/src/mailman/config/schema.cfg
index 030e5fc2c..a0f4baaf7 100644
--- a/src/mailman/config/schema.cfg
+++ b/src/mailman/config/schema.cfg
@@ -423,7 +423,7 @@ delivery_retry_period: 5d
 # Note that your MTA /must/ be configured to deliver such an addressed message
 # to mylist-bounces!
 verp_delimiter: +
-verp_format: ${bounces}+${local}=${domain}
+verp_format: ${bounces}${verp_delimiter}${local}=${domain}
 
 # For nicer confirmation emails, use a VERP-like format which encodes the
 # confirmation cookie in the reply address.  This lets us put a more user
@@ -434,6 +434,14 @@ verp_format: ${bounces}+${local}=${domain}
 # $cookie   -- the confirmation cookie
 verp_confirm_format: $address+$cookie
 
+# This regular expression unambiguously decodes VERP addresses, which will be
+# placed in the To: (or other, depending on the MTA) header of the bounce
+# message by the bouncing MTA.  Getting this right is critical -- and tricky.
+# Learn your Python regular expressions.  It must define exactly three named
+# groups, `bounces`, `local` and `domain`, with the same definition as above.
+# It will be compiled case-insensitively.
+verp_regexp: ^(?P<bounces>[^+]+?)\+(?P<local>[^=]+)=(?P<domain>[^@]+)@.*$
+
 # This is analogous to verp_regexp, but for splitting apart the
 # verp_confirm_format.  MUAs have been observed that mung
 #
diff --git a/src/mailman/email/validate.py b/src/mailman/email/validate.py
index 0c4c6a5e0..97b4b9e5f 100644
--- a/src/mailman/email/validate.py
+++ b/src/mailman/email/validate.py
@@ -29,10 +29,9 @@ import re
 
 from zope.interface import implements
 
-
-from mailman.email.utils import split_email
 from mailman.interfaces.address import (
     IEmailValidator, InvalidEmailAddressError)
+from mailman.utilities.email import split_email
 
 
 # What other characters should be disallowed?
diff --git a/src/mailman/mta/verp.py b/src/mailman/mta/verp.py
index 85b777486..4252f74d2 100644
--- a/src/mailman/mta/verp.py
+++ b/src/mailman/mta/verp.py
@@ -29,8 +29,8 @@ __all__ = [
 import logging
 
 from mailman.config import config
-from mailman.email.utils import split_email
 from mailman.mta.base import IndividualDelivery
+from mailman.utilities.email import split_email
 from mailman.utilities.string import expand
 
 
diff --git a/src/mailman/testing/helpers.py b/src/mailman/testing/helpers.py
index b5df649e0..74e00b501 100644
--- a/src/mailman/testing/helpers.py
+++ b/src/mailman/testing/helpers.py
@@ -45,6 +45,7 @@ import threading
 
 from base64 import b64encode
 from contextlib import contextmanager
+from email import message_from_string
 from httplib2 import Http
 from urllib import urlencode
 from urllib2 import HTTPError
@@ -53,6 +54,7 @@ from zope.component import getUtility
 
 from mailman.bin.master import Loop as Master
 from mailman.config import config
+from mailman.email.message import Message
 from mailman.interfaces.member import MemberRole
 from mailman.interfaces.messages import IMessageStore
 from mailman.interfaces.usermanager import IUserManager
@@ -360,3 +362,22 @@ def reset_the_world():
     # Reset the global style manager.
     config.style_manager.populate()
     
+
+
+def specialized_message_from_string(unicode_text):
+    """Parse text into a message object.
+
+    This is specialized in the sense that an instance of Mailman's own Message
+    object is returned, and this message object has an attribute
+    `original_size` which is the pre-calculated size in bytes of the message's
+    text representation.
+
+    Also, the text must be ASCII-only unicode.
+    """
+    # This mimic what Switchboard.dequeue() does when parsing a message from
+    # text into a Message instance.
+    text = unicode_text.encode('ascii')
+    original_size = len(text)
+    message = message_from_string(text, Message)
+    message.original_size = original_size
+    return message
diff --git a/src/mailman/tests/test_documentation.py b/src/mailman/tests/test_documentation.py
index ee8298b7f..8be0f74dd 100644
--- a/src/mailman/tests/test_documentation.py
+++ b/src/mailman/tests/test_documentation.py
@@ -34,14 +34,11 @@ import sys
 import doctest
 import unittest
 
-from email import message_from_string
-
 import mailman
 
 from mailman.app.lifecycle import create_list
 from mailman.config import config
-from mailman.email.message import Message
-from mailman.testing.helpers import call_api
+from mailman.testing.helpers import call_api, specialized_message_from_string
 from mailman.testing.layers import SMTPLayer
 
 
@@ -66,25 +63,6 @@ class chdir:
 
 
 
-def specialized_message_from_string(unicode_text):
-    """Parse text into a message object.
-
-    This is specialized in the sense that an instance of Mailman's own Message
-    object is returned, and this message object has an attribute
-    `original_size` which is the pre-calculated size in bytes of the message's
-    text representation.
-
-    Also, the text must be ASCII-only unicode.
-    """
-    # This mimic what Switchboard.dequeue() does when parsing a message from
-    # text into a Message instance.
-    text = unicode_text.encode('ascii')
-    original_size = len(text)
-    message = message_from_string(text, Message)
-    message.original_size = original_size
-    return message
-
-
 def stop():
     """Call into pdb.set_trace()"""
     # Do the import here so that you get the wacky special hacked pdb instead
diff --git a/src/mailman/email/utils.py b/src/mailman/utilities/email.py
index b4f3ac5f7..324b6b134 100644
--- a/src/mailman/email/utils.py
+++ b/src/mailman/utilities/email.py
@@ -15,7 +15,7 @@
 # You should have received a copy of the GNU General Public License along with
 # GNU Mailman.  If not, see <http://www.gnu.org/licenses/>.
 
-"""Module stuff."""
+"""Email helpers."""
 
 from __future__ import absolute_import, unicode_literals
 
diff --git a/src/mailman/utilities/tests/test_email.py b/src/mailman/utilities/tests/test_email.py
new file mode 100644
index 000000000..d34c55ee8
--- /dev/null
+++ b/src/mailman/utilities/tests/test_email.py
@@ -0,0 +1,49 @@
+# Copyright (C) 2011 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/>.
+
+"""Testing app.bounces functions."""
+
+from __future__ import absolute_import, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'test_suite',
+    ]
+
+
+import unittest
+
+from mailman.utilities.email import split_email
+
+
+
+class TestEmail(unittest.TestCase):
+    def test_normal_split(self):
+        self.assertEqual(split_email('anne@example.com'),
+                         ('anne', ['example', 'com']))
+        self.assertEqual(split_email('anne@foo.example.com'),
+                         ('anne', ['foo', 'example', 'com']))
+
+    def test_no_at_split(self):
+        self.assertEqual(split_email('anne'), ('anne', None))
+
+
+
+def test_suite():
+    suite = unittest.TestSuite()
+    suite.addTest(unittest.makeSuite(TestEmail))
+    return suite