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
OpenPGP_signature.asc
Description: OpenPGP digital signature