1 files changed, 51 insertions, 46 deletions

diff --git a/src/mailman/handlers/dmarc.py b/src/mailman/handlers/dmarc.py
index 99d8e08df..13707d397 100644
--- a/src/mailman/handlers/dmarc.py
+++ b/src/mailman/handlers/dmarc.py
@@ -43,6 +43,7 @@ from zope.interface import implementer
 log = logging.getLogger('mailman.error')
 
 COMMASPACE = ', '
+EMPTYSTRING = ''
 MAXLINELEN = 78
 NONASCII = re.compile('[^\s!-~]')
 # Headers from the original that we want to keep in the wrapper.  These are
@@ -84,48 +85,49 @@ def munged_headers(mlist, msg, msgdata):
     # goals is priority order.
     #
     # Be as robust as possible here.
-    faddrs = getaddresses(msg.get_all('from', []))
+    all_froms = getaddresses(msg.get_all('from', []))
     # Strip the nulls and bad emails.
-    faddrs = [x for x in faddrs if x[1].find('@') > 0]
-    if len(faddrs) == 1:
-        realname, email = o_from = faddrs[0]
+    froms = [email for email in all_froms if '@' in email[1]]
+    if len(froms) == 1:
+        realname, email = original_from = froms[0]
     else:
         # No From: or multiple addresses.  Just punt and take
         # the get_sender result.
         realname = ''
         email = msgdata['original_sender']
-        o_from = (realname, email)
+        original_from = (realname, email)
+    # If there was no display name in the email header, see if we have a
+    # matching member with a display name.
     if len(realname) == 0:
         member = mlist.members.get_member(email)
         if member:
             realname = member.display_name or email
         else:
             realname = email
-    # Remove domain from realname if it looks like an email address.
+    # Remove the domain from realname if it looks like an email address.
     realname = re.sub(r'@([^ .]+\.)+[^ .]+$', '---', realname)
     # Make a display name and RFC 2047 encode it if necessary.  This is
-    # difficult and kludgy. If the realname came from From: it should be
-    # ascii or RFC 2047 encoded. If it came from the list, it should be
-    # a string.  If it's from the email address, it should be an ascii string.
-    # In any case, ensure it's an unencoded string.
-    srn = ''
-    for frag, cs in decode_header(realname):
-        if not cs:
-            # Character set should be ascii, but use iso-8859-1 anyway.
-            cs = 'iso-8859-1'
-        if not isinstance(frag, str):
-            srn += str(frag, cs, errors='replace')
+    # difficult and kludgy.  If the realname came from From: it should be
+    # ASCII or RFC 2047 encoded.  If it came from the member record, it should
+    # be a string.  If it's from the email address, it should be an ASCII
+    # string.  In any case, ensure it's an unencoded string.
+    realname_bits = []
+    for fragment, charset in decode_header(realname):
+        if not charset:
+            # Character set should be ASCII, but use iso-8859-1 anyway.
+            charset = 'iso-8859-1'
+        if not isinstance(fragment, str):
+            realname_bits.append(str(fragment, charset, errors='replace'))
         else:
-            srn += frag
-    # The list's real_name is a string.
-    lrn = mlist.display_name      # noqa  F841
-    realname = srn
+            realname_bits.append(fragment)
+    # The member's display name is a string.
+    realname = EMPTYSTRING.join(realname_bits)
     # Ensure the i18n context is the list's preferred_language.
     with _.using(mlist.preferred_language.code):
-        via = _('$realname via $lrn')
+        via = _('$realname via $mlist.display_name')
     # Get an RFC 2047 encoded header string.
-    dn = str(Header(via, mlist.preferred_language.charset))
-    retn = [('From', formataddr((dn, mlist.posting_address)))]
+    display_name = str(Header(via, mlist.preferred_language.charset))
+    value = [('From', formataddr((display_name, mlist.posting_address)))]
     # We've made the munged From:.  Now put the original in Reply-To: or Cc:
     if mlist.reply_goes_to_list is ReplyToMunging.no_munging:
         # Add original from to Reply-To:
