diff options
| -rw-r--r-- | src/mailman/chains/headers.py | 47 | ||||
| -rw-r--r-- | src/mailman/chains/tests/test_headers.py | 27 | ||||
| -rw-r--r-- | src/mailman/docs/NEWS.rst | 2 |
3 files changed, 35 insertions, 41 deletions
diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py index c7f64c5cd..70f1da587 100644 --- a/src/mailman/chains/headers.py +++ b/src/mailman/chains/headers.py @@ -20,6 +20,7 @@ import re import logging +from itertools import count from mailman import public from mailman.chains.base import Chain, Link from mailman.config import config @@ -30,9 +31,18 @@ from zope.interface import implementer log = logging.getLogger('mailman.error') +_RULE_COUNTER = count(1) -def make_link(header, pattern, chain=None, name=None): +def _make_rule_name(suffix): + # suffix may be None, since it comes from the 'name' parameter given in + # the HeaderMatchRule constructor. + if suffix is None: + suffix = '{0:02}'.format(next(_RULE_COUNTER)) + return 'header-match-{}'.format(suffix) + + +def make_link(header, pattern, chain=None, suffix=None): """Create a Link object. The link action is to defer by default, since at the end of all the @@ -47,45 +57,30 @@ def make_link(header, pattern, chain=None, name=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 + :param suffix: An optional name suffix for the rule. + :type suffix: string :return: The link representing this rule check. :rtype: `ILink` """ - rule_name = get_rule_name(name) + rule_name = _make_rule_name(suffix) if rule_name in config.rules: rule = config.rules[rule_name] else: - rule = HeaderMatchRule(header, pattern, name) + rule = HeaderMatchRule(header, pattern, suffix) 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.""" - # Sequential rule counter. - _count = 1 - - def __init__(self, header, pattern, name=None): + def __init__(self, header, pattern, suffix=None): self.header = header self.pattern = pattern - self.name = get_rule_name(name) - HeaderMatchRule._count += 1 - self.description = '{0}: {1}'.format(header, pattern) + self.name = _make_rule_name(suffix) + self.description = '{}: {}'.format(header, pattern) # XXX I think we should do better here, somehow recording that a # particular header matched a particular pattern, but that gets ugly # with RFC 2822 headers. It also doesn't match well with the rule @@ -152,8 +147,8 @@ class HeaderMatchChain(Chain): log.error('Configuration error: [antispam]header_checks ' 'contains bogus line: {}'.format(line)) continue - rule_name = 'config-{0}'.format(index) - yield make_link(parts[0], parts[1].lstrip(), name=rule_name) + rule_name = 'config-{}'.format(index) + yield make_link(parts[0], parts[1].lstrip(), suffix=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 @@ -167,5 +162,5 @@ class HeaderMatchChain(Chain): chain = (config.antispam.jump_chain if entry.chain is None else entry.chain) - rule_name = '{0}-{1}'.format(mlist.list_id, index) + rule_name = '{}-{}'.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 2fa4bfec8..6cd37c988 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -128,13 +128,10 @@ class TestHeaderChain(unittest.TestCase): # # Save the existing rules so they can be restored later. saved_rules = config.rules.copy() - next_rule_name = 'header-match-{0:02}'.format(HeaderMatchRule._count) - config.rules[next_rule_name] = object() - try: - self.assertRaises(AssertionError, - HeaderMatchRule, 'x-spam-score', '.*') - finally: - config.rules = saved_rules + self.addCleanup(setattr, config, 'rules', saved_rules) + HeaderMatchRule('x-spam-score', '*', suffix='100') + self.assertRaises(AssertionError, + HeaderMatchRule, 'x-spam-score', '.*', suffix='100') def test_list_rule(self): # Test that the header-match chain has the header checks from the @@ -265,19 +262,19 @@ A message body. header_matches = IHeaderMatchList(self._mlist) header_matches.append('Header2', 'b+') header_matches.append('Header3', 'c+') - def get_links(): # flake8: noqa + def get_links(): # 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_1 = get_links() + self.assertEqual(len(links_1), 3) links_2 = get_links() + # The link rules both have the same name... self.assertEqual( - [l.rule.name for l in links], + [l.rule.name for l in links_1], [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], - ) + # ...and are actually the identical objects. + for link1, link2 in zip(links_1, links_2): + self.assertIs(link1.rule, link2.rule) diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index 73586269b..810ca1ce7 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -79,6 +79,8 @@ Bugs * Fix query bug for ``SubscriptionService.find_members()`` leading to the incorrect number of members being returned. Given by Aurélien Bompard. (Closes #227) + * Fix header match rule suffix inflation. Given by Aurélien Bompard. + (Closes #226) Configuration ------------- |