8 files changed, 82 insertions, 36 deletions

diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py
index b696bcfe7..12e86e8e6 100644
--- a/src/mailman/chains/headers.py
+++ b/src/mailman/chains/headers.py
@@ -53,11 +53,9 @@ def make_link(header, pattern, chain=None):
     """
     rule = HeaderMatchRule(header, pattern)
     if chain is None:
-        action = LinkAction.defer
-    else:
-        chain = config.chains[chain]
-        action = LinkAction.jump
-    return Link(rule, action, chain)
+        return Link(rule, LinkAction.defer)
+    chain = config.chains[chain]
+    return Link(rule, LinkAction.jump, chain)
 
 
 
@@ -141,14 +139,12 @@ class HeaderMatchChain(Chain):
                 continue
             yield make_link(parts[0], parts[1].lstrip())
         # Then return all the explicitly added links.
-        for link in self._extended_links:
-            yield link
+        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,
                    config.chains[config.antispam.jump_chain])
         # Then return all the list-specific header matches.
-        # Python 3.3: Use 'yield from'
         for entry in mlist.header_matches:
             yield make_link(entry.header, entry.pattern, entry.chain)
diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py
index 104913034..636eec196 100644
--- a/src/mailman/chains/tests/test_headers.py
+++ b/src/mailman/chains/tests/test_headers.py
@@ -27,13 +27,13 @@ import unittest
 from mailman.app.lifecycle import create_list
 from mailman.chains.headers import HeaderMatchRule
 from mailman.config import config
+from mailman.core.chains import process
 from mailman.email.message import Message
-from mailman.model.mailinglist import HeaderMatch
 from mailman.interfaces.chain import LinkAction, HoldEvent
-from mailman.core.chains import process
+from mailman.model.mailinglist import HeaderMatch
 from mailman.testing.layers import ConfigLayer
-from mailman.testing.helpers import (LogFileMark, configuration,
-    event_subscribers, get_queue_messages,
+from mailman.testing.helpers import (
+    configuration, event_subscribers, get_queue_messages, LogFileMark,
     specialized_message_from_string as mfs)
 
 
@@ -129,8 +129,8 @@ class TestHeaderChain(unittest.TestCase):
         # mailing-list configuration.
         chain = config.chains['header-match']
         self._mlist.header_matches = [HeaderMatch(header='Foo', pattern='a+')]
-        links = [ link for link in chain.get_links(self._mlist, Message(), {})
-                  if link.rule.name != 'any' ]
+        links = [link for link in chain.get_links(self._mlist, Message(), {})
+                  if link.rule.name != 'any']
         self.assertEqual(len(links), 1)
         self.assertEqual(links[0].action, LinkAction.defer)
         self.assertEqual(links[0].rule.header, 'Foo')
@@ -145,12 +145,12 @@ class TestHeaderChain(unittest.TestCase):
             HeaderMatch(header='Bar', pattern='b+', chain='discard'),
             HeaderMatch(header='Baz', pattern='z+', chain='accept'),
             ]
-        links = [ link for link in chain.get_links(self._mlist, Message(), {})
-                  if link.rule.name != 'any' ]
+        links = [link for link in chain.get_links(self._mlist, Message(), {})
+                  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 ],
+            [(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'),
@@ -158,7 +158,7 @@ class TestHeaderChain(unittest.TestCase):
 
     @configuration('antispam', header_checks="""
     Foo: foo
-    """, jump_chain="hold")
+    """, jump_chain='hold')
     def test_priority_site_over_list(self):
         # Test that the site-wide checks take precedence over the list-specific
         # checks.
@@ -175,13 +175,11 @@ A message body.
         msgdata = {}
         self._mlist.header_matches = [
             HeaderMatch(header='Foo', pattern='foo', chain='accept')
-        ]
+            ]
         # This event subscriber records the event that occurs when the message
         # is processed by the owner chain.
         events = []
