diff options
| -rwxr-xr-x | Mailman/Handlers/CookHeaders.py | 54 | ||||
| -rw-r--r-- | Mailman/Handlers/WrapMessage.py | 5 | ||||
| -rwxr-xr-x | NEWS | 4 | ||||
| -rw-r--r-- | tests/test_handlers.py | 41 |
4 files changed, 84 insertions, 20 deletions
diff --git a/Mailman/Handlers/CookHeaders.py b/Mailman/Handlers/CookHeaders.py index 5b20f23c..0139f3c3 100755 --- a/Mailman/Handlers/CookHeaders.py +++ b/Mailman/Handlers/CookHeaders.py @@ -1,4 +1,4 @@ -# Copyright (C) 1998-2014 by the Free Software Foundation, Inc. +# Copyright (C) 1998-2015 by the Free Software Foundation, Inc. # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -15,7 +15,10 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, # USA. -"""Cook a message's Subject header.""" +"""Cook a message's Subject header. +Also do other manipulations of From:, Reply-To: and Cc: depending on +list configuration. +""" from __future__ import nested_scopes import re @@ -68,7 +71,7 @@ def change_header(name, value, mlist, msg, msgdata, delete=True, repl=True): if ((msgdata.get('from_is_list') == 2 or (msgdata.get('from_is_list') == 0 and mlist.from_is_list == 2)) and not msgdata.get('_fasttrack') - ) or name.lower() in ('from', 'reply-to'): + ) or name.lower() in ('from', 'reply-to', 'cc'): msgdata.setdefault('add_header', {})[name] = value elif repl or not msg.has_key(name): if delete: @@ -154,6 +157,23 @@ def process(mlist, msg, msgdata): # augment it. RFC 2822 allows max one Reply-To: header so collapse them # if we're adding a value, otherwise don't touch it. (Should we collapse # in all cases?) + # MAS: We need to do some things with the original From: if we've munged + # it for DMARC mitigation. We have goals for this process which are + # not completely compatible, so we do the best we can. Our goals are: + # 1) as long as the list is not anonymous, the original From: address + # should be obviously exposed, i.e. not just in a header that MUAs + # don't display. + # 2) the original From: address should not be in a comment or display + # name in the new From: because it is claimed that multiple domains + # in any fields in From: are indicative of spamminess. This means + # it should be in Reply-To: or Cc:. + # 3) the behavior of an MUA doing a 'reply' or 'reply all' should be + # consistent regardless of whether or not the From: is munged. + # Goal 3) implies sometimes the original From: should be in Reply-To: + # and sometimes in Cc:, and even so, this goal won't be achieved in + # all cases with all MUAs. In cases of conflict, the above ordering of + # goals is priority order. + if not fasttrack: # A convenience function, requires nested scopes. pair is (name, addr) new = [] @@ -171,13 +191,20 @@ def process(mlist, msg, msgdata): # the original Reply-To:'s to the list we're building up. In both # cases we'll zap the existing field because RFC 2822 says max one is # allowed. + o_rt = False if not mlist.first_strip_reply_to: orig = msg.get_all('reply-to', []) for pair in getaddresses(orig): + # There's an original Reply-To: and we're not removing it. add(pair) - # We also need to put the old From: in Reply-To: in all cases. - if o_from: + o_rt = True + # We also need to put the old From: in Reply-To: in all cases where + # it is not going in Cc:. This is when reply_goes_to_list == 0 and + # either there was no original Reply-To: or we stripped it. + if o_from and mlist.reply_goes_to_list == 0 and not o_rt: add(o_from) + # Flag that we added it. + o_from = None # Set Reply-To: header to point back to this list. Add this last # because some folks think that some MUAs make it easier to delete # addresses from the right than from the left. @@ -206,13 +233,19 @@ def process(mlist, msg, msgdata): # because even though the list address is in From:, the Reply-To: # poster will override it. Brain dead MUAs may then address the list # twice on a 'reply all', but reasonable MUAs should do the right - # thing. - if (mlist.personalize == 2 and mlist.reply_goes_to_list <> 1 and - not mlist.anonymous_list): + # thing. We also add the original From: to Cc: if it wasn't added + # to Reply-To: + add_list = (mlist.personalize == 2 and + mlist.reply_goes_to_list <> 1 and + not mlist.anonymous_list) + if add_list or o_from: # Watch out for existing Cc headers, merge, and remove dups. Note # that RFC 2822 says only zero or one Cc header is allowed. new = [] d = {} + # If we're adding the original From:, add it first. + if o_from: + add(o_from) # AvoidDuplicates may have set a new Cc: in msgdata.add_header, # so check that. if (msgdata.has_key('add_header') and @@ -222,8 +255,9 @@ def process(mlist, msg, msgdata): else: for pair in getaddresses(msg.get_all('cc', [])): add(pair) - i18ndesc = uheader(mlist, mlist.description, 'Cc') - add((str(i18ndesc), mlist.GetListEmail())) + if add_list: + i18ndesc = uheader(mlist, mlist.description, 'Cc') + add((str(i18ndesc), mlist.GetListEmail())) change_header('Cc', COMMASPACE.join([formataddr(pair) for pair in new]), mlist, msg, msgdata) diff --git a/Mailman/Handlers/WrapMessage.py b/Mailman/Handlers/WrapMessage.py index 4fd88ed9..fba6bc2a 100644 --- a/Mailman/Handlers/WrapMessage.py +++ b/Mailman/Handlers/WrapMessage.py @@ -1,4 +1,4 @@ -# Copyright (C) 2013-2014 by the Free Software Foundation, Inc. +# Copyright (C) 2013-2015 by the Free Software Foundation, Inc. # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -49,6 +49,9 @@ def process(mlist, msg, msgdata): if a_h.get('Reply-To'): del msg['reply-to'] msg['Reply-To'] = a_h.get('Reply-To') + if a_h.get('Cc'): + del msg['cc'] + msg['Cc'] = a_h.get('Cc') return # There are various headers in msg that we don't want, so we basically @@ -40,6 +40,10 @@ Here is a history of user visible changes to Mailman. Bug fixes and other patches + - When applying DMARC mitigations, CookHeaders now adds the original From: + to Cc: rather than Reply-To: in some cases to make MUA 'reply' and + 'reply all' more consistent with the non-DMARC cases. (LP: #1407098) + - The Subject: of the list welcome message wasn't always in the user's preferred language. Fixed. (LP: #1400988) diff --git a/tests/test_handlers.py b/tests/test_handlers.py index c3df0cb8..8f6219e8 100644 --- a/tests/test_handlers.py +++ b/tests/test_handlers.py @@ -1,4 +1,4 @@ -# Copyright (C) 2001-2014 by the Free Software Foundation, Inc. +# Copyright (C) 2001-2015 by the Free Software Foundation, Inc. # # This program is free software; you can redistribute it and/or # modify it under the terms of the GNU General Public License @@ -614,8 +614,11 @@ From: aperson@dom.ain msgdata = {} CookHeaders.process(mlist, msg, msgdata) eq(msgdata['add_header']['Reply-To'], - 'aperson@dom.ain, _xtest@dom.ain') + '_xtest@dom.ain') + eq(msgdata['add_header']['Cc'], + 'aperson@dom.ain') eq(msg.get_all('reply-to'), None) + eq(msg.get_all('cc'), None) def test_reply_to_list_with_strip(self): eq = self.assertEqual @@ -647,8 +650,12 @@ Reply-To: bperson@dom.ain msgdata = {} CookHeaders.process(mlist, msg, msgdata) eq(msgdata['add_header']['Reply-To'], - 'aperson@dom.ain, _xtest@dom.ain') + '_xtest@dom.ain') + eq(msgdata['add_header']['Cc'], + 'aperson@dom.ain') eq(msg.get_all('reply-to'), ['bperson@dom.ain']) + eq(msg.get_all('cc'), None) + def test_reply_to_explicit(self): eq = self.assertEqual @@ -678,8 +685,11 @@ From: aperson@dom.ain msgdata = {} CookHeaders.process(mlist, msg, msgdata) eq(msgdata['add_header']['Reply-To'], - 'mlist@dom.ain, aperson@dom.ain') + 'mlist@dom.ain') + eq(msgdata['add_header']['Cc'], + 'aperson@dom.ain') eq(msg.get_all('reply-to'), None) + eq(msg.get_all('cc'), None) def test_reply_to_explicit_with_strip(self): eq = self.assertEqual @@ -715,8 +725,11 @@ Reply-To: bperson@dom.ain CookHeaders.process(self._mlist, msg, msgdata) eq(msgdata['add_header']['Reply-To'], - 'mlist@dom.ain, aperson@dom.ain') + 'mlist@dom.ain') + eq(msgdata['add_header']['Cc'], + 'aperson@dom.ain') eq(msg.get_all('reply-to'), ['bperson@dom.ain']) + eq(msg.get_all('cc'), None) def test_reply_to_extends_to_list(self): eq = self.assertEqual @@ -750,7 +763,11 @@ Reply-To: bperson@dom.ain CookHeaders.process(mlist, msg, msgdata) eq(msgdata['add_header']['Reply-To'], - 'bperson@dom.ain, aperson@dom.ain, _xtest@dom.ain') + 'bperson@dom.ain, _xtest@dom.ain') + eq(msgdata['add_header']['Cc'], + 'aperson@dom.ain') + eq(msg.get_all('reply-to'), ['bperson@dom.ain']) + eq(msg.get_all('cc'), None) def test_reply_to_extends_to_explicit(self): eq = self.assertEqual @@ -784,7 +801,11 @@ Reply-To: bperson@dom.ain msgdata = {} CookHeaders.process(mlist, msg, msgdata) eq(msgdata['add_header']['Reply-To'], - 'mlist@dom.ain, bperson@dom.ain, aperson@dom.ain') + 'mlist@dom.ain, bperson@dom.ain') + eq(msgdata['add_header']['Cc'], + 'aperson@dom.ain') + eq(msg.get_all('reply-to'), ['bperson@dom.ain']) + eq(msg.get_all('cc'), None) def test_list_headers_nolist(self): eq = self.assertEqual @@ -980,7 +1001,8 @@ Content-Transfer-Encoding: 7bit Content-Disposition: inline footer ---BOUNDARY--""") +--BOUNDARY-- +""") def test_image(self): eq = self.assertEqual @@ -1749,7 +1771,8 @@ class TestReplybot(TestBase): class TestSpamDetect(TestBase): def test_short_circuit(self): msgdata = {'approved': 1} - rtn = SpamDetect.process(self._mlist, None, msgdata) + msg = email.message_from_string('') + rtn = SpamDetect.process(self._mlist, msg, msgdata) # Not really a great test, but there's little else to assert self.assertEqual(rtn, None) |