On 3/5/25 10:39 AM, Andreas Vögele wrote:
Giovanni Bechis writes:
On 3/3/25 8:22 PM, Andreas Vögele wrote:
Andreas Vögele writes:
Giovanni Bechis writes:
https://bz.apache.org/SpamAssassin/show_bug.cgi?id=8319
[...]
Fixed in a different way on r1924149.
I reverted last commit, atm tests passes with new and old Mail::DMARC versions.
The approach using 'eval' is hiding possible issues because it doesn't catch 
any errors.
If you have an saample email that breaks with current code, please send it to 
me.

I'm happy that the test suite succeeds now.

Problems aren't caused by emails but by invalid DMARC DNS records. You can see in the Mail::DMARC code that there are 
several code paths that return from "discover_policy" before the policy is set. The first code path commented 
with "9.1 ..." sets the result to 'none'. The checks for "too many policies" and "policy parse 
error" return without setting the result. In these cases, all Mail::DMARC versions before 1.20250203 use the default 
result 'fail' without a policy having been set, which causes $result->published to throw an exception.

I mean emails with invalid DMARC records

The "discover_policy" method's code:

https://github.com/msimerson/mail-dmarc/blob/df107c6784b19b7fcd3a2407fe61a2ca9db1f3ad/lib/Mail/DMARC/PurePerl.pm#L151

The most recent code in "validate" sets the result to 'none' if 
"discover_policy" doesn't return a policy. But older Mail::DMARC versions keep the 
default result 'fail' without setting a policy:

https://github.com/msimerson/mail-dmarc/blob/e5301eb43679e89460f1023e28e9c1fa62f74374/lib/Mail/DMARC/PurePerl.pm#L38

This will make sure that the code is executed only on new Mail::DMARC versions:

Index: lib/Mail/SpamAssassin/Plugin/DMARC.pm
===================================================================
--- lib/Mail/SpamAssassin/Plugin/DMARC.pm       (revision 1924179)
+++ lib/Mail/SpamAssassin/Plugin/DMARC.pm       (working copy)
@@ -391,8 +391,10 @@
     }
   }
- if(defined $result and ($result->result ne 'none') and ($result->published->can('stringify'))) {
-    dbg("Found DMARC record \"" . $result->published->stringify . "\" for domain 
$mfrom_domain");
+  if (version->parse(Mail::DMARC->VERSION) >= version->parse(1.20250203)) {
+    if(defined $result and ($result->result ne 'none') and 
($result->published->can('stringify'))) {
+      dbg("Found DMARC record \"" . $result->published->stringify . "\" for domain 
$mfrom_domain");
+    }
   }
# Report that DMARC failed but it has been overridden because of AAR headers

 Regards
  Giovanni

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature

Reply via email to