@@ -133,52 +135,54 @@ def munged_headers(mlist, msg, msgdata):
     else:
         # Add original from to Cc:
         add_to = 'Cc'
-    orig = getaddresses(msg.get_all(add_to, []))
-    if o_from[1] not in [x[1] for x in orig]:
-        orig.append(o_from)
-    retn.append((add_to, COMMASPACE.join(formataddr(x) for x in orig)))
-    return retn
+    original = getaddresses(msg.get_all(add_to, []))
+    if original_from[1] not in [x[1] for x in original]:
+        original.append(original_from)
+    value.append((add_to, COMMASPACE.join(formataddr(x) for x in original)))
+    return value
 
 
 def munge_from(mlist, msg, msgdata):
-    for k, v in munged_headers(mlist, msg, msgdata):
-        del msg[k]
-        msg[k] = v
+    for key, value in munged_headers(mlist, msg, msgdata):
+        del msg[key]
+        msg[key] = value
     return
 
 
 def wrap_message(mlist, msg, msgdata):
     # Create a wrapper message around the original.
+    #
     # There are various headers in msg that we don't want, so we basically
-    # make a copy of the msg, then delete almost everything and set/copy
+    # make a copy of the message, then delete almost everything and set/copy
     # what we want.
-    omsg = copy.deepcopy(msg)
+    original_msg = copy.deepcopy(msg)
     for key in msg:
         keep = False
         for keeper in KEEPERS:
-            if re.match(keeper, key, re.I):
+            if re.match(keeper, key, re.IGNORECASE):
                 keep = True
                 break
         if not keep:
             del msg[key]
     msg['MIME-Version'] = '1.0'
     msg['Message-ID'] = make_msgid()
-    for k, v in munged_headers(mlist, omsg, msgdata):
-        msg[k] = v
+    for key, value in munged_headers(mlist, original_msg, msgdata):
+        msg[key] = value
     # Are we including dmarc_wrapped_message_text?
     if len(mlist.dmarc_wrapped_message_text) > 0:
-        part1 = MIMEText(wrap(mlist.dmarc_wrapped_message_text),
-                         'plain',
-                         mlist.preferred_language.charset)
+        part1 = MIMEText(
+            wrap(mlist.dmarc_wrapped_message_text),
+            'plain',
+            mlist.preferred_language.charset)
         part1['Content-Disposition'] = 'inline'
-        part2 = MIMEMessage(omsg)
+        part2 = MIMEMessage(original_msg)
         part2['Content-Disposition'] = 'inline'
         msg['Content-Type'] = 'multipart/mixed'
         msg.set_payload([part1, part2])
     else:
         msg['Content-Type'] = 'message/rfc822'
         msg['Content-Disposition'] = 'inline'
-        msg.set_payload([omsg])
+        msg.set_payload([original_msg])
     return
 
 
@@ -186,7 +190,7 @@ def process(mlist, msg, msgdata):
     # If we're mitigating on policy and we have no hit, return.
     if not msgdata.get('dmarc') and not mlist.dmarc_mitigate_unconditionally:
         return
-    # if we're not mitigating, return.
+    # If we're not mitigating, return.
     if mlist.dmarc_mitigate_action is DMARCMitigateAction.no_mitigation:
         return
     if mlist.anonymous_list:
@@ -198,10 +202,11 @@ def process(mlist, msg, msgdata):
         elif mlist.dmarc_mitigate_action is DMARCMitigateAction.wrap_message:
             wrap_message(mlist, msg, msgdata)
         else:
-            # We can get here if DMARCMitigateAction is reject or discard
-            # but the From: domain has no reject or quarantine policy and
-            # mlist.dmarc_mitigate_unconditionally is True.  We just ignore
+            # We can get here if DMARCMitigateAction is reject or discard but
+            # the From: domain has no reject or quarantine policy and
+            # mlist.dmarc_mitigate_unconditionally is True.  Log and ignore
             # this.
+            log.error('Invalid DMARC combination for list: %s', mlist)
             return
     else:
         raise AssertionError(