From 6a2295762adc74362ae1f2fa8704c7aba7380145 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Wed, 9 Sep 2015 15:01:13 +0200 Subject: Handle header-match rule-specific action --- src/mailman/chains/headers.py | 9 +++++++-- src/mailman/chains/tests/test_headers.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py index 7c5d11bee..ea4652062 100644 --- a/src/mailman/chains/headers.py +++ b/src/mailman/chains/headers.py @@ -37,7 +37,7 @@ log = logging.getLogger('mailman.error') -def make_link(header, pattern): +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 @@ -52,7 +52,12 @@ def make_link(header, pattern): :rtype: `ILink` """ rule = HeaderMatchRule(header, pattern) - return Link(rule, LinkAction.defer) + if chain is None: + action = LinkAction.defer + else: + chain = config.chains[chain] + action = LinkAction.jump + return Link(rule, action, chain) diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py index a00f9c588..cca2e041f 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -119,3 +119,35 @@ class TestHeaderChain(unittest.TestCase): HeaderMatchRule, 'x-spam-score', '.*') finally: config.rules = saved_rules + + def test_list_rule(self): + # Test that the header-match chain has the header checks from the + # mailing-list configuration. + chain = config.chains['header-match'] + self._mlist.header_matches = [('Foo', 'a+')] + 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') + self.assertEqual(links[0].rule.pattern, 'a+') + + def test_list_complex_rule(self): + # Test that the mailing-list header-match complex rules are read + # properly. + chain = config.chains['header-match'] + self._mlist.header_matches = [ + ('Foo', 'a+', 'reject'), + ('Bar', 'b+', 'discard'), + ('Baz', 'z+', 'accept'), + ] + 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 ], + [('Foo', 'a+', LinkAction.jump, 'reject'), + ('Bar', 'b+', LinkAction.jump, 'discard'), + ('Baz', 'z+', LinkAction.jump, 'accept'), + ]) -- cgit v1.2.3-70-g09d2 From ece5509965f64fe988a1eacc0a68cab9d50e2724 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Wed, 2 Sep 2015 15:27:10 +0200 Subject: Convert header_filter_rules from 2.1 to header_matches --- src/mailman/utilities/importer.py | 59 ++++++++++++++++ src/mailman/utilities/tests/test_import.py | 109 +++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+) (limited to 'src') diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py index 293e9c39c..df0557c08 100644 --- a/src/mailman/utilities/importer.py +++ b/src/mailman/utilities/importer.py @@ -24,9 +24,11 @@ __all__ = [ import os +import re import sys import codecs import datetime +import logging from mailman.config import config from mailman.core.errors import MailmanError @@ -38,6 +40,7 @@ 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.languages import ILanguageManager from mailman.interfaces.mailinglist import IAcceptableAliasSet from mailman.interfaces.mailinglist import Personalization, ReplyToMunging @@ -51,6 +54,8 @@ from sqlalchemy import Boolean from urllib.error import URLError from zope.component import getUtility +log = logging.getLogger('mailman.error') + class Import21Error(MailmanError): @@ -124,6 +129,24 @@ def nonmember_action_mapping(value): 3: Action.discard, }[value] + +def action_to_chain(value): + # Converts an action number in Mailman 2.1 to the name of the corresponding + # chain in 3.x. The actions "approve", "subscribe" and "unsubscribe" are + # ignored. The defer action is converted to None, because it is not a jump + # to a terminal chain. + return { + 0: None, + #1: "approve", + 2: "reject", + 3: "discard", + #4: "subscribe", + #5: "unsubscribe", + 6: "accept", + 7: "hold", + }[value] + + def check_language_code(code): if code is None: @@ -310,6 +333,42 @@ def import_config_pck(mlist, config_dict): # When .add() rejects this, the line probably contains a regular # expression. Make that explicit for MM3. alias_set.add('^' + address) + # Handle header_filter_rules conversion to header_matches + header_matches = [] + for line_patterns, action, _unused in \ + config_dict.get('header_filter_rules', []): + chain = action_to_chain(action) + # now split the pattern in a header and a pattern + for line_pattern in line_patterns.splitlines(): + if not line_pattern.strip(): + continue + for sep in (': ', ':.', ':'): + header, sep, pattern = line_pattern.partition(sep) + if sep: + break # found it. + else: + # matches any header. Those are not supported. XXX + log.warning('Unsupported header_filter_rules pattern: %r', + line_pattern) + continue + header = header.strip().lstrip("^").lower() + header = header.replace('\\', '') + if not header: + log.warning('Can\'t parse the header in header_filter_rule: %r', + line_pattern) + continue + if not pattern: + # The line matched only the header, therefore the header can + # be anything. + pattern = '.*' + try: + re.compile(pattern) + except re.error: + log.warning('Skipping header_filter rule because of an ' + 'invalid regular expression: %r', line_pattern) + continue + header_matches.append((header, pattern, chain)) + mlist.header_matches = header_matches # Handle conversion to URIs. In MM2.1, the decorations are strings # containing placeholders, and there's no provision for language-specific # templates. In MM3, template locations are specified by URLs with the diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py index dd3940cdd..52d3469c0 100644 --- a/src/mailman/utilities/tests/test_import.py +++ b/src/mailman/utilities/tests/test_import.py @@ -50,6 +50,7 @@ from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.templates import ITemplateLoader from mailman.interfaces.usermanager import IUserManager from mailman.testing.layers import ConfigLayer +from mailman.testing.helpers import LogFileMark from mailman.utilities.filesystem import makedirs from mailman.utilities.importer import import_config_pck, Import21Error from mailman.utilities.string import expand @@ -330,6 +331,114 @@ class TestBasicImport(unittest.TestCase): self.assertEqual(self._mlist.subscription_policy, SubscriptionPolicy.confirm_then_moderate) + def test_header_matches(self): + # This test contail real cases of header_filter_rules + self._pckdict['header_filter_rules'] = [ + ('^X-Spam-Status: Yes', 3, False), + ('X-Spam-Status: Yes', 3, False), + ('X\\-Spam\\-Status\\: Yes.*', 3, False), + ('X-Spam-Status: Yes\r\n\r\n', 2, False), + ('^X-Spam-Level: \\*\\*\\*.*$', 3, False), + ('^X-Spam-Level:.\\*\\*\r\n^X-Spam:.\\Yes', 3, False), + ('Subject: \\[SPAM\\].*', 3, False), + ('^Subject: .*loan.*', 3, False), + ('Original-Received: from *linkedin.com*\r\n', 3, False), + ('X-Git-Module: rhq.*git', 6, False), + ('Approved: verysecretpassword', 6, False), + ('^Subject: dev-\r\n^Subject: staging-', 3, False), + ('from: .*info@aolanchem.com\r\nfrom: .*@jw-express.com', 2, False), + ('^Received: from smtp-.*\\.fedoraproject\\.org\r\n' + '^Received: from mx.*\\.redhat.com\r\n' + '^Resent-date:\r\n' + '^Resent-from:\r\n' + '^Resent-Message-ID:\r\n' + '^Resent-to:\r\n' + '^Subject: [^mtv]\r\n', + 7, False), + ('^Received: from fedorahosted\\.org.*by fedorahosted\\.org\r\n' + '^Received: from hosted.*\\.fedoraproject.org.*by ' + 'hosted.*\\.fedoraproject\\.org\r\n' + '^Received: from hosted.*\\.fedoraproject.org.*by fedoraproject\\.org\r\n' + '^Received: from hosted.*\\.fedoraproject.org.*by fedorahosted\\.org', + 6, False), + ] + error_log = LogFileMark('mailman.error') + self._import() + self.assertListEqual(self._mlist.header_matches, [ + ('x-spam-status', 'Yes', 'discard'), + ('x-spam-status', 'Yes', 'discard'), + ('x-spam-status', 'Yes.*', 'discard'), + ('x-spam-status', 'Yes', 'reject'), + ('x-spam-level', '\\*\\*\\*.*$', 'discard'), + ('x-spam-level', '\\*\\*', 'discard'), + ('x-spam', '\\Yes', 'discard'), + ('subject', '\\[SPAM\\].*', 'discard'), + ('subject', '.*loan.*', 'discard'), + ('original-received', 'from *linkedin.com*', 'discard'), + ('x-git-module', 'rhq.*git', 'accept'), + ('approved', 'verysecretpassword', 'accept'), + ('subject', 'dev-', 'discard'), + ('subject', 'staging-', 'discard'), + ('from', '.*info@aolanchem.com', 'reject'), + ('from', '.*@jw-express.com', 'reject'), + ('received', 'from smtp-.*\\.fedoraproject\\.org', 'hold'), + ('received', 'from mx.*\\.redhat.com', 'hold'), + ('resent-date', '.*', 'hold'), + ('resent-from', '.*', 'hold'), + ('resent-message-id', '.*', 'hold'), + ('resent-to', '.*', 'hold'), + ('subject', '[^mtv]', 'hold'), + ('received', 'from fedorahosted\\.org.*by fedorahosted\\.org', 'accept'), + ('received', 'from hosted.*\\.fedoraproject.org.*by hosted.*\\.fedoraproject\\.org', 'accept'), + ('received', 'from hosted.*\\.fedoraproject.org.*by fedoraproject\\.org', 'accept'), + ('received', 'from hosted.*\\.fedoraproject.org.*by fedorahosted\\.org', 'accept'), + ]) + loglines = error_log.read().strip() + self.assertEqual(len(loglines), 0) + + def test_header_matches_header_only(self): + # Check that an empty pattern is skipped + self._pckdict['header_filter_rules'] = [ + ('SomeHeaderName', 3, False), + ] + error_log = LogFileMark('mailman.error') + self._import() + self.assertListEqual(self._mlist.header_matches, []) + self.assertIn('Unsupported header_filter_rules pattern', + error_log.readline()) + + def test_header_matches_anything(self): + # Check that an empty pattern is skipped + self._pckdict['header_filter_rules'] = [ + ('.*', 7, False), + ] + error_log = LogFileMark('mailman.error') + self._import() + self.assertListEqual(self._mlist.header_matches, []) + self.assertIn('Unsupported header_filter_rules pattern', + error_log.readline()) + + def test_header_matches_invalid_re(self): + # Check that an empty pattern is skipped + self._pckdict['header_filter_rules'] = [ + ('SomeHeaderName: *invalid-re', 3, False), + ] + error_log = LogFileMark('mailman.error') + self._import() + self.assertListEqual(self._mlist.header_matches, []) + self.assertIn('Skipping header_filter rule because of an invalid ' + 'regular expression', error_log.readline()) + + def test_header_matches_defer(self): + # Check that a defer action is properly converted. + self._pckdict['header_filter_rules'] = [ + ('^X-Spam-Status: Yes', 0, False), + ] + self._import() + self.assertListEqual(self._mlist.header_matches, [ + ('x-spam-status', 'Yes', None), + ]) + class TestArchiveImport(unittest.TestCase): -- cgit v1.2.3-70-g09d2 From 741fdc63a4415cad6226e886b761eb87c3be8256 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Fri, 11 Sep 2015 14:07:23 +0200 Subject: Use a separate table for header_matches --- .../alembic/versions/42756496720_header_matches.py | 82 ++++++++++++++++++++++ src/mailman/interfaces/mailinglist.py | 20 ++++++ src/mailman/model/mailinglist.py | 21 +++++- 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 src/mailman/database/alembic/versions/42756496720_header_matches.py (limited to 'src') diff --git a/src/mailman/database/alembic/versions/42756496720_header_matches.py b/src/mailman/database/alembic/versions/42756496720_header_matches.py new file mode 100644 index 000000000..3dd896993 --- /dev/null +++ b/src/mailman/database/alembic/versions/42756496720_header_matches.py @@ -0,0 +1,82 @@ +"""header_matches + +Revision ID: 42756496720 +Revises: 2bb9b382198 +Create Date: 2015-09-11 10:11:38.310315 + +""" + +# revision identifiers, used by Alembic. +revision = '42756496720' +down_revision = '2bb9b382198' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + # Create the new table + header_matches_table = op.create_table('headermatches', + 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('chain', sa.Unicode(), nullable=True), + sa.ForeignKeyConstraint(['mailing_list_id'], ['mailinglist.id'], ), + sa.PrimaryKeyConstraint('id') + ) + + # Now migrate the data. It can't be offline because we need to read the + # pickles. + connection = op.get_bind() + # Don't import the table definition from the models, it may break this + # migration when the model is updated in the future (see the Alembic doc). + mlist_table = sa.sql.table('mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('header_matches', sa.PickleType) + ) + for mlist_id, old_matches in connection.execute(mlist_table.select()): + for old_match in old_matches: + connection.execute(header_matches_table.insert().values( + mailing_list_id=mlist_id, + header=old_match[0], + pattern=old_match[1], + chain=None + )) + + # Now that data is migrated, drop the old column (except on SQLite which + # does not support this) + if op.get_bind().dialect.name != 'sqlite': + op.drop_column('mailinglist', 'header_matches') + + +def downgrade(): + if op.get_bind().dialect.name != 'sqlite': + # SQLite will not have deleted the former column, since it does not + # support column deletion. + op.add_column('mailinglist', sa.Column( + 'header_matches', sa.PickleType, nullable=True)) + + # Now migrate the data. It can't be offline because we need to read the + # pickles. + connection = op.get_bind() + # Don't import the table definition from the models, it may break this + # migration when the model is updated in the future (see the Alembic doc). + mlist_table = sa.sql.table('mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('header_matches', sa.PickleType) + ) + header_matches_table = sa.sql.table('headermatches', + sa.sql.column('mailing_list_id', sa.Integer), + sa.sql.column('header', sa.Unicode), + sa.sql.column('pattern', sa.Unicode), + ) + for mlist_id, header, pattern in connection.execute( + header_matches_table.select()): + mlist = connection.execute(mlist_table.select().where( + mlist_table.c.id == mlist_id)).fetchone() + if not mlist["header_matches"]: + mlist["header_matches"] = [] + mlist["header_matches"].append((header, pattern)) + + op.drop_table('headermatches') diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index f112b2a11..c69d04771 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -839,3 +839,23 @@ class IListArchiverSet(Interface): :return: the matching `IListArchiver` or None if the named archiver does not exist. """ + + + +class IHeaderMatches(Interface): + """Registration record for a single bounce event.""" + + mailing_list = Attribute( + """The mailing list for the header match.""") + + header = Attribute( + """The email header that will be checked.""") + + pattern = Attribute( + """The regular expression to match.""") + + chain = Attribute( + """The chain to jump to on a match. + + If it is None, the configuration file action for spam is used. + """) diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index f04c534e1..33cc2b60a 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -38,7 +38,8 @@ from mailman.interfaces.domain import IDomainManager from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( IAcceptableAlias, IAcceptableAliasSet, IListArchiver, IListArchiverSet, - IMailingList, Personalization, ReplyToMunging, SubscriptionPolicy) + IHeaderMatches, IMailingList, Personalization, ReplyToMunging, + SubscriptionPolicy) from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError, SubscriptionEvent) @@ -149,7 +150,6 @@ class MailingList(Model): gateway_to_mail = Column(Boolean) gateway_to_news = Column(Boolean) goodbye_message_uri = Column(Unicode) - header_matches = Column(PickleType) header_uri = Column(Unicode) hold_these_nonmembers = Column(PickleType) info = Column(Unicode) @@ -621,3 +621,20 @@ class ListArchiverSet: return store.query(ListArchiver).filter( ListArchiver.mailing_list == self._mailing_list, ListArchiver.name == archiver_name).first() + + + +@implementer(IHeaderMatches) +class HeaderMatches(Model): + """See `IHeaderMatches`.""" + + __tablename__ = 'headermatches' + + id = Column(Integer, primary_key=True) + + 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) + chain = Column(Unicode, nullable=True) -- cgit v1.2.3-70-g09d2 From 583a7639eb78dc52cf899076fef777e303101567 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Fri, 11 Sep 2015 14:15:28 +0200 Subject: Rename from plural to singular --- .../database/alembic/versions/42756496720_header_matches.py | 10 +++++----- src/mailman/interfaces/mailinglist.py | 4 ++-- src/mailman/model/mailinglist.py | 10 +++++----- 3 files changed, 12 insertions(+), 12 deletions(-) (limited to 'src') diff --git a/src/mailman/database/alembic/versions/42756496720_header_matches.py b/src/mailman/database/alembic/versions/42756496720_header_matches.py index 3dd896993..ae3463286 100644 --- a/src/mailman/database/alembic/versions/42756496720_header_matches.py +++ b/src/mailman/database/alembic/versions/42756496720_header_matches.py @@ -16,7 +16,7 @@ import sqlalchemy as sa def upgrade(): # Create the new table - header_matches_table = op.create_table('headermatches', + 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), @@ -37,7 +37,7 @@ def upgrade(): ) for mlist_id, old_matches in connection.execute(mlist_table.select()): for old_match in old_matches: - connection.execute(header_matches_table.insert().values( + connection.execute(header_match_table.insert().values( mailing_list_id=mlist_id, header=old_match[0], pattern=old_match[1], @@ -66,17 +66,17 @@ def downgrade(): sa.sql.column('id', sa.Integer), sa.sql.column('header_matches', sa.PickleType) ) - header_matches_table = sa.sql.table('headermatches', + header_match_table = sa.sql.table('headermatch', sa.sql.column('mailing_list_id', sa.Integer), sa.sql.column('header', sa.Unicode), sa.sql.column('pattern', sa.Unicode), ) for mlist_id, header, pattern in connection.execute( - header_matches_table.select()): + header_match_table.select()): mlist = connection.execute(mlist_table.select().where( mlist_table.c.id == mlist_id)).fetchone() if not mlist["header_matches"]: mlist["header_matches"] = [] mlist["header_matches"].append((header, pattern)) - op.drop_table('headermatches') + op.drop_table('headermatch') diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index c69d04771..1db5f0dae 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -842,8 +842,8 @@ class IListArchiverSet(Interface): -class IHeaderMatches(Interface): - """Registration record for a single bounce event.""" +class IHeaderMatch(Interface): + """A header matching rule for mailinglist messages.""" mailing_list = Attribute( """The mailing list for the header match.""") diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 33cc2b60a..c9f5e8ef1 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -38,7 +38,7 @@ from mailman.interfaces.domain import IDomainManager from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( IAcceptableAlias, IAcceptableAliasSet, IListArchiver, IListArchiverSet, - IHeaderMatches, IMailingList, Personalization, ReplyToMunging, + IHeaderMatch, IMailingList, Personalization, ReplyToMunging, SubscriptionPolicy) from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError, @@ -624,11 +624,11 @@ class ListArchiverSet: -@implementer(IHeaderMatches) -class HeaderMatches(Model): - """See `IHeaderMatches`.""" +@implementer(IHeaderMatch) +class HeaderMatch(Model): + """See `IHeaderMatch`.""" - __tablename__ = 'headermatches' + __tablename__ = 'headermatch' id = Column(Integer, primary_key=True) -- cgit v1.2.3-70-g09d2 From ed772e4fe2296460f0261a114b4f4eea3b318d6a Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Fri, 11 Sep 2015 14:31:41 +0200 Subject: Adapt the code and the tests to the new HeaderMatch object --- src/mailman/chains/headers.py | 2 +- src/mailman/chains/tests/test_headers.py | 9 +++++---- src/mailman/rules/docs/header-matching.rst | 5 ++++- src/mailman/utilities/importer.py | 6 +++--- src/mailman/utilities/tests/test_import.py | 13 +++++++++---- 5 files changed, 22 insertions(+), 13 deletions(-) (limited to 'src') diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py index ea4652062..c98726e21 100644 --- a/src/mailman/chains/headers.py +++ b/src/mailman/chains/headers.py @@ -143,7 +143,7 @@ class HeaderMatchChain(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) + yield make_link(entry.header, entry.pattern, entry.chain) # Then return all the explicitly added links. for link in self._extended_links: yield link diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py index cca2e041f..c55d39876 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -28,6 +28,7 @@ from mailman.app.lifecycle import create_list from mailman.chains.headers import HeaderMatchRule from mailman.config import config from mailman.email.message import Message +from mailman.model.mailinglist import HeaderMatch from mailman.interfaces.chain import LinkAction from mailman.testing.layers import ConfigLayer from mailman.testing.helpers import LogFileMark, configuration @@ -124,7 +125,7 @@ class TestHeaderChain(unittest.TestCase): # Test that the header-match chain has the header checks from the # mailing-list configuration. chain = config.chains['header-match'] - self._mlist.header_matches = [('Foo', 'a+')] + 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' ] self.assertEqual(len(links), 1) @@ -137,9 +138,9 @@ class TestHeaderChain(unittest.TestCase): # properly. chain = config.chains['header-match'] self._mlist.header_matches = [ - ('Foo', 'a+', 'reject'), - ('Bar', 'b+', 'discard'), - ('Baz', 'z+', 'accept'), + HeaderMatch(header='Foo', pattern='a+', chain='reject'), + 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' ] diff --git a/src/mailman/rules/docs/header-matching.rst b/src/mailman/rules/docs/header-matching.rst index 3c175e6e1..7b2d8c6d7 100644 --- a/src/mailman/rules/docs/header-matching.rst +++ b/src/mailman/rules/docs/header-matching.rst @@ -125,7 +125,10 @@ with the same semantics as the global `[antispam]` section. The list administrator wants to match not on four stars, but on three plus signs, but only for the current mailing list. - >>> mlist.header_matches = [('x-spam-score', '[+]{3,}')] + >>> from mailman.model.mailinglist import HeaderMatch + >>> mlist.header_matches = [ + ... HeaderMatch(header='x-spam-score', pattern='[+]{3,}') + ... ] A message with a spam score of two pluses does not match. diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py index df0557c08..1261823fd 100644 --- a/src/mailman/utilities/importer.py +++ b/src/mailman/utilities/importer.py @@ -48,6 +48,7 @@ from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import DeliveryMode, DeliveryStatus, MemberRole from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.usermanager import IUserManager +from mailman.model.mailinglist import HeaderMatch from mailman.utilities.filesystem import makedirs from mailman.utilities.i18n import search from sqlalchemy import Boolean @@ -334,7 +335,6 @@ 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 - header_matches = [] for line_patterns, action, _unused in \ config_dict.get('header_filter_rules', []): chain = action_to_chain(action) @@ -367,8 +367,8 @@ def import_config_pck(mlist, config_dict): log.warning('Skipping header_filter rule because of an ' 'invalid regular expression: %r', line_pattern) continue - header_matches.append((header, pattern, chain)) - mlist.header_matches = header_matches + mlist.header_matches.append(HeaderMatch( + header=header, pattern=pattern, chain=chain)) # Handle conversion to URIs. In MM2.1, the decorations are strings # containing placeholders, and there's no provision for language-specific # templates. In MM3, template locations are specified by URLs with the diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py index 52d3469c0..e9ad29a1b 100644 --- a/src/mailman/utilities/tests/test_import.py +++ b/src/mailman/utilities/tests/test_import.py @@ -49,6 +49,7 @@ from mailman.interfaces.member import DeliveryMode, DeliveryStatus 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.utilities.filesystem import makedirs @@ -364,7 +365,9 @@ class TestBasicImport(unittest.TestCase): ] error_log = LogFileMark('mailman.error') self._import() - self.assertListEqual(self._mlist.header_matches, [ + self.assertListEqual( + [ (hm.header, hm.pattern, hm.chain) + for hm in self._mlist.header_matches ], [ ('x-spam-status', 'Yes', 'discard'), ('x-spam-status', 'Yes', 'discard'), ('x-spam-status', 'Yes.*', 'discard'), @@ -435,9 +438,11 @@ class TestBasicImport(unittest.TestCase): ('^X-Spam-Status: Yes', 0, False), ] self._import() - self.assertListEqual(self._mlist.header_matches, [ - ('x-spam-status', 'Yes', None), - ]) + self.assertListEqual( + [ (hm.header, hm.pattern, hm.chain) + for hm in self._mlist.header_matches ], + [ ('x-spam-status', 'Yes', None) ] + ) -- cgit v1.2.3-70-g09d2 From d468d096b35e42f8450a5ae449501ea155992a95 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Fri, 11 Sep 2015 16:09:08 +0200 Subject: Make sure site-wide header_matches take precedence over list-specific ones --- src/mailman/chains/headers.py | 13 ++++++----- src/mailman/chains/tests/test_headers.py | 40 ++++++++++++++++++++++++++++++-- 2 files changed, 45 insertions(+), 8 deletions(-) (limited to 'src') diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py index c98726e21..b696bcfe7 100644 --- a/src/mailman/chains/headers.py +++ b/src/mailman/chains/headers.py @@ -140,14 +140,15 @@ class HeaderMatchChain(Chain): 'contains bogus line: {0}'.format(line)) continue yield make_link(parts[0], parts[1].lstrip()) - # 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) # Then return all the explicitly added links. for link in self._extended_links: yield link - # Finally, if any of the above rules matched, jump to the chain - # defined in the configuration file. + # 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 c55d39876..104913034 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -29,9 +29,12 @@ from mailman.chains.headers import HeaderMatchRule from mailman.config import config from mailman.email.message import Message from mailman.model.mailinglist import HeaderMatch -from mailman.interfaces.chain import LinkAction +from mailman.interfaces.chain import LinkAction, HoldEvent +from mailman.core.chains import process from mailman.testing.layers import ConfigLayer -from mailman.testing.helpers import LogFileMark, configuration +from mailman.testing.helpers import (LogFileMark, configuration, + event_subscribers, get_queue_messages, + specialized_message_from_string as mfs) @@ -152,3 +155,36 @@ class TestHeaderChain(unittest.TestCase): ('Bar', 'b+', LinkAction.jump, 'discard'), ('Baz', 'z+', LinkAction.jump, 'accept'), ]) + + @configuration('antispam', header_checks=""" + Foo: foo + """, jump_chain="hold") + def test_priority_site_over_list(self): + # Test that the site-wide checks take precedence over the list-specific + # checks. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: A message +Message-ID: +Foo: foo +MIME-Version: 1.0 + +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): + process(self._mlist, msg, msgdata, start_chain='header-match') + self.assertEqual(len(events), 1) + event = events[0] + # Site-wide wants to hold the message, the list wants to accept it. + self.assertTrue(isinstance(event, HoldEvent)) + self.assertEqual(event.chain, config.chains['hold']) -- cgit v1.2.3-70-g09d2 From b9baf43abd023b74d92aa0efa31b45d97111394a Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Sat, 12 Sep 2015 13:20:58 +0200 Subject: Implement changes from the review --- src/mailman/chains/headers.py | 12 +++----- src/mailman/chains/tests/test_headers.py | 28 +++++++++--------- .../alembic/versions/42756496720_header_matches.py | 4 +-- src/mailman/interfaces/mailinglist.py | 3 +- src/mailman/model/mailinglist.py | 4 +-- src/mailman/rules/docs/header-matching.rst | 33 ++++++++++++++++++++-- src/mailman/utilities/importer.py | 15 ++++++---- src/mailman/utilities/tests/test_import.py | 19 ++++++++++++- 8 files changed, 82 insertions(+), 36 deletions(-) (limited to 'src') 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'] = '' + >>> with event_subscribers(handler): + ... process(mlist, msg, msgdata, 'header-match') + DiscardEvent discard + >>> 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): -- cgit v1.2.3-70-g09d2 From e2f6b111f0bbf2287fbf793cd6f09fb829ee9758 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mon, 14 Sep 2015 10:25:25 +0200 Subject: Fix a code typo in the docs --- src/mailman/rules/docs/header-matching.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/mailman/rules/docs/header-matching.rst b/src/mailman/rules/docs/header-matching.rst index e3f9f9ab2..c552ea521 100644 --- a/src/mailman/rules/docs/header-matching.rst +++ b/src/mailman/rules/docs/header-matching.rst @@ -179,7 +179,7 @@ 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') + ... HeaderMatch(header='x-spam-score', pattern='[+]{3,}', chain='discard') ... ] A message with a spam score of three pluses will still match, and the message -- cgit v1.2.3-70-g09d2 From 87f2f50b08eb0a7b0a99924b82fd246a6ce10983 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Mon, 14 Sep 2015 11:16:45 +0200 Subject: Use and interface for the set of header_matches --- src/mailman/chains/tests/test_headers.py | 27 ++++++++-------- src/mailman/config/configure.zcml | 6 ++++ src/mailman/interfaces/mailinglist.py | 31 ++++++++++++++++++ src/mailman/model/mailinglist.py | 50 +++++++++++++++++++++++++++-- src/mailman/model/tests/test_mailinglist.py | 46 +++++++++++++++++++++++++- src/mailman/rules/docs/header-matching.rst | 12 +++---- src/mailman/utilities/importer.py | 12 ++++--- src/mailman/utilities/tests/test_import.py | 25 +++++++++++---- 8 files changed, 174 insertions(+), 35 deletions(-) (limited to 'src') diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py index 636eec196..851720f95 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -30,7 +30,7 @@ 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.model.mailinglist import HeaderMatch +from mailman.interfaces.mailinglist import IHeaderMatchSet from mailman.testing.layers import ConfigLayer from mailman.testing.helpers import ( configuration, event_subscribers, get_queue_messages, LogFileMark, @@ -128,32 +128,32 @@ class TestHeaderChain(unittest.TestCase): # Test that the header-match chain has the header checks from the # mailing-list configuration. chain = config.chains['header-match'] - self._mlist.header_matches = [HeaderMatch(header='Foo', pattern='a+')] + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Foo', 'a+', None) 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') + self.assertEqual(links[0].rule.header, 'foo') self.assertEqual(links[0].rule.pattern, 'a+') def test_list_complex_rule(self): # Test that the mailing-list header-match complex rules are read # properly. chain = config.chains['header-match'] - self._mlist.header_matches = [ - HeaderMatch(header='Foo', pattern='a+', chain='reject'), - HeaderMatch(header='Bar', pattern='b+', chain='discard'), - HeaderMatch(header='Baz', pattern='z+', chain='accept'), - ] + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Foo', 'a+', 'reject') + 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'] self.assertEqual(len(links), 3) self.assertListEqual( [(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'), + [('foo', 'a+', LinkAction.jump, 'reject'), + ('bar', 'b+', LinkAction.jump, 'discard'), + ('baz', 'z+', LinkAction.jump, 'accept'), ]) @configuration('antispam', header_checks=""" @@ -173,9 +173,8 @@ MIME-Version: 1.0 A message body. """) msgdata = {} - self._mlist.header_matches = [ - HeaderMatch(header='Foo', pattern='foo', chain='accept') - ] + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Foo', 'foo', 'accept') # This event subscriber records the event that occurs when the message # is processed by the owner chain. events = [] diff --git a/src/mailman/config/configure.zcml b/src/mailman/config/configure.zcml index 632771d42..a8fa2c119 100644 --- a/src/mailman/config/configure.zcml +++ b/src/mailman/config/configure.zcml @@ -34,6 +34,12 @@ factory="mailman.model.mailinglist.ListArchiverSet" /> + + 0: + raise ValueError('Pattern already exists') + header_match = HeaderMatch( + mailing_list=self._mailing_list, + header=header, pattern=pattern, chain=chain) + store.add(header_match) + + @dbconnection + def remove(self, store, header, pattern): + header = header.lower() + # Don't just filter and use delete(), or the MailingList.header_matches + # collection will not be updated: + # http://docs.sqlalchemy.org/en/rel_1_0/orm/collections.html#dynamic-relationship-loaders + try: + existing = store.query(HeaderMatch).filter( + HeaderMatch.mailing_list == self._mailing_list, + HeaderMatch.header == header, + HeaderMatch.pattern == pattern).one() + except NoResultFound: + raise ValueError('Pattern does not exist') + else: + self._mailing_list.header_matches.remove(existing) diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py index 745096b4b..c497cb474 100644 --- a/src/mailman/model/tests/test_mailinglist.py +++ b/src/mailman/model/tests/test_mailinglist.py @@ -32,7 +32,7 @@ from mailman.config import config from mailman.database.transaction import transaction from mailman.interfaces.listmanager import IListManager from mailman.interfaces.mailinglist import ( - IAcceptableAliasSet, IListArchiverSet) + IAcceptableAliasSet, IHeaderMatchSet, IListArchiverSet) from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError) from mailman.interfaces.usermanager import IUserManager @@ -163,3 +163,47 @@ class TestAcceptableAliases(unittest.TestCase): self.assertEqual(['bee@example.com'], list(alias_set.aliases)) getUtility(IListManager).delete(self._mlist) self.assertEqual(len(list(alias_set.aliases)), 0) + + + +class TestHeaderMatch(unittest.TestCase): + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('ant@example.com') + + def test_lowercase_header(self): + with transaction(): + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + self.assertEqual(self._mlist.header_matches[0].header, 'header') + + def test_chain_defaults_to_none(self): + with transaction(): + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + self.assertEqual(self._mlist.header_matches[0].chain, None) + + def test_duplicate(self): + with transaction(): + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Header', 'pattern') + self.assertRaises(ValueError, + header_matches.add, 'Header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + + def test_remove_non_existent(self): + with transaction(): + header_matches = IHeaderMatchSet(self._mlist) + self.assertRaises(ValueError, + header_matches.remove, 'header', 'pattern') + + def test_add_remove(self): + with transaction(): + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + header_matches.remove('header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 0) diff --git a/src/mailman/rules/docs/header-matching.rst b/src/mailman/rules/docs/header-matching.rst index c552ea521..05d01efb2 100644 --- a/src/mailman/rules/docs/header-matching.rst +++ b/src/mailman/rules/docs/header-matching.rst @@ -131,10 +131,9 @@ action. The list administrator wants to match not on four stars, but on three plus signs, but only for the current mailing list. - >>> from mailman.model.mailinglist import HeaderMatch - >>> mlist.header_matches = [ - ... HeaderMatch(header='x-spam-score', pattern='[+]{3,}') - ... ] + >>> from mailman.interfaces.mailinglist import IHeaderMatchSet + >>> header_matches = IHeaderMatchSet(mlist) + >>> header_matches.add('x-spam-score', '[+]{3,}') A message with a spam score of two pluses does not match. @@ -178,9 +177,8 @@ As does a message with a spam score of four pluses. 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,}', chain='discard') - ... ] + >>> header_matches.remove('x-spam-score', '[+]{3,}') + >>> header_matches.add('x-spam-score', '[+]{3,}', 'discard') A message with a spam score of three pluses will still match, and the message will be discarded. diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py index f767ddbe3..4e3eab6cf 100644 --- a/src/mailman/utilities/importer.py +++ b/src/mailman/utilities/importer.py @@ -42,13 +42,12 @@ from mailman.interfaces.bounce import UnrecognizedBounceDisposition 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 IAcceptableAliasSet, IHeaderMatchSet from mailman.interfaces.mailinglist import Personalization, ReplyToMunging from mailman.interfaces.mailinglist import SubscriptionPolicy from mailman.interfaces.member import DeliveryMode, DeliveryStatus, MemberRole from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.usermanager import IUserManager -from mailman.model.mailinglist import HeaderMatch from mailman.utilities.filesystem import makedirs from mailman.utilities.i18n import search from sqlalchemy import Boolean @@ -335,6 +334,7 @@ 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 + header_match_set = IHeaderMatchSet(mlist) header_filter_rules = config_dict.get('header_filter_rules', []) for line_patterns, action, _unused in header_filter_rules: try: @@ -372,8 +372,12 @@ def import_config_pck(mlist, config_dict): log.warning('Skipping header_filter rule because of an ' 'invalid regular expression: %r', line_pattern) continue - mlist.header_matches.append(HeaderMatch( - header=header, pattern=pattern, chain=chain)) + try: + header_match_set.add(header, pattern, chain) + except ValueError: + log.warning('Skipping duplicate header_filter rule: %r', + line_pattern) + continue # Handle conversion to URIs. In MM2.1, the decorations are strings # containing placeholders, and there's no provision for language-specific # templates. In MM3, template locations are specified by URLs with the diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py index 85ee993ba..64f8e061f 100644 --- a/src/mailman/utilities/tests/test_import.py +++ b/src/mailman/utilities/tests/test_import.py @@ -44,12 +44,11 @@ from mailman.interfaces.bans import IBanManager from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( - IAcceptableAliasSet, SubscriptionPolicy) + IAcceptableAliasSet, IHeaderMatchSet, SubscriptionPolicy) from mailman.interfaces.member import DeliveryMode, DeliveryStatus 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.helpers import LogFileMark from mailman.testing.layers import ConfigLayer from mailman.utilities.filesystem import makedirs @@ -335,10 +334,8 @@ class TestBasicImport(unittest.TestCase): def test_header_matches(self): # This test contail real cases of header_filter_rules self._pckdict['header_filter_rules'] = [ - ('^X-Spam-Status: Yes', 3, False), - ('X-Spam-Status: Yes', 3, False), ('X\\-Spam\\-Status\\: Yes.*', 3, False), - ('X-Spam-Status: Yes\r\n\r\n', 2, False), + ('^X-Spam-Status: Yes\r\n\r\n', 2, False), ('^X-Spam-Level: \\*\\*\\*.*$', 3, False), ('^X-Spam-Level:.\\*\\*\r\n^X-Spam:.\\Yes', 3, False), ('Subject: \\[SPAM\\].*', 3, False), @@ -368,8 +365,6 @@ class TestBasicImport(unittest.TestCase): self.assertListEqual( [ (hm.header, hm.pattern, hm.chain) for hm in self._mlist.header_matches ], [ - ('x-spam-status', 'Yes', 'discard'), - ('x-spam-status', 'Yes', 'discard'), ('x-spam-status', 'Yes.*', 'discard'), ('x-spam-status', 'Yes', 'reject'), ('x-spam-level', '\\*\\*\\*.*$', 'discard'), @@ -461,6 +456,22 @@ class TestBasicImport(unittest.TestCase): for member in self._mlist.owners.members: member.unsubscribe() + def test_header_matches_duplicate(self): + # Check that duplicate patterns don't cause tracebacks + self._pckdict['header_filter_rules'] = [ + ('SomeHeaderName: test-pattern', 3, False), + ('SomeHeaderName: test-pattern', 2, False), + ] + error_log = LogFileMark('mailman.error') + self._import() + self.assertListEqual( + [ (hm.header, hm.pattern, hm.chain) + for hm in self._mlist.header_matches ], + [ ('someheadername', 'test-pattern', 'discard') ] + ) + self.assertIn('Skipping duplicate header_filter rule', + error_log.readline()) + class TestArchiveImport(unittest.TestCase): -- cgit v1.2.3-70-g09d2 From 0fcb68ac1b88801a9679e00a82d0bdabf86d19d1 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Tue, 15 Sep 2015 11:27:04 +0200 Subject: Test schema migration for the header matches --- .../alembic/versions/42756496720_header_matches.py | 20 ++++++---- src/mailman/database/tests/test_migrations.py | 44 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 7 deletions(-) (limited to 'src') diff --git a/src/mailman/database/alembic/versions/42756496720_header_matches.py b/src/mailman/database/alembic/versions/42756496720_header_matches.py index f79b02b86..83a1275a2 100644 --- a/src/mailman/database/alembic/versions/42756496720_header_matches.py +++ b/src/mailman/database/alembic/versions/42756496720_header_matches.py @@ -10,9 +10,11 @@ Create Date: 2015-09-11 10:11:38.310315 revision = '42756496720' down_revision = '2bb9b382198' -from alembic import op import sqlalchemy as sa +from alembic import op +from mailman.database.helpers import is_sqlite, exists_in_db + def upgrade(): # Create the new table @@ -46,12 +48,12 @@ def upgrade(): # Now that data is migrated, drop the old column (except on SQLite which # does not support this) - if op.get_bind().dialect.name != 'sqlite': + if not is_sqlite(connection): op.drop_column('mailinglist', 'header_matches') def downgrade(): - if op.get_bind().dialect.name != 'sqlite': + if not exists_in_db(op.get_bind(), 'mailinglist', 'header_matches'): # SQLite will not have deleted the former column, since it does not # support column deletion. op.add_column('mailinglist', sa.Column( @@ -72,11 +74,15 @@ def downgrade(): sa.sql.column('pattern', sa.Unicode), ) for mlist_id, header, pattern in connection.execute( - header_match_table.select()): + header_match_table.select()).fetchall(): mlist = connection.execute(mlist_table.select().where( mlist_table.c.id == mlist_id)).fetchone() - if not mlist["header_matches"]: - mlist["header_matches"] = [] - mlist["header_matches"].append((header, pattern)) + header_matches = mlist['header_matches'] + if not header_matches: + header_matches = [] + header_matches.append((header, pattern)) + connection.execute(mlist_table.update().where( + mlist_table.c.id == mlist_id).values( + header_matches=header_matches)) op.drop_table('headermatch') diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index 98d8deb79..d54c525ea 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -28,6 +28,7 @@ import sqlalchemy as sa from mailman.config import config from mailman.database.alembic import alembic_cfg +from mailman.database.helpers import exists_in_db from mailman.database.model import Model from mailman.testing.layers import ConfigLayer @@ -41,8 +42,17 @@ class TestMigrations(unittest.TestCase): def tearDown(self): # Drop and restore a virgin database. + config.db.store.rollback() md = sa.MetaData(bind=config.db.engine) md.reflect() + # We have circular dependencies between user and address, thus we can't + # use drop_all() without getting a warning. Setting use_alter to True + # on the foreign keys helps SQLAlchemy mark those loops as known. + for tablename in ("user", "address"): + if tablename not in md.tables: + continue + for fk in md.tables[tablename].foreign_key_constraints: + fk.use_alter = True md.drop_all() Model.metadata.create_all(config.db.engine) @@ -55,3 +65,37 @@ class TestMigrations(unittest.TestCase): revisions.reverse() for revision in revisions: alembic.command.upgrade(alembic_cfg, revision) + + def test_42756496720_header_matches(self): + test_header_matches = [ + ('test-header-1', 'test-pattern-1'), + ('test-header-2', 'test-pattern-2'), + ('test-header-3', 'test-pattern-3'), + ] + mlist_table = sa.sql.table('mailinglist', + sa.sql.column('id', sa.Integer), + sa.sql.column('header_matches', sa.PickleType) + ) + header_match_table = sa.sql.table('headermatch', + sa.sql.column('mailing_list_id', sa.Integer), + sa.sql.column('header', sa.Unicode), + sa.sql.column('pattern', sa.Unicode), + ) + # Downgrading + config.db.store.execute(mlist_table.insert().values(id=1)) + config.db.store.execute(header_match_table.insert().values( + [{'mailing_list_id': 1, 'header': hm[0], 'pattern': hm[1]} + for hm in test_header_matches])) + config.db.store.commit() + alembic.command.downgrade(alembic_cfg, '2bb9b382198') + results = config.db.store.execute( + mlist_table.select()).fetchall() + self.assertEqual(results[0].header_matches, test_header_matches) + self.assertFalse(exists_in_db(config.db.engine, 'headermatch')) + config.db.store.commit() + # Upgrading + alembic.command.upgrade(alembic_cfg, '42756496720') + results = config.db.store.execute( + header_match_table.select()).fetchall() + self.assertEqual(results, + [(1, hm[0], hm[1]) for hm in test_header_matches]) -- cgit v1.2.3-70-g09d2 From adb9e3164ee1e19802a4373e76b42b1435d1e687 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Wed, 16 Sep 2015 15:25:36 +0200 Subject: Be compatible with older versions of SQLAlchemy and Alembic --- src/mailman/database/tests/test_migrations.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'src') diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index d54c525ea..6f167cd13 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -51,15 +51,14 @@ class TestMigrations(unittest.TestCase): for tablename in ("user", "address"): if tablename not in md.tables: continue - for fk in md.tables[tablename].foreign_key_constraints: - fk.use_alter = True + for fk in md.tables[tablename].foreign_keys: + fk.constraint.use_alter = True md.drop_all() Model.metadata.create_all(config.db.engine) def test_all_migrations(self): script_dir = alembic.script.ScriptDirectory.from_config(alembic_cfg) - revisions = [sc.revision for sc in - script_dir.walk_revisions('base', 'heads')] + revisions = [sc.revision for sc in script_dir.walk_revisions()] for revision in revisions: alembic.command.downgrade(alembic_cfg, revision) revisions.reverse() -- cgit v1.2.3-70-g09d2 From 5f917c8aec735608cb85195e691fb68fbb835af7 Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 20 Oct 2015 22:48:59 -0400 Subject: Clean up pass through abompard's branch. --- src/mailman/chains/headers.py | 24 +++++--- src/mailman/chains/tests/test_headers.py | 37 +++++++++--- .../alembic/versions/42756496720_header_matches.py | 35 ++++++------ src/mailman/database/tests/test_migrations.py | 18 +++--- src/mailman/interfaces/mailinglist.py | 25 +++++--- src/mailman/model/mailinglist.py | 9 ++- src/mailman/model/tests/test_mailinglist.py | 63 ++++++++++++--------- src/mailman/rules/docs/header-matching.rst | 16 +++--- src/mailman/utilities/importer.py | 41 +++++++------- src/mailman/utilities/tests/test_import.py | 66 +++++++++++++--------- 10 files changed, 198 insertions(+), 136 deletions(-) (limited to 'src') 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'), diff --git a/src/mailman/database/alembic/versions/42756496720_header_matches.py b/src/mailman/database/alembic/versions/42756496720_header_matches.py index 83a1275a2..5e8db9756 100644 --- a/src/mailman/database/alembic/versions/42756496720_header_matches.py +++ b/src/mailman/database/alembic/versions/42756496720_header_matches.py @@ -18,7 +18,8 @@ from mailman.database.helpers import is_sqlite, exists_in_db def upgrade(): # Create the new table - header_match_table = op.create_table('headermatch', + 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=False), @@ -26,17 +27,17 @@ def upgrade(): sa.Column('chain', sa.Unicode(), nullable=True), sa.ForeignKeyConstraint(['mailing_list_id'], ['mailinglist.id'], ), sa.PrimaryKeyConstraint('id') - ) - - # Now migrate the data. It can't be offline because we need to read the + ) + # Now migrate the data. It can't be offline because we need to read the # pickles. connection = op.get_bind() # Don't import the table definition from the models, it may break this # migration when the model is updated in the future (see the Alembic doc). - mlist_table = sa.sql.table('mailinglist', + mlist_table = sa.sql.table( + 'mailinglist', sa.sql.column('id', sa.Integer), sa.sql.column('header_matches', sa.PickleType) - ) + ) for mlist_id, old_matches in connection.execute(mlist_table.select()): for old_match in old_matches: connection.execute(header_match_table.insert().values( @@ -44,8 +45,7 @@ def upgrade(): header=old_match[0], pattern=old_match[1], chain=None - )) - + )) # Now that data is migrated, drop the old column (except on SQLite which # does not support this) if not is_sqlite(connection): @@ -56,23 +56,25 @@ def downgrade(): if not exists_in_db(op.get_bind(), 'mailinglist', 'header_matches'): # SQLite will not have deleted the former column, since it does not # support column deletion. - op.add_column('mailinglist', sa.Column( - 'header_matches', sa.PickleType, nullable=True)) - - # Now migrate the data. It can't be offline because we need to read the + op.add_column( + 'mailinglist', + sa.Column('header_matches', sa.PickleType, nullable=True)) + # Now migrate the data. It can't be offline because we need to read the # pickles. connection = op.get_bind() # Don't import the table definition from the models, it may break this # migration when the model is updated in the future (see the Alembic doc). - mlist_table = sa.sql.table('mailinglist', + mlist_table = sa.sql.table( + 'mailinglist', sa.sql.column('id', sa.Integer), sa.sql.column('header_matches', sa.PickleType) - ) - header_match_table = sa.sql.table('headermatch', + ) + header_match_table = sa.sql.table( + 'headermatch', sa.sql.column('mailing_list_id', sa.Integer), sa.sql.column('header', sa.Unicode), sa.sql.column('pattern', sa.Unicode), - ) + ) for mlist_id, header, pattern in connection.execute( header_match_table.select()).fetchall(): mlist = connection.execute(mlist_table.select().where( @@ -84,5 +86,4 @@ def downgrade(): connection.execute(mlist_table.update().where( mlist_table.c.id == mlist_id).values( header_matches=header_matches)) - op.drop_table('headermatch') diff --git a/src/mailman/database/tests/test_migrations.py b/src/mailman/database/tests/test_migrations.py index 6f167cd13..91f19bfb2 100644 --- a/src/mailman/database/tests/test_migrations.py +++ b/src/mailman/database/tests/test_migrations.py @@ -46,9 +46,9 @@ class TestMigrations(unittest.TestCase): md = sa.MetaData(bind=config.db.engine) md.reflect() # We have circular dependencies between user and address, thus we can't - # use drop_all() without getting a warning. Setting use_alter to True + # use drop_all() without getting a warning. Setting use_alter to True # on the foreign keys helps SQLAlchemy mark those loops as known. - for tablename in ("user", "address"): + for tablename in ('user', 'address'): if tablename not in md.tables: continue for fk in md.tables[tablename].foreign_keys: @@ -70,17 +70,19 @@ class TestMigrations(unittest.TestCase): ('test-header-1', 'test-pattern-1'), ('test-header-2', 'test-pattern-2'), ('test-header-3', 'test-pattern-3'), - ] - mlist_table = sa.sql.table('mailinglist', + ] + mlist_table = sa.sql.table( + 'mailinglist', sa.sql.column('id', sa.Integer), sa.sql.column('header_matches', sa.PickleType) ) - header_match_table = sa.sql.table('headermatch', + header_match_table = sa.sql.table( + 'headermatch', sa.sql.column('mailing_list_id', sa.Integer), sa.sql.column('header', sa.Unicode), sa.sql.column('pattern', sa.Unicode), - ) - # Downgrading + ) + # Downgrading. config.db.store.execute(mlist_table.insert().values(id=1)) config.db.store.execute(header_match_table.insert().values( [{'mailing_list_id': 1, 'header': hm[0], 'pattern': hm[1]} @@ -92,7 +94,7 @@ class TestMigrations(unittest.TestCase): self.assertEqual(results[0].header_matches, test_header_matches) self.assertFalse(exists_in_db(config.db.engine, 'headermatch')) config.db.store.commit() - # Upgrading + # Upgrading. alembic.command.upgrade(alembic_cfg, '42756496720') results = config.db.store.execute( header_match_table.select()).fetchall() diff --git a/src/mailman/interfaces/mailinglist.py b/src/mailman/interfaces/mailinglist.py index 7f3b68008..0be8c2b68 100644 --- a/src/mailman/interfaces/mailinglist.py +++ b/src/mailman/interfaces/mailinglist.py @@ -20,6 +20,7 @@ __all__ = [ 'IAcceptableAlias', 'IAcceptableAliasSet', + 'IHeaderMatch', 'IListArchiver', 'IListArchiverSet', 'IMailingList', @@ -843,7 +844,7 @@ class IListArchiverSet(Interface): class IHeaderMatch(Interface): - """A header matching rule for mailinglist messages.""" + """A mailing list-specific message header matching rule.""" mailing_list = Attribute( """The mailing list for the header match.""") @@ -868,26 +869,32 @@ class IHeaderMatchSet(Interface): def clear(): """Clear the set of header matching rules.""" - def add(header, pattern, chain): - """Add the given header matching rule to this mailinglist's set. + def add(header, pattern, chain=None): + """Add the given header matching rule to this mailing list's set. - :param header: The email header to filter on. It will be converted to - lowercase for easier removal. + :param header: The email header to filter on. It will be converted to + lower case for consistency. :type header: string :param pattern: The regular expression to use. :type pattern: string :param chain: The chain to jump to, or None to use the site-wide - configuration. Defaults to None. + configuration. Defaults to None. :type chain: string or None - :raises ValueError: there can be only one couple of header and pattern - for a mailinglist. + :raises ValueError: if the header/pattern pair already exists for this + mailing list. """ def remove(header, pattern): - """Remove the given header matching rule from this mailinglist's set. + """Remove the given header matching rule from this mailing list's set. :param header: The email header part of the rule to be removed. :type header: string :param pattern: The regular expression part of the rule to be removed. :type pattern: string """ + + def __iter__(): + """An iterator over all the IHeaderMatches defined in this set. + + :return: iterator over `IHeaderMatch`. + """ diff --git a/src/mailman/model/mailinglist.py b/src/mailman/model/mailinglist.py index 617109a7e..0a5b20dd8 100644 --- a/src/mailman/model/mailinglist.py +++ b/src/mailman/model/mailinglist.py @@ -37,8 +37,8 @@ from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.domain import IDomainManager from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( - IAcceptableAlias, IAcceptableAliasSet, IListArchiver, IListArchiverSet, - IHeaderMatch, IHeaderMatchSet, IMailingList, Personalization, + IAcceptableAlias, IAcceptableAliasSet, IHeaderMatch, IHeaderMatchSet, + IListArchiver, IListArchiverSet, IMailingList, Personalization, ReplyToMunging, SubscriptionPolicy) from mailman.interfaces.member import ( AlreadySubscribedError, MemberRole, MissingPreferredAddressError, @@ -684,3 +684,8 @@ class HeaderMatchSet: raise ValueError('Pattern does not exist') else: self._mailing_list.header_matches.remove(existing) + + @dbconnection + def __iter__(self, store): + yield from store.query(HeaderMatch).filter( + HeaderMatch.mailing_list == self._mailing_list) diff --git a/src/mailman/model/tests/test_mailinglist.py b/src/mailman/model/tests/test_mailinglist.py index c497cb474..8d35a50f6 100644 --- a/src/mailman/model/tests/test_mailinglist.py +++ b/src/mailman/model/tests/test_mailinglist.py @@ -173,37 +173,48 @@ class TestHeaderMatch(unittest.TestCase): self._mlist = create_list('ant@example.com') def test_lowercase_header(self): - with transaction(): - header_matches = IHeaderMatchSet(self._mlist) - header_matches.add('Header', 'pattern') - self.assertEqual(len(self._mlist.header_matches), 1) - self.assertEqual(self._mlist.header_matches[0].header, 'header') + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + self.assertEqual(self._mlist.header_matches[0].header, 'header') def test_chain_defaults_to_none(self): - with transaction(): - header_matches = IHeaderMatchSet(self._mlist) - header_matches.add('header', 'pattern') - self.assertEqual(len(self._mlist.header_matches), 1) - self.assertEqual(self._mlist.header_matches[0].chain, None) + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + self.assertEqual(self._mlist.header_matches[0].chain, None) def test_duplicate(self): - with transaction(): - header_matches = IHeaderMatchSet(self._mlist) - header_matches.add('Header', 'pattern') - self.assertRaises(ValueError, - header_matches.add, 'Header', 'pattern') - self.assertEqual(len(self._mlist.header_matches), 1) + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Header', 'pattern') + self.assertRaises( + ValueError, header_matches.add, 'Header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) def test_remove_non_existent(self): - with transaction(): - header_matches = IHeaderMatchSet(self._mlist) - self.assertRaises(ValueError, - header_matches.remove, 'header', 'pattern') + header_matches = IHeaderMatchSet(self._mlist) + self.assertRaises( + ValueError, header_matches.remove, 'header', 'pattern') def test_add_remove(self): - with transaction(): - header_matches = IHeaderMatchSet(self._mlist) - header_matches.add('header', 'pattern') - self.assertEqual(len(self._mlist.header_matches), 1) - header_matches.remove('header', 'pattern') - self.assertEqual(len(self._mlist.header_matches), 0) + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 1) + header_matches.remove('header', 'pattern') + self.assertEqual(len(self._mlist.header_matches), 0) + + def test_iterator(self): + header_matches = IHeaderMatchSet(self._mlist) + header_matches.add('Header', 'pattern') + header_matches.add('Subject', 'patt.*') + header_matches.add('From', '.*@example.com', 'discard') + header_matches.add('From', '.*@example.org', 'accept') + matches = sorted((match.header, match.pattern, match.chain) + for match in IHeaderMatchSet(self._mlist)) + self.assertEqual( + matches, + [('from', '.*@example.com', 'discard'), + ('from', '.*@example.org', 'accept'), + ('header', 'pattern', None), + ('subject', 'patt.*', None), + ]) diff --git a/src/mailman/rules/docs/header-matching.rst b/src/mailman/rules/docs/header-matching.rst index 05d01efb2..2eb8d9bdf 100644 --- a/src/mailman/rules/docs/header-matching.rst +++ b/src/mailman/rules/docs/header-matching.rst @@ -120,11 +120,11 @@ List-specific header matching Each mailing list can also be configured with a set of header matching regular 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 +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 +``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. @@ -147,8 +147,8 @@ A message with a spam score of two pluses does not match. x-spam-score: [+]{3,} But a message with a spam score of three pluses does match. Because a message -with the previous Message-Id is already in the moderation queue, we need to -give this message a new Message-Id. +with the previous ``Message-Id`` is already in the moderation queue, we need +to give this message a new ``Message-Id``. >>> msgdata = {} >>> del msg['x-spam-score'] @@ -174,8 +174,8 @@ As does a message with a spam score of four pluses. 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. +Now, the list administrator wants to match on three plus signs, but wants +those emails to be discarded instead of held. >>> header_matches.remove('x-spam-score', '[+]{3,}') >>> header_matches.add('x-spam-score', '[+]{3,}', 'discard') @@ -187,10 +187,10 @@ will be discarded. >>> del msg['x-spam-score'] >>> msg['X-Spam-Score'] = '+++' >>> del msg['message-id'] - >>> msg['Message-Id'] = '' + >>> msg['Message-Id'] = '' >>> with event_subscribers(handler): ... process(mlist, msg, msgdata, 'header-match') - DiscardEvent discard + DiscardEvent discard >>> hits_and_misses(msgdata) Rule hits: x-spam-score: [+]{3,} diff --git a/src/mailman/utilities/importer.py b/src/mailman/utilities/importer.py index 4e3eab6cf..a15eac7f9 100644 --- a/src/mailman/utilities/importer.py +++ b/src/mailman/utilities/importer.py @@ -39,7 +39,6 @@ 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.chain import LinkAction from mailman.interfaces.digests import DigestFrequency from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import IAcceptableAliasSet, IHeaderMatchSet @@ -132,18 +131,18 @@ def nonmember_action_mapping(value): def action_to_chain(value): # Converts an action number in Mailman 2.1 to the name of the corresponding - # chain in 3.x. The actions "approve", "subscribe" and "unsubscribe" are - # ignored. The defer action is converted to None, because it is not a jump - # to a terminal chain. + # chain in 3.x. The actions 'approve', 'subscribe' and 'unsubscribe' are + # ignored. The defer action is converted to None, because it is not + # a jump to a terminal chain. return { 0: None, - #1: "approve", - 2: "reject", - 3: "discard", - #4: "subscribe", - #5: "unsubscribe", - 6: "accept", - 7: "hold", + #1: 'approve', + 2: 'reject', + 3: 'discard', + #4: 'subscribe', + #5: 'unsubscribe', + 6: 'accept', + 7: 'hold', }[value] @@ -333,7 +332,7 @@ def import_config_pck(mlist, config_dict): # When .add() rejects this, the line probably contains a regular # expression. Make that explicit for MM3. alias_set.add('^' + address) - # Handle header_filter_rules conversion to header_matches + # Handle header_filter_rules conversion to header_matches. header_match_set = IHeaderMatchSet(mlist) header_filter_rules = config_dict.get('header_filter_rules', []) for line_patterns, action, _unused in header_filter_rules: @@ -343,26 +342,28 @@ def import_config_pck(mlist, config_dict): log.warning('Unsupported header_filter_rules action: %r', action) continue - # now split the pattern in a header and a pattern + # Now split the line into a header and a pattern. for line_pattern in line_patterns.splitlines(): - if not line_pattern.strip(): + if len(line_pattern.strip()) == 0: continue for sep in (': ', ':.', ':'): header, sep, pattern = line_pattern.partition(sep) if sep: - break # found it. + # We found it. + break else: - # matches any header. Those are not supported. XXX + # Matches any header, which is not supported. XXX log.warning('Unsupported header_filter_rules pattern: %r', line_pattern) continue - header = header.strip().lstrip("^").lower() + header = header.strip().lstrip('^').lower() header = header.replace('\\', '') if not header: - log.warning('Can\'t parse the header in header_filter_rule: %r', - line_pattern) + log.warning( + 'Cannot parse the header in header_filter_rule: %r', + line_pattern) continue - if not pattern: + if len(pattern) == 0: # The line matched only the header, therefore the header can # be anything. pattern = '.*' diff --git a/src/mailman/utilities/tests/test_import.py b/src/mailman/utilities/tests/test_import.py index 64f8e061f..e687e8a00 100644 --- a/src/mailman/utilities/tests/test_import.py +++ b/src/mailman/utilities/tests/test_import.py @@ -44,7 +44,7 @@ from mailman.interfaces.bans import IBanManager from mailman.interfaces.bounce import UnrecognizedBounceDisposition from mailman.interfaces.languages import ILanguageManager from mailman.interfaces.mailinglist import ( - IAcceptableAliasSet, IHeaderMatchSet, SubscriptionPolicy) + IAcceptableAliasSet, SubscriptionPolicy) from mailman.interfaces.member import DeliveryMode, DeliveryStatus from mailman.interfaces.nntp import NewsgroupModeration from mailman.interfaces.templates import ITemplateLoader @@ -344,7 +344,8 @@ class TestBasicImport(unittest.TestCase): ('X-Git-Module: rhq.*git', 6, False), ('Approved: verysecretpassword', 6, False), ('^Subject: dev-\r\n^Subject: staging-', 3, False), - ('from: .*info@aolanchem.com\r\nfrom: .*@jw-express.com', 2, False), + ('from: .*info@aolanchem.com\r\nfrom: .*@jw-express.com', + 2, False), ('^Received: from smtp-.*\\.fedoraproject\\.org\r\n' '^Received: from mx.*\\.redhat.com\r\n' '^Resent-date:\r\n' @@ -356,15 +357,17 @@ class TestBasicImport(unittest.TestCase): ('^Received: from fedorahosted\\.org.*by fedorahosted\\.org\r\n' '^Received: from hosted.*\\.fedoraproject.org.*by ' 'hosted.*\\.fedoraproject\\.org\r\n' - '^Received: from hosted.*\\.fedoraproject.org.*by fedoraproject\\.org\r\n' - '^Received: from hosted.*\\.fedoraproject.org.*by fedorahosted\\.org', + '^Received: from hosted.*\\.fedoraproject.org.*by ' + 'fedoraproject\\.org\r\n' + '^Received: from hosted.*\\.fedoraproject.org.*by ' + 'fedorahosted\\.org', 6, False), ] error_log = LogFileMark('mailman.error') self._import() self.assertListEqual( - [ (hm.header, hm.pattern, hm.chain) - for hm in self._mlist.header_matches ], [ + [(hm.header, hm.pattern, hm.chain) + for hm in self._mlist.header_matches ], [ ('x-spam-status', 'Yes.*', 'discard'), ('x-spam-status', 'Yes', 'reject'), ('x-spam-level', '\\*\\*\\*.*$', 'discard'), @@ -386,19 +389,26 @@ class TestBasicImport(unittest.TestCase): ('resent-message-id', '.*', 'hold'), ('resent-to', '.*', 'hold'), ('subject', '[^mtv]', 'hold'), - ('received', 'from fedorahosted\\.org.*by fedorahosted\\.org', 'accept'), - ('received', 'from hosted.*\\.fedoraproject.org.*by hosted.*\\.fedoraproject\\.org', 'accept'), - ('received', 'from hosted.*\\.fedoraproject.org.*by fedoraproject\\.org', 'accept'), - ('received', 'from hosted.*\\.fedoraproject.org.*by fedorahosted\\.org', 'accept'), + ('received', 'from fedorahosted\\.org.*by fedorahosted\\.org', + 'accept'), + ('received', + 'from hosted.*\\.fedoraproject.org.*by ' + 'hosted.*\\.fedoraproject\\.org', 'accept'), + ('received', + 'from hosted.*\\.fedoraproject.org.*by ' + 'fedoraproject\\.org', 'accept'), + ('received', + 'from hosted.*\\.fedoraproject.org.*by ' + 'fedorahosted\\.org', 'accept'), ]) loglines = error_log.read().strip() self.assertEqual(len(loglines), 0) def test_header_matches_header_only(self): - # Check that an empty pattern is skipped + # Check that an empty pattern is skipped. self._pckdict['header_filter_rules'] = [ ('SomeHeaderName', 3, False), - ] + ] error_log = LogFileMark('mailman.error') self._import() self.assertListEqual(self._mlist.header_matches, []) @@ -406,10 +416,10 @@ class TestBasicImport(unittest.TestCase): error_log.readline()) def test_header_matches_anything(self): - # Check that an empty pattern is skipped + # Check that a wild card header pattern is skipped. self._pckdict['header_filter_rules'] = [ ('.*', 7, False), - ] + ] error_log = LogFileMark('mailman.error') self._import() self.assertListEqual(self._mlist.header_matches, []) @@ -417,10 +427,10 @@ class TestBasicImport(unittest.TestCase): error_log.readline()) def test_header_matches_invalid_re(self): - # Check that an empty pattern is skipped + # Check that an invalid regular expression pattern is skipped. self._pckdict['header_filter_rules'] = [ ('SomeHeaderName: *invalid-re', 3, False), - ] + ] error_log = LogFileMark('mailman.error') self._import() self.assertListEqual(self._mlist.header_matches, []) @@ -431,20 +441,20 @@ class TestBasicImport(unittest.TestCase): # Check that a defer action is properly converted. self._pckdict['header_filter_rules'] = [ ('^X-Spam-Status: Yes', 0, False), - ] + ] self._import() self.assertListEqual( - [ (hm.header, hm.pattern, hm.chain) - for hm in self._mlist.header_matches ], - [ ('x-spam-status', 'Yes', None) ] - ) + [(hm.header, hm.pattern, hm.chain) + for hm in self._mlist.header_matches], + [('x-spam-status', 'Yes', None)] + ) def test_header_matches_unsupported_action(self): - # Check that an unsupported actions are skipped + # Check that 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, []) @@ -457,17 +467,17 @@ class TestBasicImport(unittest.TestCase): member.unsubscribe() def test_header_matches_duplicate(self): - # Check that duplicate patterns don't cause tracebacks + # Check that duplicate patterns don't cause tracebacks. self._pckdict['header_filter_rules'] = [ ('SomeHeaderName: test-pattern', 3, False), ('SomeHeaderName: test-pattern', 2, False), - ] + ] error_log = LogFileMark('mailman.error') self._import() self.assertListEqual( - [ (hm.header, hm.pattern, hm.chain) - for hm in self._mlist.header_matches ], - [ ('someheadername', 'test-pattern', 'discard') ] + [(hm.header, hm.pattern, hm.chain) + for hm in self._mlist.header_matches], + [('someheadername', 'test-pattern', 'discard')] ) self.assertIn('Skipping duplicate header_filter rule', error_log.readline()) -- cgit v1.2.3-70-g09d2 From 5104e712380acca2faef5cfd7dc24a3ffc82bfbe Mon Sep 17 00:00:00 2001 From: Barry Warsaw Date: Tue, 20 Oct 2015 22:51:37 -0400 Subject: Add NEWS. --- src/mailman/docs/NEWS.rst | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'src') diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 6d20ad019..424c42916 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -43,6 +43,11 @@ Bugs Configuration ------------- + * Mailing lists can now have their own header matching rules, although + site-defined rules still take precedence. Importing a Mailman 2.1 list + with header matching rules defined will create them in Mailman 3, albeit + with a few unsupported corner cases. Definition of new header matching + rules is not yet exposed through the REST API. Given by Aurélien Bompard. * The default languages from Mailman 2.1 have been ported over. Given by Aurélien Bompard. -- cgit v1.2.3-70-g09d2