Moved the dmarc_moderation_action checks from the Moderate handler to

the SpamDetect handler so that the Reject and Discard actions will be
done before the message might be held by header_filter_rules, and the
Wrap Message and Munge From actions will be done on messages held by
header_filter_rules if the message is approved.  (LP: #1334450)

3 files changed, 37 insertions, 28 deletions

diff --git a/Mailman/Handlers/Moderate.py b/Mailman/Handlers/Moderate.py
index 096eedce..4400d086 100644
--- a/Mailman/Handlers/Moderate.py
+++ b/Mailman/Handlers/Moderate.py
@@ -50,32 +50,7 @@ class ModeratedMemberPost(Hold.ModeratedPost):
 def process(mlist, msg, msgdata):
     if msgdata.get('approved'):
         return
-    # Before anything else, check DMARC if necessary.
-    msgdata['from_is_list'] = 0
-    dn, addr = parseaddr(msg.get('from'))
-    if addr and mlist.dmarc_moderation_action > 0:
-        if Utils.IsDMARCProhibited(mlist, addr):
-            # Note that for dmarc_moderation_action, 0 = Accept, 
-            #    1 = Munge, 2 = Wrap, 3 = Reject, 4 = Discard
-            if mlist.dmarc_moderation_action == 1:
-                msgdata['from_is_list'] = 1
-            elif mlist.dmarc_moderation_action == 2:
-                msgdata['from_is_list'] = 2
-            elif mlist.dmarc_moderation_action == 3:
-                # Reject
-                text = mlist.dmarc_moderation_notice
-                if text:
-                    text = Utils.wrap(text)
-                else:
-                    text = Utils.wrap(_(
-"""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)s."""))
-                raise Errors.RejectMessage, text
-            elif mlist.dmarc_moderation_action == 4:
-                raise Errors.DiscardMessage
-    # Then, is the poster a member or not?
+    # Is the poster a member or not?
     for sender in msg.get_senders():
         if mlist.isMember(sender):
             break
diff --git a/Mailman/Handlers/SpamDetect.py b/Mailman/Handlers/SpamDetect.py
index 9e01f623..85d8af58 100644
--- a/Mailman/Handlers/SpamDetect.py
+++ b/Mailman/Handlers/SpamDetect.py
@@ -29,11 +29,12 @@ import re
 
 from email.Errors import HeaderParseError
 from email.Header import decode_header
+from email.Utils import parseaddr
 
 from Mailman import mm_cfg
 from Mailman import Errors
 from Mailman import i18n
-from Mailman.Utils import GetCharSet
+from Mailman import Utils
 from Mailman.Handlers.Hold import hold_for_approval
 
 try:
@@ -83,6 +84,33 @@ def getDecodedHeaders(msg, cset='utf-8'):
 
 
 def process(mlist, msg, msgdata):
+    # Before anything else, check DMARC if necessary.  We do this as early
+    # as possible so reject/discard actions trump other holds/approvals and
+    # wrap/munge actions get flagged even for approved messages.
+    msgdata['from_is_list'] = 0
+    dn, addr = parseaddr(msg.get('from'))
+    if addr and mlist.dmarc_moderation_action > 0:
+        if Utils.IsDMARCProhibited(mlist, addr):
+            # Note that for dmarc_moderation_action, 0 = Accept, 
+            #    1 = Munge, 2 = Wrap, 3 = Reject, 4 = Discard
+            if mlist.dmarc_moderation_action == 1:
+                msgdata['from_is_list'] = 1
+            elif mlist.dmarc_moderation_action == 2:
+                msgdata['from_is_list'] = 2
+            elif mlist.dmarc_moderation_action == 3:
+                # Reject
+                text = mlist.dmarc_moderation_notice
+                if text:
+                    text = Utils.wrap(text)
+                else:
+                    text = Utils.wrap(_(
+"""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)s."""))
+                raise Errors.RejectMessage, text
+            elif mlist.dmarc_moderation_action == 4:
+                raise Errors.DiscardMessage
     if msgdata.get('approved'):
         return
     # First do site hard coded header spam checks
@@ -98,7 +126,7 @@ def process(mlist, msg, msgdata):
     # extension may be a clue to possible virus/spam.
     headers = ''
     # Get the character set of the lists preferred language for headers
-    lcset = GetCharSet(mlist.preferred_language)
+    lcset = Utils.GetCharSet(mlist.preferred_language)
     for p in msg.walk():
         headers += getDecodedHeaders(p, lcset)
     for patterns, action, empty in mlist.header_filter_rules:
diff --git a/NEWS b/NEWS
index af46d7ad..445174a2 100755
--- a/NEWS
+++ b/NEWS
@@ -23,6 +23,12 @@ Here is a history of user visible changes to Mailman.
 
   Bug fixes and other patches
 
+    - Moved the dmarc_moderation_action checks from the Moderate handler to
+      the SpamDetect handler so that the Reject and Discard actions will be
+      done before the message might be held by header_filter_rules, and the
+      Wrap Message and Munge From actions will be done on messages held by
+      header_filter_rules if the message is approved.  (LP: #1334450)
+
     - <label> tags have been added around most check boxes and radio buttons
       and their text labels in the admin and admindb web GUI so they can be
       (de)selected by clicking the text.  (LP: #266391)