Commit 9cdcffbc by Barry Warsaw

Rename metadata key for clarity

parent e6326533
......@@ -45,6 +45,7 @@ elog = logging.getLogger('mailman.error')
blog = logging.getLogger('mailman.bounce')
DOT = '.'
NL = '\n'
@public
......@@ -56,8 +57,12 @@ def bounce_message(mlist, msg, error=None):
:param msg: The original message.
:type msg: `email.message.Message`
:param error: Optional exception causing the bounce. The exception
instance must have a `.message` attribute.
:type error: Exception
instance must have a `.message` attribute. The exception *may* have a
non-None `.reasons` attribute which would be a list of reasons for the
rejection, and it may have a non-None `.substitutions` attribute. The
latter, along with the formatted reasons will be interpolated into the
message (`.reasons` gets put into the `$reasons` placeholder).
:type error: RejectMessage
"""
# Bounce a message back to the sender, with an error message if provided
# in the exception argument. .sender might be None or the empty string.
......@@ -67,10 +72,9 @@ def bounce_message(mlist, msg, error=None):
return
subject = msg.get('subject', _('(no subject)'))
subject = oneline(subject, mlist.preferred_language.charset)
if error is None:
notice = _('[No bounce details are available]')
else:
notice = _(error.message)
notice = (_('[No bounce details are available]')
if error is None
else str(error))
# Currently we always craft bounces as MIME messages.
bmsg = UserNotification(msg.sender, mlist.owner_address, subject,
lang=mlist.preferred_language)
......
......@@ -12,12 +12,12 @@ Mailman can bounce messages back to the original sender. This is essentially
equivalent to rejecting the message with notification. Mailing lists can
bounce a message with an optional error message.
>>> mlist = create_list('text@example.com')
>>> mlist = create_list('ant@example.com')
Any message can be bounced.
>>> msg = message_from_string("""\
... To: text@example.com
... To: ant@example.com
... From: aperson@example.com
... Subject: Something important
...
......@@ -36,7 +36,7 @@ to the original message author.
1
>>> print(items[0].msg.as_string())
Subject: Something important
From: text-owner@example.com
From: ant-owner@example.com
To: aperson@example.com
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="..."
......@@ -54,7 +54,7 @@ to the original message author.
Content-Type: message/rfc822
MIME-Version: 1.0
<BLANKLINE>
To: text@example.com
To: ant@example.com
From: aperson@example.com
Subject: Something important
<BLANKLINE>
......@@ -63,18 +63,16 @@ to the original message author.
--...--
An error message can be given when the message is bounced, and this will be
included in the payload of the text/plain part. The error message must be
included in the payload of the ``text/plain`` part. The error message must be
passed in as an instance of a ``RejectMessage`` exception.
>>> from mailman.interfaces.pipeline import RejectMessage
>>> error = RejectMessage("This wasn't very important after all.")
>>> bounce_message(mlist, msg, error)
>>> items = get_queue_messages('virgin')
>>> len(items)
1
>>> items = get_queue_messages('virgin', expected_count=1)
>>> print(items[0].msg.as_string())
Subject: Something important
From: text-owner@example.com
From: ant-owner@example.com
To: aperson@example.com
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="..."
......@@ -92,10 +90,56 @@ passed in as an instance of a ``RejectMessage`` exception.
Content-Type: message/rfc822
MIME-Version: 1.0
<BLANKLINE>
To: text@example.com
To: ant@example.com
From: aperson@example.com
Subject: Something important
<BLANKLINE>
I sometimes say something important.
<BLANKLINE>
--...--
The ``RejectMessage`` exception can also include a set of reasons, which will
be interpolated into the message using the ``{reasons}`` placeholder.
>>> error = RejectMessage("""This message is rejected because:
...
... $reasons
... """, [
... 'I am not happy',
... 'You are not happy',
... 'We are not happy'])
>>> bounce_message(mlist, msg, error)
>>> items = get_queue_messages('virgin', expected_count=1)
>>> print(items[0].msg.as_string())
Subject: Something important
From: ant-owner@example.com
To: aperson@example.com
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="..."
Message-ID: ...
Date: ...
Precedence: bulk
<BLANKLINE>
--...
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
<BLANKLINE>
This message is rejected because:
<BLANKLINE>
I am not happy
You are not happy
We are not happy
<BLANKLINE>
--...
Content-Type: message/rfc822
MIME-Version: 1.0
<BLANKLINE>
To: ant@example.com
From: aperson@example.com
Subject: Something important
<BLANKLINE>
I sometimes say something important.
<BLANKLINE>
--...
<BLANKLINE>
......@@ -18,7 +18,6 @@
"""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
......@@ -28,24 +27,6 @@ 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:
"""A chain link."""
......@@ -107,6 +88,31 @@ class TerminalChainBase:
@public
@abstract_component
@implementer(IChain)
class JumpChainBase:
"""A base chain that simplifies jumping to another chain."""
def jump_to(self, mlist, msg, msgsdata):
"""Return the chain to jump to.
This must be overridden by subclasses.
:param mlist: The mailing list.
:param msg: The message.
:param msgdata: The message metadata.
:return: The name of the chain to jump to.
:rtype: str
"""
raise NotImplementedError
def get_links(self, mlist, msg, msgdata):
jump_chain = self.jump_to(mlist, msg, msgdata)
return iter([
Link('truth', LinkAction.jump, jump_chain),
])
@public
@abstract_component
@implementer(IMutableChain)
class Chain:
"""Generic chain base class."""
......
......@@ -41,7 +41,7 @@ class BuiltInChain:
# First check DMARC. For a reject or discard, the rule hits and we
# jump to the moderation chain to do the action. Otherwise, the rule
# misses buts sets msgdata['dmarc'] for the handler.
('dmarc-mitigation', LinkAction.jump, 'moderation'),
('dmarc-mitigation', LinkAction.jump, 'dmarc'),
# Discard emails with no valid senders.
('no-senders', LinkAction.jump, 'discard'),
('approved', LinkAction.jump, 'accept'),
......
# Copyright (C) 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 <http://www.gnu.org/licenses/>.
"""DMARC mitigation chain."""
from mailman.chains.base import JumpChainBase
from mailman.core.i18n import _
from public import public
@public
class DMARCMitigationChain(JumpChainBase):
"""Perform DMARC mitigation."""
name = 'dmarc'
description = _('Process DMARC reject or discard mitigations')
def jump_to(self, mlist, msg, msgdata):
# Which action should be taken?
jump_chain = msgdata['dmarc_action']
assert jump_chain in ('discard', 'reject'), (
'{}: Invalid DMARC action: {} for sender: {}'.format(
mlist.list_id, jump_chain,
msgdata.get('moderation_sender', '(unknown)')))
return jump_chain
......@@ -24,9 +24,9 @@ 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, format_reasons
from mailman.chains.base import TerminalChainBase
from mailman.config import config
from mailman.core.i18n import _
from mailman.core.i18n import _, format_reasons
from mailman.email.message import UserNotification
from mailman.interfaces.autorespond import IAutoResponseSet, Response
from mailman.interfaces.chain import HoldEvent
......
......@@ -34,17 +34,14 @@ made as to the disposition of the message. `defer` is the default for
members, while `hold` is the default for nonmembers.
"""
from mailman.chains.base import Link
from mailman.chains.base import JumpChainBase
from mailman.core.i18n import _
from mailman.interfaces.action import Action
from mailman.interfaces.chain import IChain, LinkAction
from public import public
from zope.interface import implementer
@public
@implementer(IChain)
class ModerationChain:
class ModerationChain(JumpChainBase):
"""Dynamically produce a link jumping to the appropriate terminal chain.
The terminal chain will be one of the Accept, Hold, Discard, or Reject
......@@ -53,13 +50,12 @@ class ModerationChain:
name = 'moderation'
description = _('Moderation chain')
def get_links(self, mlist, msg, msgdata):
"""See `IChain`."""
def jump_to(self, mlist, msg, msgdata):
# Get the moderation action from the message metadata. It can only be
# one of the expected values (i.e. not Action.defer). See the
# moderation.py rule for details. This is stored in the metadata as a
# string so that it can be stored in the pending table.
action = Action[msgdata.get('moderation_action')]
action = Action[msgdata.get('member_moderation_action')]
# defer is not a valid moderation action.
jump_chain = {
Action.accept: 'accept',
......@@ -68,9 +64,7 @@ class ModerationChain:
Action.reject: 'reject',
}.get(action)
assert jump_chain is not None, (
'{0}: Invalid moderation action: {1} for sender: {2}'.format(
mlist.fqdn_listname, action,
'{}: Invalid moderation action: {} for sender: {}'.format(
mlist.list_id, action,
msgdata.get('moderation_sender', '(unknown)')))
return iter([
Link('truth', LinkAction.jump, jump_chain),
])
return jump_chain
......@@ -20,11 +20,13 @@
import logging
from mailman.app.bounces import bounce_message
from mailman.chains.base import TerminalChainBase, format_reasons
from mailman.chains.base import TerminalChainBase
from mailman.core.i18n import _
from mailman.interfaces.chain import RejectEvent
from mailman.interfaces.pipeline import RejectMessage
from mailman.interfaces.template import ITemplateLoader
from public import public
from zope.component import getUtility
from zope.event import notify
......@@ -56,17 +58,10 @@ class RejectChain(TerminalChainBase):
if reasons is None:
error = None
else:
error = RejectMessage(_("""
Your message to the {list_name} mailing-list was rejected for the following
reasons:
{reasons}
The original message as received by Mailman is attached.
""").format(
list_name=mlist.display_name, # noqa: E122
reasons=NEWLINE.join(format_reasons(reasons))
))
template = getUtility(ITemplateLoader).get(
'list:user:notice:rejected', mlist)
error = RejectMessage(
template, reasons, dict(listname=mlist.display_name))
bounce_message(mlist, msg, error)
log.info('REJECT: %s', msg.get('message-id', 'n/a'))
notify(RejectEvent(mlist, msg, msgdata, self))
......@@ -51,7 +51,7 @@ class TestAccept(unittest.TestCase):
self._mlist = create_list('ant@example.com')
self._msg = mfs("""\
From: anne@example.com
To: test@example.com
To: ant@example.com
Subject: Ignore
""")
......
# Copyright (C) 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 <http://www.gnu.org/licenses/>.
"""Test for the DMARC chain."""
import unittest
from mailman.app.lifecycle import create_list
from mailman.core.chains import process as process_chain
from mailman.interfaces.chain import DiscardEvent, RejectEvent
from mailman.testing.helpers import (
event_subscribers, get_queue_messages,
specialized_message_from_string as mfs)
from mailman.testing.layers import ConfigLayer
class TestDMARC(unittest.TestCase):
layer = ConfigLayer
maxDiff = None
def setUp(self):
self._mlist = create_list('ant@example.com')
self._msg = mfs("""\
From: anne@example.com
To: test@example.com
Subject: Ignore
""")
def test_discard(self):
msgdata = dict(dmarc_action='discard')
# When a message is discarded, the only artifacts are a log message
# and an event. Catch the event to prove it happened.
events = []
def handler(event): # noqa: E306
if isinstance(event, DiscardEvent):
events.append(event)
with event_subscribers(handler):
process_chain(self._mlist, self._msg, msgdata, start_chain='dmarc')
self.assertEqual(len(events), 1)
self.assertIs(events[0].msg, self._msg)
def test_reject(self):
msgdata = dict(
dmarc_action='reject',
moderation_reasons=['DMARC violation'],
)
# When a message is reject, an event will be triggered and the message
# will be bounced.
events = []
def handler(event): # noqa: E306
if isinstance(event, RejectEvent):
events.append(event)
with event_subscribers(handler):
process_chain(self._mlist, self._msg, msgdata, start_chain='dmarc')
self.assertEqual(len(events), 1)
self.assertIs(events[0].msg, self._msg)
items = get_queue_messages('virgin', expected_count=1)
# Unpack the rejection message.
rejection = items[0].msg.get_payload(0).get_payload()
self.assertEqual(rejection, """\
Your message to the Ant mailing-list was rejected for the following
reasons:
DMARC violation
The original message as received by Mailman is attached.
""")
......@@ -80,7 +80,7 @@ def start_python(overrides, banner):
# Set the tab completion.
with ExitStack() as resources:
try: # pragma: nocover
import readline, rlcompleter # noqa: F401, E401
import readline, rlcompleter # noqa: F401,E401
except ImportError: # pragma: nocover
print(_('readline not available'), file=sys.stderr)
pass
......
......@@ -47,3 +47,21 @@ def initialize(application=None):
def handle_ConfigurationUpdatedEvent(event):
if isinstance(event, ConfigurationUpdatedEvent):
_.default = event.config.mailman.default_language
@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
......@@ -55,7 +55,7 @@ def process(mlist, msg, msgdata, pipeline_name='built-in'):
except RejectMessage as error:
vlog.info(
'{} rejected by "{}" pipeline handler "{}": {}'.format(
message_id, pipeline_name, handler.name, error.message))
message_id, pipeline_name, handler.name, str(error)))
bounce_message(mlist, msg, error)
......
......@@ -48,8 +48,11 @@ class DiscardingHandler:
class RejectHandler:
name = 'rejecting'
def __init__(self, message):
self.message = message
def process(self, mlist, msg, msgdata):
raise RejectMessage('by test handler')
raise RejectMessage(self.message)
@implementer(IPipeline)
......@@ -66,8 +69,11 @@ class RejectingPipeline:
name = 'test-rejecting'
description = 'Rejectinging test pipeline'
def __init__(self):
self.message = 'by test handler'
def __iter__(self):
yield RejectHandler()
yield RejectHandler(self.message)
class TestPostingPipeline(unittest.TestCase):
......@@ -109,18 +115,49 @@ testing
'"discarding": by test handler'))
def test_rejecting_pipeline(self):
# If a handler in the pipeline raises DiscardMessage, the message will
# be thrown away, but with a log message.
# If a handler in the pipeline raises RejectMessage, the post will
# be bounced with a log message.
mark = LogFileMark('mailman.vette')
process(self._mlist, self._msg, {}, 'test-rejecting')
line = mark.readline()[:-1]
self.assertTrue(line.endswith(
self.assertEqual(
line[-80:],
'<ant> rejected by "test-rejecting" pipeline handler '
'"rejecting": by test handler',
line)
# In the rejection case, the original message will also be in the
# virgin queue.
items = get_queue_messages('virgin', expected_count=1)
self.assertEqual(
str(items[0].msg.get_payload(1).get_payload(0)['subject']),
'a test')
# The first payload contains the rejection reason.
payload = items[0].msg.get_payload(0).get_payload()
self.assertEqual(payload, 'by test handler')
def test_rejecting_pipeline_without_message(self):
# Similar to above, but without a rejection message.
pipeline = config.pipelines['test-rejecting']
message = pipeline.message
self.addCleanup(setattr, pipeline, 'message', message)
pipeline.message = None
mark = LogFileMark('mailman.vette')
process(self._mlist, self._msg, {}, 'test-rejecting')
line = mark.readline()[:-1]
self.assertEqual(
line[-91:],
'<ant> rejected by "test-rejecting" pipeline handler '
'"rejecting": by test handler'))
'"rejecting": [No details are available]',
line)
# In the rejection case, the original message will also be in the
# virgin queue.
items = get_queue_messages('virgin', expected_count=1)
self.assertEqual(str(items[0].msg['subject']), 'a test')
self.assertEqual(
str(items[0].msg.get_payload(1).get_payload(0)['subject']),
'a test')
# The first payload contains the rejection reason.
payload = items[0].msg.get_payload(0).get_payload()
self.assertEqual(payload, '[No details are available]')
def test_decorate_bulk(self):
# Ensure that bulk postings get decorated with the footer.
......
......@@ -23,6 +23,7 @@ Bugs
discarded. (Closes #369)
* Various message holds and rejects that gave 'N/A' as a reason now give an
appropriate reason. (Closes #368)
* Bounce messages are now composed for proper translations.
Command line
------------
......@@ -39,6 +40,8 @@ Interfaces
both ``List-ID``s and fully qualified list names, since that's the most
common use case. There's now a separate ``.get_by_fqdn()`` which only
accepts the latter and mirrors the already existing ``.get_by_list_id()``.
* A new template ``list:user:notice:rejected`` has been added for customizing
the bounce message rejection notice.
Other
-----
......
......@@ -17,13 +17,12 @@
"""Interface for describing pipelines."""
from mailman.core.i18n import _, format_reasons
from public import public
from zope.interface import Attribute, Interface
# For i18n extraction.
def _(s):
return s
NL = '\n'
# These are thrown but they aren't exceptions so don't inherit from
......@@ -44,11 +43,20 @@ class DiscardMessage(BaseException):
class RejectMessage(BaseException):
"""The message will be bounced back to the sender"""
def __init__(self, message=None):
def __init__(self, message=None, reasons=None, substitutions=None):
self.message = message
self.reasons = reasons
self.substitutions = ({} if substitutions is None else substitutions)
def __str__(self):
return self.message
if self.message is None:
return _('[No details are available]')
reasons = (_('[No reasons given]')
if self.reasons is None
else NL.join(format_reasons(self.reasons)))
substitutions = self.substitutions.copy()
substitutions['reasons'] = reasons
return _(self.message, substitutions)
@public
......
......@@ -179,6 +179,7 @@ ALL_TEMPLATES = {
'list:user:notice:post',
'list:user:notice:probe',
'list:user:notice:refuse',
'list:user:notice:rejected',
'list:user:notice:welcome',
}
}
......
......@@ -90,15 +90,15 @@ def deliver(mlist, msg, msgdata):
if size is None:
size = len(msg.as_string())
substitutions = dict(
msgid = msg.get('message-id', 'n/a'), # noqa: E221, E251
listname = mlist.fqdn_listname, # noqa: E221, E251
sender = original_sender, # noqa: E221, E251
recip = len(original_recipients), # noqa: E221, E251
size = size, # noqa: E221, E251
time = t1 - t0, # noqa: E221, E251
refused = len(refused), # noqa: E221, E251
smtpcode = 'n/a', # noqa: E221, E251
smtpmsg = 'n/a', # noqa: E221, E251
msgid = msg.get('message-id', 'n/a'), # noqa: E221,E251
listname = mlist.fqdn_listname, # noqa: E221,E251
sender = original_sender, # noqa: E221,E251
recip = len(original_recipients), # noqa: E221,E251
size = size, # noqa: E221,E251
time = t1 - t0, # noqa: E221,E251
refused = len(refused), # noqa: E221,E251
smtpcode = 'n/a', # noqa: E221,E251
smtpmsg = 'n/a', # noqa: E221,E251
)
template = config.logging.smtp.every
if template.lower() != 'no':
......@@ -141,9 +141,9 @@ def deliver(mlist, msg, msgdata):
template = config.logging.smtp.failure
if template.lower() != 'no':
substitutions.update(
recip = recipient, # noqa: E221, E251
smtpcode = code, # noqa: E221, E251
smtpmsg = smtp_message, # noqa: E221, E251
recip = recipient, # noqa: E221,E251
smtpcode = code, # noqa: E221,E251
smtpmsg = smtp_message, # noqa: E221,E251
)
log.info('%s', expand(template, mlist, substitutions))
# Return the results
......
......@@ -346,6 +346,7 @@ class TestDomainTemplates(unittest.TestCase):
'list:user:notice:post': '',
'list:user:notice:probe': '',
'list:user:notice:refuse': '',
'list:user:notice:rejected': '',
'list:user:notice:welcome': 'http://example.org/welcome',
'password': 'some password',
'username': 'anne.person',
......
......@@ -705,6 +705,7 @@ class TestListTemplates(unittest.TestCase):
'list:user:notice:post': '',
'list:user:notice:probe': '',
'list:user:notice:refuse': '',
'list:user:notice:rejected': '',
'list:user:notice:welcome': 'http://example.org/welcome',
'password': 'some password',
'username': 'anne.person',
......
......@@ -283,6 +283,7 @@ class TestSiteTemplates(unittest.TestCase):
'list:user:notice:post': '',
'list:user:notice:probe': '',
'list:user:notice:refuse': '',
'list:user:notice:rejected': '',
'list:user:notice:welcome': 'http://example.org/welcome',
'password': 'some password',
'username': 'anne.person',
......
......@@ -298,15 +298,15 @@ class DMARCMitigation:
if mlist.dmarc_mitigate_action is DMARCMitigateAction.no_mitigation:
# Don't bother to check if we're not going to do anything.
return False
dn, addr = parseaddr(msg.get('from'))
if maybe_mitigate(mlist, addr):
display_name, address = parseaddr(msg.get('from'))
if maybe_mitigate(mlist, address):
# If dmarc_mitigate_action is discard or reject, this rule fires
# and jumps to the 'moderation' chain to do the actual discard.
# Otherwise, the rule misses but sets a flag for the dmarc handler
# to do the appropriate action.
msgdata['dmarc'] = True
if mlist.dmarc_mitigate_action is DMARCMitigateAction.discard:
msgdata['moderation_action'] = 'discard'
msgdata['dmarc_action'] = 'discard'
with _.defer_translation():
# This will be translated at the point of use.
msgdata.setdefault('moderation_reasons', []).append(
......@@ -324,9 +324,9 @@ class DMARCMitigation:
'contact the mailing list owner at ${listowner}.'))
msgdata.setdefault('moderation_reasons', []).append(
wrap(reason))
msgdata['moderation_action'] = 'reject'
msgdata['dmarc_action'] = 'reject'
else:
return False
msgdata['moderation_sender'] = addr
msgdata['moderation_sender'] = address
return True
return False
......@@ -102,7 +102,7 @@ message.
True
>>> dump_msgdata(msgdata)
dmarc : True
moderation_action : discard
dmarc_action : discard
moderation_reasons: ['DMARC moderation']
moderation_sender : aperson@example.biz
......@@ -121,7 +121,7 @@ We can reject the message with a default reason.