diff options
| -rw-r--r-- | src/mailman/chains/headers.py | 36 | ||||
| -rw-r--r-- | src/mailman/chains/tests/test_headers.py | 29 |
2 files changed, 57 insertions, 8 deletions
diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py index 3e6d4bfaa..c7f64c5cd 100644 --- a/src/mailman/chains/headers.py +++ b/src/mailman/chains/headers.py @@ -32,7 +32,7 @@ from zope.interface import implementer log = logging.getLogger('mailman.error') -def make_link(header, pattern, chain=None): +def make_link(header, pattern, chain=None, name=None): """Create a Link object. The link action is to defer by default, since at the end of all the @@ -47,15 +47,32 @@ def make_link(header, pattern, chain=None): :param chain: When given, this is the name of the chain to jump to if the pattern matches the header. :type chain: string + :param name: An optional name for the rule. + :type name: string :return: The link representing this rule check. :rtype: `ILink` """ - rule = HeaderMatchRule(header, pattern) + rule_name = get_rule_name(name) + if rule_name in config.rules: + rule = config.rules[rule_name] + else: + rule = HeaderMatchRule(header, pattern, name) if chain is None: return Link(rule) return Link(rule, LinkAction.jump, chain) +def get_rule_name(suffix): + name = ['header-match'] + if suffix: + name.append(suffix) + else: + name.append('{0:02}'.format( + HeaderMatchRule._count + )) + return '-'.join(name) + + @implementer(IRule) class HeaderMatchRule: """Header matching rule used by header-match chain.""" @@ -63,10 +80,10 @@ class HeaderMatchRule: # Sequential rule counter. _count = 1 - def __init__(self, header, pattern): + def __init__(self, header, pattern, name=None): self.header = header self.pattern = pattern - self.name = 'header-match-{0:02}'.format(HeaderMatchRule._count) + self.name = get_rule_name(name) HeaderMatchRule._count += 1 self.description = '{0}: {1}'.format(header, pattern) # XXX I think we should do better here, somehow recording that a @@ -126,7 +143,8 @@ class HeaderMatchChain(Chain): def get_links(self, mlist, msg, msgdata): """See `IChain`.""" # First return all the configuration file links. - for line in config.antispam.header_checks.splitlines(): + for index, line in enumerate( + config.antispam.header_checks.splitlines()): if len(line.strip()) == 0: continue parts = line.split(':', 1) @@ -134,7 +152,8 @@ class HeaderMatchChain(Chain): log.error('Configuration error: [antispam]header_checks ' 'contains bogus line: {}'.format(line)) continue - yield make_link(parts[0], parts[1].lstrip()) + rule_name = 'config-{0}'.format(index) + yield make_link(parts[0], parts[1].lstrip(), name=rule_name) # Then return all the explicitly added links. yield from self._extended_links # If any of the above rules matched, they will have deferred their @@ -143,9 +162,10 @@ class HeaderMatchChain(Chain): # list-specific matches. yield Link('any', LinkAction.jump, config.antispam.jump_chain) # Then return all the list-specific header matches. - for entry in mlist.header_matches: + for index, entry in enumerate(mlist.header_matches): # Jump to the default antispam chain if the entry chain is None. chain = (config.antispam.jump_chain if entry.chain is None else entry.chain) - yield make_link(entry.header, entry.pattern, chain) + rule_name = '{0}-{1}'.format(mlist.list_id, index) + yield make_link(entry.header, entry.pattern, chain, rule_name) diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py index 9c3cc1b40..2fa4bfec8 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -149,6 +149,8 @@ class TestHeaderChain(unittest.TestCase): self.assertEqual(links[0].chain.name, config.antispam.jump_chain) self.assertEqual(links[0].rule.header, 'foo') self.assertEqual(links[0].rule.pattern, 'a+') + self.assertTrue(links[0].rule.name.startswith( + 'header-match-test.example.com-')) def test_list_complex_rule(self): # Test that the mailing-list header-match complex rules are read @@ -252,3 +254,30 @@ A message body. self.assertEqual(event.chain, config.chains['discard']) self.assertEqual(event.mlist, self._mlist) self.assertEqual(event.msg, msg) + + @configuration('antispam', header_checks=""" + Header1: a+ + """, jump_chain='hold') + def test_reuse_rules(self): + # Test that existing header-match rules are used instead of creating + # new ones. + chain = config.chains['header-match'] + header_matches = IHeaderMatchList(self._mlist) + header_matches.append('Header2', 'b+') + header_matches.append('Header3', 'c+') + def get_links(): # flake8: noqa + return [ + link for link in chain.get_links(self._mlist, Message(), {}) + if link.rule.name != 'any' + ] + links = get_links() + self.assertEqual(len(links), 3) + links_2 = get_links() + self.assertEqual( + [l.rule.name for l in links], + [l.rule.name for l in links_2], + ) + self.assertEqual( + [id(l.rule) for l in links], + [id(l.rule) for l in links_2], + ) |