-        def catch_event(event):
-            events.append(event)
-        with event_subscribers(catch_event):
+        with event_subscribers(events.append):
             process(self._mlist, msg, msgdata, start_chain='header-match')
         self.assertEqual(len(events), 1)
         event = events[0]
diff --git a/src/mailman/database/alembic/versions/42756496720_header_matches.py b/src/mailman/database/alembic/versions/42756496720_header_matches.py
index ae3463286..f79b02b86 100644
--- a/src/mailman/database/alembic/versions/42756496720_header_matches.py
+++ b/src/mailman/database/alembic/versions/42756496720_header_matches.py
@@ -19,8 +19,8 @@ def upgrade():
     header_match_table = op.create_table('headermatch',
         sa.Column('id', sa.Integer(), nullable=False),
         sa.Column('mailing_list_id', sa.Integer(), nullable=True),
-        sa.Column('header', sa.Unicode(), nullable=True),
-        sa.Column('pattern', sa.Unicode(), nullable=True),
+        sa.Column('header', sa.Unicode(), nullable=False),
+        sa.Column('pattern', sa.Unicode(), nullable=False),
         sa.Column('chain', sa.Unicode(), nullable=True),
         sa.ForeignKeyConstraint(['mailing_list_id'], ['mailinglist.id'], ),
         sa.PrimaryKeyConstraint('id')
diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py
index 1db5f0dae..59cb0ffd4 100644
--- a/src/mailman/interfaces/mailinglist.py
+++ b/src/mailman/interfaces/mailinglist.py
@@ -857,5 +857,6 @@ class IHeaderMatch(Interface):
     chain = Attribute(
         """The chain to jump to on a match.
 
-        If it is None, the configuration file action for spam is used.
+        If it is None, the `[antispam]jump_chain` action in the configuration
+        file is used.
         """)
diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py
index c9f5e8ef1..f7b1cf72c 100644
--- a/src/mailman/model/mailinglist.py
+++ b/src/mailman/model/mailinglist.py
@@ -635,6 +635,6 @@ class HeaderMatch(Model):
     mailing_list_id = Column(Integer, ForeignKey('mailinglist.id'))
     mailing_list = relationship('MailingList', backref='header_matches')
 
-    header = Column(Unicode, nullable=True)
-    pattern = Column(Unicode, nullable=True)
+    header = Column(Unicode)
+    pattern = Column(Unicode)
     chain = Column(Unicode, nullable=True)
diff --git a/src/mailman/rules/docs/header-matching.rst b/src/mailman/rules/docs/header-matching.rst
index 7b2d8c6d7..e3f9f9ab2 100644
--- a/src/mailman/rules/docs/header-matching.rst
+++ b/src/mailman/rules/docs/header-matching.rst
@@ -119,8 +119,14 @@ List-specific header matching
 =============================
 
 Each mailing list can also be configured with a set of header matching regular
-expression rules.  These are used to impose list-specific header filtering
-with the same semantics as the global `[antispam]` section.
+expression rules.  These can be used to impose list-specific header filtering
+with the same semantics as the global `[antispam]` section, or to have a
+different action.
+
+To follow the global antispam action, the header match rule must not specify a
+`chain` to jump to. If the default antispam action is changed in the
+configuration file and Mailman is restarted, those rules will get the new jump
+action.
 
 The list administrator wants to match not on four stars, but on three plus
 signs, but only for the current mailing list.
@@ -168,3 +174,26 @@ As does a message with a spam score of four pluses.
     Rule hits:
         x-spam-score: [+]{3,}
     No rules missed
+
+Now, the list administrator wants to match on three plus signs, but wants those
+emails to be discarded instead of held.
+
+    >>> mlist.header_matches = [
+    ...     HeaderMatch(header='x-spam-score', pattern='[+]{3,}', 'discard')
+    ...     ]
+
+A message with a spam score of three pluses will still match, and the message
+will be discarded.
+
+    >>> msgdata = {}
+    >>> del msg['x-spam-score']
+    >>> msg['X-Spam-Score'] = '+++'
+    >>> del msg['message-id']
+    >>> msg['Message-Id'] = '<dee>'
+    >>> with event_subscribers(handler):
+    ...     process(mlist, msg, msgdata, 'header-match')
+    DiscardEvent discard <dee>
+    >>> hits_and_misses(msgdata)
+    Rule hits:
+        x-spam-score: [+]{3,}
+    No rules missed
diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py
index 1261823fd..f767ddbe3 100644
--- a/src/mailman/utilities/importer.py
+++ b/src/mailman/utilities/importer.py
@@ -27,8 +27,8 @@ import os
 import re
 import sys
 import codecs
-import datetime
 import logging
+import datetime
 
 from mailman.config import config
 from mailman.core.errors import MailmanError
@@ -39,8 +39,8 @@ from mailman.interfaces.archiver import ArchivePolicy
 from mailman.interfaces.autorespond import ResponseAction
 from mailman.interfaces.bans import IBanManager
 from mailman.interfaces.bounce import UnrecognizedBounceDisposition
-from mailman.interfaces.digests import DigestFrequency
 from mailman.interfaces.chain import LinkAction
+from mailman.interfaces.digests import DigestFrequency
 from mailman.interfaces.languages import ILanguageManager
 from mailman.interfaces.mailinglist import IAcceptableAliasSet
 from mailman.interfaces.mailinglist import Personalization, ReplyToMunging
@@ -335,9 +335,14 @@ def import_config_pck(mlist, config_dict):
             # expression.  Make that explicit for MM3.
             alias_set.add('^' + address)
     # Handle header_filter_rules conversion to header_matches
-    for line_patterns, action, _unused in \
-            config_dict.get('header_filter_rules', []):
-        chain = action_to_chain(action)
+    header_filter_rules = config_dict.get('header_filter_rules', [])
+    for line_patterns, action, _unused in header_filter_rules:
+        try:
+            chain = action_to_chain(action)
+        except KeyError:
+            log.warning('Unsupported header_filter_rules action: %r',
+                        action)
+            continue
         # now split the pattern in a header and a pattern
         for line_pattern in line_patterns.splitlines():
             if not line_pattern.strip():
diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py
index e9ad29a1b..85ee993ba 100644
--- a/src/mailman/utilities/tests/test_import.py
+++ b/src/mailman/utilities/tests/test_import.py
@@ -50,8 +50,8 @@ from mailman.interfaces.nntp import NewsgroupModeration
 from mailman.interfaces.templates import ITemplateLoader
 from mailman.interfaces.usermanager import IUserManager
 from mailman.model.mailinglist import HeaderMatch
-from mailman.testing.layers import ConfigLayer
 from mailman.testing.helpers import LogFileMark
+from mailman.testing.layers import ConfigLayer
 from mailman.utilities.filesystem import makedirs
 from mailman.utilities.importer import import_config_pck, Import21Error
 from mailman.utilities.string import expand
@@ -444,6 +444,23 @@ class TestBasicImport(unittest.TestCase):
             [ ('x-spam-status', 'Yes', None) ]
         )
 
+    def test_header_matches_unsupported_action(self):
+        # Check that an unsupported actions are skipped
+        for action_num in (1, 4, 5):
+            self._pckdict['header_filter_rules'] = [
+                ('HeaderName: test-re', action_num, False),
+            ]
+            error_log = LogFileMark('mailman.error')
+            self._import()
+            self.assertListEqual(self._mlist.header_matches, [])
+            self.assertIn('Unsupported header_filter_rules action',
+                          error_log.readline())
+            # Avoid a useless warning.
+            for member in self._mlist.members.members:
+                member.unsubscribe()
+            for member in self._mlist.owners.members:
+                member.unsubscribe()
+
 
 
 class TestArchiveImport(unittest.TestCase):