Commit b9baf43a authored by Aurélien Bompard's avatar Aurélien Bompard Committed by Barry Warsaw

Implement changes from the review

parent d468d096
......@@ -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)
......@@ -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]
......
......@@ -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')
......
......@@ -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.
""")
......@@ -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)
......@@ -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
......@@ -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():
......
......@@ -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):
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment