From bea94cb9538a55b1376afd42c2ce751efce62cfe Mon Sep 17 00:00:00 2001 From: Mark Sapiro Date: Wed, 26 Jul 2017 08:11:33 -0700 Subject: Ensure all holds/rejects have a reason. --- setup.py | 2 +- src/mailman/chains/base.py | 19 +++++++++ src/mailman/chains/headers.py | 5 +++ src/mailman/chains/hold.py | 11 ++--- src/mailman/chains/reject.py | 4 +- src/mailman/chains/tests/test_headers.py | 55 ++++++++++++++++++++++++- src/mailman/chains/tests/test_hold.py | 7 +++- src/mailman/chains/tests/test_reject.py | 2 + src/mailman/docs/NEWS.rst | 2 + src/mailman/rules/administrivia.py | 5 +++ src/mailman/rules/banned_address.py | 6 +++ src/mailman/rules/dmarc.py | 24 +++++++---- src/mailman/rules/emergency.py | 9 +++- src/mailman/rules/implicit_dest.py | 5 +++ src/mailman/rules/loop.py | 9 +++- src/mailman/rules/max_recipients.py | 10 ++++- src/mailman/rules/max_size.py | 10 ++++- src/mailman/rules/moderation.py | 23 ++++++----- src/mailman/rules/news_moderation.py | 9 +++- src/mailman/rules/no_senders.py | 10 ++--- src/mailman/rules/no_subject.py | 9 +++- src/mailman/rules/suspicious.py | 10 ++++- src/mailman/rules/tests/test_administrivia.py | 51 +++++++++++++++++++++++ src/mailman/rules/tests/test_banned_address.py | 24 +++++++++++ src/mailman/rules/tests/test_emergency.py | 52 +++++++++++++++++++++++ src/mailman/rules/tests/test_implicit_dest.py | 51 +++++++++++++++++++++++ src/mailman/rules/tests/test_loop.py | 52 +++++++++++++++++++++++ src/mailman/rules/tests/test_max_recipients.py | 53 ++++++++++++++++++++++++ src/mailman/rules/tests/test_max_size.py | 55 +++++++++++++++++++++++++ src/mailman/rules/tests/test_news_moderation.py | 53 ++++++++++++++++++++++++ src/mailman/rules/tests/test_no_senders.py | 5 +-- src/mailman/rules/tests/test_no_subject.py | 9 ++++ src/mailman/rules/tests/test_suspicious.py | 13 ++++++ 33 files changed, 619 insertions(+), 45 deletions(-) create mode 100644 src/mailman/rules/tests/test_administrivia.py create mode 100644 src/mailman/rules/tests/test_emergency.py create mode 100644 src/mailman/rules/tests/test_implicit_dest.py create mode 100644 src/mailman/rules/tests/test_loop.py create mode 100644 src/mailman/rules/tests/test_max_recipients.py create mode 100644 src/mailman/rules/tests/test_max_size.py create mode 100644 src/mailman/rules/tests/test_news_moderation.py diff --git a/setup.py b/setup.py index 67fdb16bf..d6303676d 100644 --- a/setup.py +++ b/setup.py @@ -111,7 +111,7 @@ case second `m'. Any other spelling is incorrect.""", 'dnspython>=1.14.0', 'falcon>=1.0.0rc1', 'flufl.bounce', - 'flufl.i18n', + 'flufl.i18n>=2.0', 'flufl.lock>=3.1', 'lazr.config', 'passlib', diff --git a/src/mailman/chains/base.py b/src/mailman/chains/base.py index 59125ba69..9507d8fbc 100644 --- a/src/mailman/chains/base.py +++ b/src/mailman/chains/base.py @@ -18,6 +18,7 @@ """Base class for terminal chains.""" from mailman.config import config +from mailman.core.i18n import _ from mailman.interfaces.chain import ( IChain, IChainIterator, IChainLink, IMutableChain, LinkAction) from mailman.interfaces.rules import IRule @@ -26,6 +27,24 @@ from public import public from zope.interface import implementer +@public +def format_reasons(reasons): + """Translate and format hold and rejection reasons. + + :param reasons: A list of reasons from the rules that hit. Each reason is + a string to be translated or a tuple consisting of a string with {} + replacements and one or more replacement values. + :returns: A list of the translated and formatted strings. + """ + new_reasons = [] + for reason in reasons: + if isinstance(reason, tuple): + new_reasons.append(_(reason[0]).format(*reason[1:])) + else: + new_reasons.append(_(reason)) + return new_reasons + + @public @implementer(IChainLink) class Link: diff --git a/src/mailman/chains/headers.py b/src/mailman/chains/headers.py index 6fc061fe4..01ba96971 100644 --- a/src/mailman/chains/headers.py +++ b/src/mailman/chains/headers.py @@ -105,6 +105,11 @@ class HeaderMatchRule: 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 return False diff --git a/src/mailman/chains/hold.py b/src/mailman/chains/hold.py index 4e72b685a..edc80da3c 100644 --- a/src/mailman/chains/hold.py +++ b/src/mailman/chains/hold.py @@ -24,7 +24,7 @@ from email.mime.text import MIMEText from email.utils import formatdate, make_msgid from mailman.app.moderator import hold_message from mailman.app.replybot import can_acknowledge -from mailman.chains.base import TerminalChainBase +from mailman.chains.base import TerminalChainBase, format_reasons from mailman.config import config from mailman.core.i18n import _ from mailman.email.message import UserNotification @@ -57,8 +57,8 @@ def _compose_reasons(msgdata, column=66): # Rules can add reasons to the metadata. reasons = msgdata.get('moderation_reasons', [_('N/A')]) return NL.join( - [(SPACE * 4) + wrap(_(reason), column=column) - for reason in reasons]) + [(SPACE * 4) + wrap(reason, column=column) + for reason in format_reasons(reasons)]) def autorespond_to_sender(mlist, sender, language=None): @@ -142,7 +142,7 @@ class HoldChain(TerminalChainBase): rule_misses = msgdata.get('rule_misses') if rule_misses: msg['X-Mailman-Rule-Misses'] = SEMISPACE.join(rule_misses) - reasons = msgdata.get('moderation_reasons', ['n/a']) + reasons = format_reasons(msgdata.get('moderation_reasons', ['n/a'])) # Hold the message by adding it to the list's request database. request_id = hold_message(mlist, msg, msgdata, SEMISPACE.join(reasons)) # Calculate a confirmation token to send to the author of the @@ -251,7 +251,8 @@ also appear in the first line of the body of the reply.""")), # Log the held message. Log messages are not translated, so recast # the reasons in the English. with _.using('en'): - reasons = msgdata.get('moderation_reasons', ['N/A']) + reasons = format_reasons( + msgdata.get('moderation_reasons', ['N/A'])) log.info('HOLD: %s post from %s held, message-id=%s: %s', mlist.fqdn_listname, msg.sender, msg.get('message-id', 'n/a'), SEMISPACE.join(reasons)) diff --git a/src/mailman/chains/reject.py b/src/mailman/chains/reject.py index 3284bba9f..31f66c8fa 100644 --- a/src/mailman/chains/reject.py +++ b/src/mailman/chains/reject.py @@ -20,7 +20,7 @@ import logging from mailman.app.bounces import bounce_message -from mailman.chains.base import TerminalChainBase +from mailman.chains.base import TerminalChainBase, format_reasons from mailman.core.i18n import _ from mailman.interfaces.chain import RejectEvent from mailman.interfaces.pipeline import RejectMessage @@ -65,7 +65,7 @@ reasons: The original message as received by Mailman is attached. """).format( list_name=mlist.display_name, # noqa: E122 - reasons=NEWLINE.join(reasons) + reasons=NEWLINE.join(format_reasons(reasons)) )) bounce_message(mlist, msg, error) log.info('REJECT: %s', msg.get('message-id', 'n/a')) diff --git a/src/mailman/chains/tests/test_headers.py b/src/mailman/chains/tests/test_headers.py index cd4c932cc..2aae503b2 100644 --- a/src/mailman/chains/tests/test_headers.py +++ b/src/mailman/chains/tests/test_headers.py @@ -25,7 +25,8 @@ 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 DiscardEvent, HoldEvent, LinkAction +from mailman.interfaces.chain import ( + DiscardEvent, HoldEvent, LinkAction, RejectEvent) from mailman.interfaces.mailinglist import IHeaderMatchList from mailman.testing.helpers import ( LogFileMark, configuration, event_subscribers, @@ -343,3 +344,55 @@ A message body. # ...and are actually the identical objects. for link1, link2 in zip(links_1, links_2): self.assertIs(link1.rule, link2.rule) + + def test_hold_returns_reason(self): + # Test that a match with hold action returns a reason + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: Bad subject +Message-ID: + +body + +""") + msgdata = {} + header_matches = IHeaderMatchList(self._mlist) + header_matches.append('Subject', 'Bad', 'hold') + # This event subscriber records the event that occurs when the message + # is processed by the owner chain. + events = [] + with event_subscribers(events.append): + process(self._mlist, msg, msgdata, start_chain='header-match') + self.assertEqual(len(events), 1) + event = events[0] + self.assertIsInstance(event, HoldEvent) + self.assertEqual(msgdata['moderation_reasons'], + [('Header "{}" matched a header rule', + 'Bad subject')]) + + def test_reject_returns_reason(self): + # Test that a match with reject action returns a reason + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: Bad subject +Message-ID: + +body + +""") + msgdata = {} + header_matches = IHeaderMatchList(self._mlist) + header_matches.append('Subject', 'Bad', 'reject') + # This event subscriber records the event that occurs when the message + # is processed by the owner chain. + events = [] + with event_subscribers(events.append): + process(self._mlist, msg, msgdata, start_chain='header-match') + self.assertEqual(len(events), 1) + event = events[0] + self.assertIsInstance(event, RejectEvent) + self.assertEqual(msgdata['moderation_reasons'], + [('Header "{}" matched a header rule', + 'Bad subject')]) diff --git a/src/mailman/chains/tests/test_hold.py b/src/mailman/chains/tests/test_hold.py index b973b874c..560916e3b 100644 --- a/src/mailman/chains/tests/test_hold.py +++ b/src/mailman/chains/tests/test_hold.py @@ -111,6 +111,7 @@ A message body. msgdata = dict(moderation_reasons=[ 'TEST-REASON-1', 'TEST-REASON-2', + ('TEST-{}-REASON-{}', 'FORMAT', 3), ]) logfile = LogFileMark('mailman.vette') process_chain(self._mlist, msg, msgdata, start_chain='hold') @@ -126,18 +127,22 @@ A message body. self.fail('Unexpected message: %s' % item.msg) self.assertIn(' TEST-REASON-1', payloads['owner']) self.assertIn(' TEST-REASON-2', payloads['owner']) + self.assertIn(' TEST-FORMAT-REASON-3', payloads['owner']) self.assertIn(' TEST-REASON-1', payloads['sender']) self.assertIn(' TEST-REASON-2', payloads['sender']) + self.assertIn(' TEST-FORMAT-REASON-3', payloads['sender']) logged = logfile.read() self.assertIn('TEST-REASON-1', logged) self.assertIn('TEST-REASON-2', logged) + self.assertIn('TEST-FORMAT-REASON-3', logged) # Check the reason passed to hold_message(). requests = IListRequests(self._mlist) self.assertEqual(requests.count_of(RequestType.held_message), 1) request = requests.of_type(RequestType.held_message)[0] key, data = requests.get_request(request.id) self.assertEqual( - data.get('_mod_reason'), 'TEST-REASON-1; TEST-REASON-2') + data.get('_mod_reason'), + 'TEST-REASON-1; TEST-REASON-2; TEST-FORMAT-REASON-3') def test_hold_chain_no_reasons_given(self): msg = mfs("""\ diff --git a/src/mailman/chains/tests/test_reject.py b/src/mailman/chains/tests/test_reject.py index f6fd6a8fe..8fce1de4a 100644 --- a/src/mailman/chains/tests/test_reject.py +++ b/src/mailman/chains/tests/test_reject.py @@ -45,12 +45,14 @@ Subject: Ignore msgdata = dict(moderation_reasons=[ 'TEST-REASON-1', 'TEST-REASON-2', + ('TEST-{}-REASON-{}', 'FORMAT', 3), ]) process_chain(self._mlist, self._msg, msgdata, start_chain='reject') bounces = get_queue_messages('virgin', expected_count=1) payload = bounces[0].msg.get_payload(0).as_string() self.assertIn('TEST-REASON-1', payload) self.assertIn('TEST-REASON-2', payload) + self.assertIn('TEST-FORMAT-REASON-3', payload) def test_no_reason(self): # There may be no moderation reasons. diff --git a/src/mailman/docs/NEWS.rst b/src/mailman/docs/NEWS.rst index f61233569..420e0fbe2 100644 --- a/src/mailman/docs/NEWS.rst +++ b/src/mailman/docs/NEWS.rst @@ -21,6 +21,8 @@ Bugs subject prefixing is fixed. (Closes #359) * Messages with no syntactically valid senders are now automatically discarded. (Closes #369) +* Various message holds and rejects that gave 'N/A' as a reason now give an + appropriate reason. (Closes #368) Command line ------------ diff --git a/src/mailman/rules/administrivia.py b/src/mailman/rules/administrivia.py index d3d18d693..bd4602149 100644 --- a/src/mailman/rules/administrivia.py +++ b/src/mailman/rules/administrivia.py @@ -89,5 +89,10 @@ class Administrivia: continue minargs, maxargs = EMAIL_COMMANDS[words[0]] if minargs <= len(words) - 1 <= maxargs: + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('Message contains administrivia')) return True return False diff --git a/src/mailman/rules/banned_address.py b/src/mailman/rules/banned_address.py index 77fb739a6..9fd011102 100644 --- a/src/mailman/rules/banned_address.py +++ b/src/mailman/rules/banned_address.py @@ -38,5 +38,11 @@ class BannedAddress: ban_manager = IBanManager(mlist) for sender in msg.senders: if ban_manager.is_banned(sender): + msgdata['moderation_sender'] = sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + (_('Message sender {} is banned from this list'), + sender)) return True return False diff --git a/src/mailman/rules/dmarc.py b/src/mailman/rules/dmarc.py index 90fff0855..9f9b11673 100644 --- a/src/mailman/rules/dmarc.py +++ b/src/mailman/rules/dmarc.py @@ -307,17 +307,23 @@ class DMARCMitigation: msgdata['dmarc'] = True if mlist.dmarc_mitigate_action is DMARCMitigateAction.discard: msgdata['moderation_action'] = 'discard' - msgdata['moderation_reasons'] = [_('DMARC moderation')] + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('DMARC moderation')) elif mlist.dmarc_mitigate_action is DMARCMitigateAction.reject: listowner = mlist.owner_address # noqa F841 - reason = (mlist.dmarc_moderation_notice or - _('You are not allowed to post to this mailing ' - 'list From: a domain which publishes a DMARC ' - 'policy of reject or quarantine, and your message' - ' has been automatically rejected. If you think ' - 'that your messages are being rejected in error, ' - 'contact the mailing list owner at ${listowner}.')) - msgdata['moderation_reasons'] = [wrap(reason)] + with _.defer_translation(): + # This will be translated at the point of use. + reason = (mlist.dmarc_moderation_notice or _( + 'You are not allowed to post to this mailing ' + 'list From: a domain which publishes a DMARC ' + 'policy of reject or quarantine, and your message' + ' has been automatically rejected. If you think ' + 'that your messages are being rejected in error, ' + 'contact the mailing list owner at ${listowner}.')) + msgdata.setdefault('moderation_reasons', []).append( + wrap(reason)) msgdata['moderation_action'] = 'reject' else: return False diff --git a/src/mailman/rules/emergency.py b/src/mailman/rules/emergency.py index ac512f391..3f8da002b 100644 --- a/src/mailman/rules/emergency.py +++ b/src/mailman/rules/emergency.py @@ -39,4 +39,11 @@ class Emergency: def check(self, mlist, msg, msgdata): """See `IRule`.""" - return mlist.emergency and not msgdata.get('moderator_approved') + if mlist.emergency and not msgdata.get('moderator_approved'): + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('Emergency moderation is in effect for this list')) + return True + return False diff --git a/src/mailman/rules/implicit_dest.py b/src/mailman/rules/implicit_dest.py index 7e6658d49..26729a86e 100644 --- a/src/mailman/rules/implicit_dest.py +++ b/src/mailman/rules/implicit_dest.py @@ -88,4 +88,9 @@ class ImplicitDestination: if re.match(escaped, recipient, re.IGNORECASE): return False # Nothing matched. + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('Message has implicit destination')) return True diff --git a/src/mailman/rules/loop.py b/src/mailman/rules/loop.py index ac4d00461..b50b55ed6 100644 --- a/src/mailman/rules/loop.py +++ b/src/mailman/rules/loop.py @@ -37,4 +37,11 @@ class Loop: # Has this message already been posted to this list? list_posts = set(value.strip().lower() for value in msg.get_all('list-post', [])) - return mlist.posting_address in list_posts + if mlist.posting_address in list_posts: + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('Message has already been posted to this list')) + return True + return False diff --git a/src/mailman/rules/max_recipients.py b/src/mailman/rules/max_recipients.py index 1f95f71cc..8a90fd451 100644 --- a/src/mailman/rules/max_recipients.py +++ b/src/mailman/rules/max_recipients.py @@ -41,4 +41,12 @@ class MaximumRecipients: # Figure out how many recipients there are recipients = getaddresses(msg.get_all('to', []) + msg.get_all('cc', [])) - return len(recipients) >= mlist.max_num_recipients + if len(recipients) >= mlist.max_num_recipients: + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + (_('Message has more than {} recipients'), + mlist.max_num_recipients)) + return True + return False diff --git a/src/mailman/rules/max_size.py b/src/mailman/rules/max_size.py index 46c8fc5ac..2554e0db0 100644 --- a/src/mailman/rules/max_size.py +++ b/src/mailman/rules/max_size.py @@ -39,4 +39,12 @@ class MaximumSize: assert hasattr(msg, 'original_size'), ( 'Message was not sized on initial parsing.') # The maximum size is specified in 1024 bytes. - return msg.original_size / 1024.0 > mlist.max_message_size + if msg.original_size / 1024.0 > mlist.max_message_size: + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + (_('The message is larger than the {} KB maximum size'), + mlist.max_message_size)) + return True + return False diff --git a/src/mailman/rules/moderation.py b/src/mailman/rules/moderation.py index 322216fb8..7f78d3ce7 100644 --- a/src/mailman/rules/moderation.py +++ b/src/mailman/rules/moderation.py @@ -84,9 +84,10 @@ class MemberModeration: # stored in the pending request table. msgdata['moderation_action'] = action.name msgdata['moderation_sender'] = sender - msgdata.setdefault('moderation_reasons', []).append( - # This will get translated at the point of use. - 'The message comes from a moderated member') + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('The message comes from a moderated member')) return True # The sender is not a member so this rule does not match. return False @@ -146,10 +147,12 @@ class NonmemberModeration: for addr in checklist: if ((addr.startswith('^') and re.match(addr, sender)) or addr == sender): # noqa: W503 - # The reason will get translated at the point of use. - reason = 'The sender is in the nonmember {} list' - _record_action(msgdata, action, sender, - reason.format(action)) + with _.defer_translation(): + # This will be translated at the point of use. + reason = ( + _('The sender is in the nonmember {} list'), + action) + _record_action(msgdata, action, sender, reason) return True action = (mlist.default_nonmember_action if nonmember.moderation_action is None @@ -160,9 +163,9 @@ class NonmemberModeration: elif action is not None: # We must stringify the moderation action so that it can be # stored in the pending request table. - # - # The reason will get translated at the point of use. - reason = 'The message is not from a list member' + with _.defer_translation(): + # This will be translated at the point of use. + reason = _('The message is not from a list member') _record_action(msgdata, action.name, sender, reason) return True # The sender must be a member, so this rule does not match. diff --git a/src/mailman/rules/news_moderation.py b/src/mailman/rules/news_moderation.py index a606197f0..3d3d39dc6 100644 --- a/src/mailman/rules/news_moderation.py +++ b/src/mailman/rules/news_moderation.py @@ -38,4 +38,11 @@ class ModeratedNewsgroup: def check(self, mlist, msg, msgdata): """See `IRule`.""" - return mlist.newsgroup_moderation == NewsgroupModeration.moderated + if mlist.newsgroup_moderation is NewsgroupModeration.moderated: + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('Post to a moderated newsgroup gateway')) + return True + return False diff --git a/src/mailman/rules/no_senders.py b/src/mailman/rules/no_senders.py index 7e8c78fa7..24e5fa5bd 100644 --- a/src/mailman/rules/no_senders.py +++ b/src/mailman/rules/no_senders.py @@ -37,9 +37,9 @@ class NoSenders: if msg.sender: return False else: - msgdata['moderation_action'] = 'discard' - msgdata['moderation_sender'] = _('None') - msgdata.setdefault('moderation_reasons', []).append( - # This will get translated at the point of use. - 'The message has no valid senders') + msgdata['moderation_sender'] = 'N/A' + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('The message has no valid senders')) return True diff --git a/src/mailman/rules/no_subject.py b/src/mailman/rules/no_subject.py index f4d2f6a5d..9793ca2a5 100644 --- a/src/mailman/rules/no_subject.py +++ b/src/mailman/rules/no_subject.py @@ -37,4 +37,11 @@ class NoSubject: # Convert the header value to a str because it may be an # email.header.Header instance. subject = str(msg.get('subject', '')).strip() - return subject == '' + if subject == '': + msgdata['moderation_sender'] = msg.sender + with _.defer_translation(): + # This will be translated at the point of use. + msgdata.setdefault('moderation_reasons', []).append( + _('Message has no subject')) + return True + return False diff --git a/src/mailman/rules/suspicious.py b/src/mailman/rules/suspicious.py index 9f0f0e922..1bf96d22a 100644 --- a/src/mailman/rules/suspicious.py +++ b/src/mailman/rules/suspicious.py @@ -41,7 +41,7 @@ class SuspiciousHeader: def check(self, mlist, msg, msgdata): """See `IRule`.""" return (mlist.bounce_matching_headers and - has_matching_bounce_header(mlist, msg)) + has_matching_bounce_header(mlist, msg, msgdata)) def _parse_matching_header_opt(mlist): @@ -77,7 +77,7 @@ bad regexp in bounce_matching_header line: %s return all -def has_matching_bounce_header(mlist, msg): +def has_matching_bounce_header(mlist, msg, msgdata): """Does the message have a matching bounce header? :param mlist: The mailing list the message is destined for. @@ -90,5 +90,11 @@ def has_matching_bounce_header(mlist, msg): # Convert the header value to a str because it may be an # email.header.Header instance. if cre.search(str(value)): + 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 bounce_matching_header line'), + str(value))) return True return False diff --git a/src/mailman/rules/tests/test_administrivia.py b/src/mailman/rules/tests/test_administrivia.py new file mode 100644 index 000000000..e370a0bf2 --- /dev/null +++ b/src/mailman/rules/tests/test_administrivia.py @@ -0,0 +1,51 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `administrivia` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.rules import administrivia +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestAdministrivia(unittest.TestCase): + """Test the administrivia rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_administrivia_returns_reason(self): + # Ensure administrivia rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: unsubscribe +Message-ID: + +A message body. +""") + rule = administrivia.Administrivia() + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + ['Message contains administrivia']) diff --git a/src/mailman/rules/tests/test_banned_address.py b/src/mailman/rules/tests/test_banned_address.py index 1ecab5b18..2d73ed3fb 100644 --- a/src/mailman/rules/tests/test_banned_address.py +++ b/src/mailman/rules/tests/test_banned_address.py @@ -74,6 +74,30 @@ A message body. result = rule.check(self._mlist, msg, {}) self.assertTrue(result) + def test_rule_returns_reason(self): + # Ensure a reason is returned. + user_manager = getUtility(IUserManager) + anne = user_manager.create_user('anne@example.com') + set_preferred(anne) + IBanManager(self._mlist).ban('anne@example.com') + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: A test message +Message-ID: +MIME-Version: 1.0 + +A message body. +""") + rule = banned_address.BannedAddress() + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual( + msgdata['moderation_reasons'], + [('Message sender {} is banned from this list', + 'anne@example.com')]) + def test_banned_address_linked_to_user(self): # Anne is subscribed to a mailing list as a user with her preferred # address. She also has a secondary address which is banned and which diff --git a/src/mailman/rules/tests/test_emergency.py b/src/mailman/rules/tests/test_emergency.py new file mode 100644 index 000000000..529742987 --- /dev/null +++ b/src/mailman/rules/tests/test_emergency.py @@ -0,0 +1,52 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `emergency` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.rules import emergency +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestEmergency(unittest.TestCase): + """Test the emergency rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_emergency_returns_reason(self): + # Ensure emergency rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: A Subject +Message-ID: + +A message body. +""") + rule = emergency.Emergency() + self._mlist.emergency = True + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + ['Emergency moderation is in effect for this list']) diff --git a/src/mailman/rules/tests/test_implicit_dest.py b/src/mailman/rules/tests/test_implicit_dest.py new file mode 100644 index 000000000..aed317f57 --- /dev/null +++ b/src/mailman/rules/tests/test_implicit_dest.py @@ -0,0 +1,51 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `implicit_dest` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.rules import implicit_dest +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestImplicitDestination(unittest.TestCase): + """Test the implicit_dest rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_implicit_dest_returns_reason(self): + # Ensure implicit_dest rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: bogus@example.com +Subject: A Subject +Message-ID: + +A message body. +""") + rule = implicit_dest.ImplicitDestination() + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + ['Message has implicit destination']) diff --git a/src/mailman/rules/tests/test_loop.py b/src/mailman/rules/tests/test_loop.py new file mode 100644 index 000000000..09f9fc905 --- /dev/null +++ b/src/mailman/rules/tests/test_loop.py @@ -0,0 +1,52 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `loop` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.rules import loop +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestLoop(unittest.TestCase): + """Test the loop rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_loop_returns_reason(self): + # Ensure loop rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: A Subject +List-Post: test@example.com +Message-ID: + +A message body. +""") + rule = loop.Loop() + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + ['Message has already been posted to this list']) diff --git a/src/mailman/rules/tests/test_max_recipients.py b/src/mailman/rules/tests/test_max_recipients.py new file mode 100644 index 000000000..08663f1e5 --- /dev/null +++ b/src/mailman/rules/tests/test_max_recipients.py @@ -0,0 +1,53 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `max_recipients` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.rules import max_recipients +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestMaximumRecipients(unittest.TestCase): + """Test the max_recipients rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_max_recipients_returns_reason(self): + # Ensure max_recipients rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Cc: anne@example.com, bill@example.com +Subject: A Subject +Message-ID: + +A message body. +""") + rule = max_recipients.MaximumRecipients() + self._mlist.max_num_recipients = 2 + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + [('Message has more than {} recipients', 2)]) diff --git a/src/mailman/rules/tests/test_max_size.py b/src/mailman/rules/tests/test_max_size.py new file mode 100644 index 000000000..56b279a6b --- /dev/null +++ b/src/mailman/rules/tests/test_max_size.py @@ -0,0 +1,55 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `max_size` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.rules import max_size +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestMaximumSize(unittest.TestCase): + """Test the max_size rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_max_size_returns_reason(self): + # Ensure max_size rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: A Subject +Message-ID: + +A message body. +""") + rule = max_size.MaximumSize() + self._mlist.max_message_size = 1 + # Fake the size. + msg.original_size = 2048 + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + [('The message is larger than the {} KB maximum size', + 1)]) diff --git a/src/mailman/rules/tests/test_news_moderation.py b/src/mailman/rules/tests/test_news_moderation.py new file mode 100644 index 000000000..aec9e2b14 --- /dev/null +++ b/src/mailman/rules/tests/test_news_moderation.py @@ -0,0 +1,53 @@ +# Copyright (C) 2016-2017 by the Free Software Foundation, Inc. +# +# This file is part of GNU Mailman. +# +# GNU Mailman is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free +# Software Foundation, either version 3 of the License, or (at your option) +# any later version. +# +# GNU Mailman is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for +# more details. +# +# You should have received a copy of the GNU General Public License along with +# GNU Mailman. If not, see . + +"""Test the `news_moderation` rule.""" + +import unittest + +from mailman.app.lifecycle import create_list +from mailman.interfaces.nntp import NewsgroupModeration +from mailman.rules import news_moderation +from mailman.testing.helpers import specialized_message_from_string as mfs +from mailman.testing.layers import ConfigLayer + + +class TestModeratedNewsgroup(unittest.TestCase): + """Test the news_moderation rule.""" + + layer = ConfigLayer + + def setUp(self): + self._mlist = create_list('test@example.com') + + def test_news_moderation_returns_reason(self): + # Ensure news_moderation rule returns a reason. + msg = mfs("""\ +From: anne@example.com +To: test@example.com +Subject: A Subject +Message-ID: + +A message body. +""") + rule = news_moderation.ModeratedNewsgroup() + self._mlist.newsgroup_moderation = NewsgroupModeration.moderated + msgdata = {} + result = rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + ['Post to a moderated newsgroup gateway']) diff --git a/src/mailman/rules/tests/test_no_senders.py b/src/mailman/rules/tests/test_no_senders.py index eb59dfe71..06bda5360 100644 --- a/src/mailman/rules/tests/test_no_senders.py +++ b/src/mailman/rules/tests/test_no_senders.py @@ -25,7 +25,7 @@ from mailman.rules import no_senders from mailman.testing.layers import ConfigLayer -class TestNoSubject(unittest.TestCase): +class TestNoSender(unittest.TestCase): """Test the no_senders rule.""" layer = ConfigLayer @@ -39,10 +39,9 @@ class TestNoSubject(unittest.TestCase): msgdata = {} result = self._rule.check(self._mlist, msg, msgdata) self.assertTrue(result) - self.assertEqual(msgdata['moderation_action'], 'discard') self.assertEqual(msgdata['moderation_reasons'], ['The message has no valid senders']) - self.assertEqual(msgdata['moderation_sender'], 'None') + self.assertEqual(msgdata['moderation_sender'], 'N/A') def test_message_has_sender(self): msg = Message() diff --git a/src/mailman/rules/tests/test_no_subject.py b/src/mailman/rules/tests/test_no_subject.py index 0379ea689..e80e2e4eb 100644 --- a/src/mailman/rules/tests/test_no_subject.py +++ b/src/mailman/rules/tests/test_no_subject.py @@ -46,3 +46,12 @@ class TestNoSubject(unittest.TestCase): msg['Subject'] = Header('Test subject') result = self._rule.check(self._mlist, msg, {}) self.assertFalse(result) + + def test_no_subject_returns_reason(self): + msg = Message() + msg['Subject'] = Header('') + msgdata = {} + result = self._rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual(msgdata['moderation_reasons'], + ['Message has no subject']) diff --git a/src/mailman/rules/tests/test_suspicious.py b/src/mailman/rules/tests/test_suspicious.py index b40292283..4012c89d5 100644 --- a/src/mailman/rules/tests/test_suspicious.py +++ b/src/mailman/rules/tests/test_suspicious.py @@ -42,3 +42,16 @@ class TestSuspicious(unittest.TestCase): self._mlist.bounce_matching_headers = 'from: spam@example.com' result = self._rule.check(self._mlist, msg, {}) self.assertFalse(result) + + def test_suspicious_returns_reason(self): + msg = Message() + msg['From'] = Header('spam@example.com') + self._mlist.bounce_matching_headers = 'from: spam@example.com' + msgdata = {} + result = self._rule.check(self._mlist, msg, msgdata) + self.assertTrue(result) + self.assertEqual( + msgdata['moderation_reasons'], + [('Header "{}" matched a bounce_matching_header line', + 'spam@example.com')] + ) -- cgit v1.2.3-70-g09d2