Clean up pass through abompard's branch.

2 files changed, 43 insertions, 18 deletions

diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py
index 12e86e8e6..138f34035 100644
--- a/src/mailman/chains/headers.py
+++ b/src/mailman/chains/headers.py
@@ -40,20 +40,24 @@ log = logging.getLogger('mailman.error')
 def make_link(header, pattern, chain=None):
     """Create a Link object.
 
-    The link action is always to defer, since at the end of all the header
-    checks, we'll jump to the chain defined in the configuration file, should
-    any of them have matched.
+    The link action is to defer by default, since at the end of all the
+    header checks, we'll jump to the chain defined in the configuration
+    file, should any of them have matched.  However, it is possible to
+    create a link which jumps to a specific chain.
 
     :param header: The email header name to check, e.g. X-Spam.
     :type header: string
     :param pattern: A regular expression for matching the header value.
     :type pattern: string
+    :param chain: When given, this is the chain to jump to if the
+        pattern matches the header.
+    :type chain: string
     :return: The link representing this rule check.
     :rtype: `ILink`
     """
     rule = HeaderMatchRule(header, pattern)
     if chain is None:
-        return Link(rule, LinkAction.defer)
+        return Link(rule)
     chain = config.chains[chain]
     return Link(rule, LinkAction.jump, chain)
 
@@ -135,15 +139,17 @@ class HeaderMatchChain(Chain):
             parts = line.split(':', 1)
             if len(parts) != 2:
                 log.error('Configuration error: [antispam]header_checks '
-                          'contains bogus line: {0}'.format(line))
+                          'contains bogus line: {}'.format(line))
                 continue
             yield make_link(parts[0], parts[1].lstrip())
         # Then return all the explicitly added links.
         yield from self._extended_links
-        # If any of the above rules matched, jump to the chain
-        # defined in the configuration file. This takes precedence over
-        # list-specific matches for security considerations.
-        yield Link(config.rules['any'], LinkAction.jump,
+        # If any of the above rules matched, they will have deferred their
+        # action until now, so jump to the chain defined in the configuration
+        # file.  For security considerations, this takes precedence over
+        # list-specific matches.
+        yield Link(config.rules['any'],
+                   LinkAction.jump,
                    config.chains[config.antispam.jump_chain])
         # Then return all the list-specific header matches.
         for entry in mlist.header_matches:
diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py
index 851720f95..312f1eb54 100644
--- a/src/mailman/chains/tests/test_headers.py
+++ b/src/mailman/chains/tests/test_headers.py
@@ -25,16 +25,16 @@ __all__ = [
 import unittest
 
 from mailman.app.lifecycle import create_list
-from mailman.chains.headers import HeaderMatchRule
+from mailman.chains.headers import HeaderMatchRule, make_link
 from mailman.config import config
 from mailman.core.chains import process
 from mailman.email.message import Message
 from mailman.interfaces.chain import LinkAction, HoldEvent
 from mailman.interfaces.mailinglist import IHeaderMatchSet
-from mailman.testing.layers import ConfigLayer
 from mailman.testing.helpers import (
-    configuration, event_subscribers, get_queue_messages, LogFileMark,
+    LogFileMark, configuration, event_subscribers,
     specialized_message_from_string as mfs)
+from mailman.testing.layers import ConfigLayer
 
 
 
@@ -46,6 +46,24 @@ class TestHeaderChain(unittest.TestCase):
     def setUp(self):
         self._mlist = create_list('test@example.com')
 
+    def test_make_link(self):
+        # Test that make_link() with no given chain creates a Link with a
+        # deferred link action.
+        link = make_link('Subject', '[tT]esting')
+        self.assertEqual(link.rule.header, 'Subject')
+        self.assertEqual(link.rule.pattern, '[tT]esting')
+        self.assertEqual(link.action, LinkAction.defer)
+        self.assertIsNone(link.chain)
+
+    def test_make_link_with_chain(self):
+        # Test that make_link() with a given chain creates a Link with a jump
+        # action to the chain.
+        link = make_link('Subject', '[tT]esting', 'accept')
+        self.assertEqual(link.rule.header, 'Subject')
+        self.assertEqual(link.rule.pattern, '[tT]esting')
+        self.assertEqual(link.action, LinkAction.jump)
+        self.assertEqual(link.chain, config.chains['accept'])
+
     @configuration('antispam', header_checks="""
     Foo: a+
     Bar: bb?
@@ -129,9 +147,9 @@ class TestHeaderChain(unittest.TestCase):
         # mailing-list configuration.
         chain = config.chains['header-match']
         header_matches = IHeaderMatchSet(self._mlist)
-        header_matches.add('Foo', 'a+', None)
+        header_matches.add('Foo', 'a+')
         links = [link for link in chain.get_links(self._mlist, Message(), {})
-                  if link.rule.name != 'any']
+                 if link.rule.name != 'any']
         self.assertEqual(len(links), 1)
         self.assertEqual(links[0].action, LinkAction.defer)
         self.assertEqual(links[0].rule.header, 'foo')
@@ -146,11 +164,12 @@ class TestHeaderChain(unittest.TestCase):
         header_matches.add('Bar', 'b+', 'discard')
         header_matches.add('Baz', 'z+', 'accept')
         links = [link for link in chain.get_links(self._mlist, Message(), {})
-                  if link.rule.name != 'any']
+                 if link.rule.name != 'any']
         self.assertEqual(len(links), 3)
-        self.assertListEqual(
-            [(link.rule.header, link.rule.pattern, link.action, link.chain.name)
-              for link in links],
+        self.assertEqual([
+            (link.rule.header, link.rule.pattern, link.action, link.chain.name)
+            for link in links
+            ],
             [('foo', 'a+', LinkAction.jump, 'reject'),
              ('bar', 'b+', LinkAction.jump, 'discard'),
              ('baz', 'z+', LinkAction.jump, 'accept'),