https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6884
--- Comment #14 from Mark Martinec <[email protected]> --- (In reply to comment #13) > Good catch on uribl.t as well. I'm moving this to a blocker status. > > BTW, re: "subroutines lookup_a, lookup_ptr, lookup_mx, and > lookup_mx_exists in Dns.pm are not used anywhere??? Drop? > (And the lookup_ns is only used during startup by is_dns_available() > in the same module.)" > > I would say not to drop them. Too likely we will need those functions at > some point in my opinion. > > Regards, > KAM Brace yourselves, here comes a biggie! Better not look at the diff :) there are just too many - following a couple of days of fixing & rewriting lead one thing to another, finding new code simplification opportunities and cleanups. Looks like all the tests are passing now, and our production mail server seems happy too. The original approach in URIDNSBL.pm just didn't hold water. Grouping rules under a zone and subrule hashes and launching a single DNS query for these rules lost information on which queried domains belong to each rule and what are their flags, leading to unrelated rules being hit when another rule triggered with the same zone and subrule but a different query domain. The changed approach now pretends to launch a DNS query for each uri rule, but avoids actual duplicate DNS queries by letting a new subroutine AsyncLoop::bgsend_and_start_lookup() take care or grouping equal DNS queries into a single launch, while dealing with individual callbacks behind the scenes. This has the side effect of simplifying several code sections in URIDNSBL.pm, ASN.pm, AskDNS.pm and Dns.pm, without increasing the number of DNS queries (in some cases even reducing it). It also fixes the problem in AskDNS.pm, issuing warnings about 'such lookup has already been issued'. It also revealed several unused code sections, leftovers from the distant past before URIDNSBL.pm and AsyncLoop came to be. While at it, it came naturally to also add a long-desired feature: +=item dns_query_restriction (allow|deny) domain1 domain2 ... + +Option allows disabling of rules which would result in a DNS query to one of +the listed domains. The first argument must be a literal C<allow> or C<deny>, +remaining arguments are domains names. + +Most DNS queries (with some exceptions) are subject to dns_query_restriction. +A domain to be queried is successively stripped-off of its leading labels +(thus yielding a series of its parent domains), and on each iterations a +check is made against an associative array generated by dns_query_restriction +options. Search stops at the first match (i.e. a tightest match), and the +matching entry with its C<allow> or C<deny> value then controls if a DNS query +is allowed to be launched. + +If no match is found an implicit default is to allow all queries. The purpose +of an explicit C<allow> entry is to be able to override a previously configured +C<deny> on the same domain or to override an entry (possibly still to be +configured) on one of its parent domains. Thus an 'allow zen.spamhaus.org' +with a 'deny spamhaus.org' would permit DNS queries on a specific DNS BL zone +but deny queries to other zones under the same parent domain 'spamhaus.org'. + +Domains are matched case-insensitively, no wildcards are recognized, +there should be no leading or trailing dot. + +Specifying blocked domains has a similar effect as setting a score of such +rules to zero, and can be a handy alternative to hunting for such rules +when a site policy does not allow certain DNS block lists to be queried. + +Example: + dns_query_restriction deny dnswl.org surbl.org + dns_query_restriction allow zen.spamhaus.org + dns_query_restriction deny spamhaus.org mailspike.net spamcop.net +=item clear_dns_query_restriction + +The option removes any entries entered by previous 'dns_query_restriction' +options, leaving the list empty, i.e. allowing DNS queries for any domain +(including any DNS BL zone). So here it comes: - a new subroutine AsyncLoop::bgsend_and_start_lookup() implements a commonly used idiom of calling bgsend(), followed by start_lookup(), while implementing a mechanism for grouping equal DNS queries, simplifying several other code sections; - rewrite URIDNSBL.pm to put the new mechanism to good use, fixing the particular Bug 6884 and test 5 in t/uribl.t; - fix the t/uribl.t, moving X_URIBL_FULL_NS from %patterns to %anti_patterns; - add a dns_query_restriction configuration option; - some cosmetic edits, like renaming some variables for consistency. trunk: Sending lib/Mail/SpamAssassin/AsyncLoop.pm Sending lib/Mail/SpamAssassin/Conf.pm Sending lib/Mail/SpamAssassin/Dns.pm Sending lib/Mail/SpamAssassin/DnsResolver.pm Sending lib/Mail/SpamAssassin/Plugin/ASN.pm Sending lib/Mail/SpamAssassin/Plugin/AskDNS.pm Sending lib/Mail/SpamAssassin/Plugin/DNSEval.pm Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm Committed revision 1434360. Please pay attention to any anomalies I may have missed ;) -- You are receiving this mail because: You are the assignee for the bug.
