Commit 50007671 authored by Barry Warsaw's avatar Barry Warsaw

Merge branch 'header_matches' into 'master'

Properly handle invalid regexps for header_matches patterns.

Closes #418

See merge request !331
parents 46cd88f6 d6827ac9
Pipeline #12908079 passed with stage
in 17 minutes and 40 seconds
......@@ -14,6 +14,7 @@ omit =
[report]
exclude_lines =
pragma: nocover
pragma: missed
raise NotImplementedError
raise AssertionError
assert\s
......
......@@ -104,13 +104,22 @@ class HeaderMatchRule:
for value in headers:
if isinstance(value, Header):
value = value.encode()
if re.search(self.pattern, value, re.IGNORECASE):
msgdata['moderation_sender'] = msg.sender
with _.defer_translation():
# This will be translated at the point of use.
msgdata.setdefault('moderation_reasons', []).append(
(_('Header "{}" matched a header rule'), str(value)))
return True
try:
mo = re.search(self.pattern, value, re.IGNORECASE)
except re.error as error:
log.error(
"Invalid regexp '{}' in header_matches for {}: {}".format(
self.pattern, mlist.list_id, error.msg))
return False
else:
if mo:
msgdata['moderation_sender'] = msg.sender
with _.defer_translation():
# This will be translated at the point of use.
msgdata.setdefault('moderation_reasons', []).append(
(_('Header "{}" matched a header rule'),
str(value)))
return True
return False
......
......@@ -125,6 +125,35 @@ class TestHeaderChain(unittest.TestCase):
'Configuration error: [antispam]header_checks '
'contains bogus line: A-bad-line')
def test_bad_regexp(self):
# Take a mark on the error log file.
mark = LogFileMark('mailman.error')
# A bad regexp in header_checks should just get ignored, but
# with an error message logged.
header_matches = IHeaderMatchList(self._mlist)
header_matches.append('Foo', '+a bad regexp', 'reject')
msg = mfs("""\
From: anne@example.com
To: test@example.com
Subject: A message
Message-ID: <ant>
Foo: foo
MIME-Version: 1.0
A message body.
""")
msgdata = {}
# This event subscriber records the event that occurs when the message
# is processed by the header-match chain.
events = []
with event_subscribers(events.append):
process(self._mlist, msg, msgdata, start_chain='header-match')
self.assertEqual(len(events), 0)
# Check the error log.
self.assertEqual(mark.readline()[-89:-1],
"Invalid regexp '+a bad regexp' in header_matches "
'for test.example.com: nothing to repeat')
def test_duplicate_header_match_rule(self):
# 100% coverage: test an assertion in a corner case.
#
......
......@@ -34,6 +34,8 @@ Bugs
will now result in mitigations being applied. (Closes #415)
* Messages without a sender can no longer bypass the ``nonmember-moderation``
rule. (Closes #414)
* Invalid regexps in header_matches rules are properly logged and can't be set
via REST. (Closes #418)
Command line
------------
......
......@@ -20,9 +20,9 @@
from mailman.interfaces.action import Action
from mailman.interfaces.mailinglist import IHeaderMatchList
from mailman.rest.helpers import (
CollectionMixin, bad_request, child, created, etag, no_content, not_found,
okay)
from mailman.rest.validator import Validator, enum_validator
CollectionMixin, GetterSetter, bad_request, child, created,
etag, no_content, not_found, okay)
from mailman.rest.validator import Validator, enum_validator, regexp_validator
from public import public
......@@ -92,7 +92,7 @@ class HeaderMatch(_HeaderMatchBase):
return
kws = dict(
header=lowercase,
pattern=str,
pattern=GetterSetter(regexp_validator),
position=int,
action=enum_validator(Action),
)
......@@ -142,7 +142,7 @@ class HeaderMatches(_HeaderMatchBase, CollectionMixin):
"""Add a header match."""
validator = Validator(
header=str,
pattern=str,
pattern=GetterSetter(regexp_validator),
action=enum_validator(Action),
_optional=('action',)
)
......
......@@ -70,3 +70,28 @@ class TestHeaderMatches(unittest.TestCase):
call_api('http://localhost:9001/3.0/lists/bee.example.com'
'/header-matches/')
self.assertEqual(cm.exception.code, 404)
def test_add_bad_regexp(self):
with self.assertRaises(HTTPError) as cm:
call_api('http://localhost:9001/3.0/lists/ant.example.com'
'/header-matches', {
'header': 'header',
'pattern': '+invalid',
})
self.assertEqual(cm.exception.code, 400)
self.assertEqual(cm.exception.reason,
'Cannot convert parameters: pattern')
def test_patch_bad_regexp(self):
header_matches = IHeaderMatchList(self._mlist)
with transaction():
header_matches.append('header', 'pattern')
with self.assertRaises(HTTPError) as cm:
call_api('http://localhost:9001/3.0/lists/ant.example.com'
'/header-matches/0', {
'header': 'header',
'pattern': '+invalid',
}, method='PATCH')
self.assertEqual(cm.exception.code, 400)
self.assertEqual(cm.exception.reason,
'Cannot convert parameters: pattern')
......@@ -17,6 +17,8 @@
"""REST web form validation."""
import re
from mailman.interfaces.address import IEmailValidator
from mailman.interfaces.errors import MailmanError
from mailman.interfaces.languages import ILanguageManager
......@@ -108,6 +110,21 @@ def integer_ge_zero_validator(value):
return value
@public
def regexp_validator(value): # pragma: missed
"""Validate that the value is a valid regexp."""
# This code is covered as proven by the fact that the tests
# test_add_bad_regexp and test_patch_bad_regexp in
# mailman/rest/tests/test_header_matches.py fail with AssertionError:
# HTTPError not raised if the code is bypassed, but coverage says it's
# not covered so work around it for now.
try:
re.compile(value)
except re.error:
raise ValueError('Expected a valid regexp, got {}'.format(value))
return value
@public
class Validator:
"""A validator of parameter input."""
......
......@@ -261,7 +261,7 @@ def is_reject_or_quarantine(mlist, email, dmarc_domain, org=False):
# Coverage BitBucket issue #198 and
# http://bugs.python.org/issue2506 coverage cannot report
# it as such, so just pragma it away.
continue # pragma: no cover
continue # pragma: missed
if policy in ('reject', 'quarantine'):
vlog.info(
'%s: DMARC lookup for %s (%s) found p=%s in %s = %s',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment