Bug 7231 - Net::DNS 1.01 returns answers formatted differently, breaks SA
Summary: Net::DNS 1.01 returns answers formatted differently, breaks SA
Status: RESOLVED FIXED
Alias: None
Product: Spamassassin
Classification: Unclassified
Component: Libraries (show other bugs)
Version: 3.4.1
Hardware: All All
: P2 major
Target Milestone: 3.4.2
Assignee: SpamAssassin Developer Mailing List
URL:
Whiteboard:
Keywords:
: 7338 7339 (view as bug list)
Depends on:
Blocks:
 
Reported: 2015-07-31 17:50 UTC by Mark Martinec
Modified: 2017-06-08 22:47 UTC (History)
7 users (show)


Attachment Type Modified Status Actions Submitter/CLA Status
A minimalistic patch for URIDNSBL.pm to fix Net::DNS 1.01 incompatibility patch None Mark Martinec [HasCLA]
Suggested patch for trunk: compatibility with Net::DNS patch None Mark Martinec [HasCLA]
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Martinec 2015-07-31 17:50:18 UTC 
With Net::DNS 0.83 one gets A resource records in a DNS answer
formatted like:

$ perl -MNet::DNS::Resolver -le '
  $p=Net::DNS::Resolver->new->query("api.jetlore.com","A");
  print $_->string for $p->answer'

api.jetlore.com.        181     IN      CNAME   api-production-483399471.us-west-1.elb.amazonaws.com.
api-production-483399471.us-west-1.elb.amazonaws.com.   52      IN      A       184.169.180.44
api-production-483399471.us-west-1.elb.amazonaws.com.   52      IN      A       184.169.172.167
api-production-483399471.us-west-1.elb.amazonaws.com.   52      IN      A       50.18.178.63
api-production-483399471.us-west-1.elb.amazonaws.com.   52      IN      A       50.18.171.17
api-production-483399471.us-west-1.elb.amazonaws.com.   52      IN      A       184.72.41.175
api-production-483399471.us-west-1.elb.amazonaws.com.   52      IN      A       50.18.175.224

now with Net::DNS 1.01 one gets IP addresses in parenthesis:

$ perl -MNet::DNS::Resolver -le '
  $p=Net::DNS::Resolver->new->query("api.jetlore.com","A");
  print $_->string for $p->answer'

api.jetlore.com.        252     IN      CNAME   (
        api-production-483399471.us-west-1.elb.amazonaws.com. )
api-production-483399471.us-west-1.elb.amazonaws.com.   12      IN      A       (
        50.18.175.224 )
api-production-483399471.us-west-1.elb.amazonaws.com.   12      IN      A       (
        50.18.178.63 )
api-production-483399471.us-west-1.elb.amazonaws.com.   12      IN      A       (
        184.169.180.44 )
api-production-483399471.us-west-1.elb.amazonaws.com.   12      IN      A       (
        184.169.172.167 )
api-production-483399471.us-west-1.elb.amazonaws.com.   12      IN      A       (
        50.18.171.17 )
api-production-483399471.us-west-1.elb.amazonaws.com.   12      IN      A       (
        184.72.41.175 )

... which is syntactically still correct, but breaks our parsing
of replies at least in Plugin/URIDNSBL.pm, and likely elsewhere.

The immediate telltale sign are warnings like:

_WARN: Use of uninitialized value $4 in concatenation (.) or string at /usr/local/lib/perl5/site_perl/Mail/SpamAssassin/Plugin/URIDNSBL.pm line 1045.
_WARN: Use of uninitialized value $3 in concatenation (.) or string at /usr/local/lib/perl5/site_perl/Mail/SpamAssassin/Plugin/URIDNSBL.pm line 1045.
_WARN: Use of uninitialized value $2 in concatenation (.) or string at /usr/local/lib/perl5/site_perl/Mail/SpamAssassin/Plugin/URIDNSBL.pm line 1045.
Comment 1 Mark Martinec 2015-07-31 18:19:01 UTC 
Modules which deal with resource records in DNS answers more carefully
(like calling $rr->rdatastr or $rr->char_str_list or $rr->txtdata)
are not affected (plugins ASN, AskDSN, Dns::process_dnsbl_result).

The breakage is only where the result is pulled from $rr->string
and then a manual parse is attempted (apparently subroutine
Dns::dnsbl_hit(), and in several corners of Plugin/URIDNSBL.pm).
Comment 2 Mark Martinec 2015-07-31 18:33:33 UTC 
Broken in:
  Plugin::URIDNSBL::complete_ns_lookup
  Plugin::URIDNSBL::complete_a_lookup
Comment 3 Mark Martinec 2015-08-04 13:12:08 UTC 
Created attachment 5320 [details]
A minimalistic patch for URIDNSBL.pm to fix Net::DNS 1.01 incompatibility

This minimalistic patch is intended for 3.4.2 and package maintainers
to fix the immediate problem of incompatibility with Net::DNS 1.01.

A more comprehensive patch will follow, intended for trunk.
Comment 4 Mark Martinec 2015-08-04 14:17:08 UTC 
Created attachment 5321 [details]
Suggested patch for trunk: compatibility with Net::DNS

Checking the documentation of Net::DNS it turns out that a method
rdatastr() on a resource record object (Net::DNS::RR) is undocumented,
but there is an equivalent and documented method rdstring().

The rdstring() appeared in version Net::DNS 0.69. Currently at
Net::DNS 1.01 the rdatastr() still exists and calls rdstring,
but considering that it is undocumented it would be better to
avoid using the old method - one can never be sure when it will
disappear.

Another point is a method address(), which is applicable to
objects of type Net::DNS::RR::A and Net::DNS::RR::AAAA,
and returns an IP address in textual form. As it happens
it is much like the more generic rdstring(), but that might
change in future. The address() method appeared in version
Net::DNS 0.69.

The attached patch makes use of the new methods, but falls back
to pre-0.69 if necessary. I'd like to fold it into trunk.

Alternatively, if we decide that the minimal necessary version
of Net::DNS is 0.69 (December 2012), then all these compatibility
fallbacks can be avoided, greatly simplifying the change.
Comment 5 Kevin A. McGrail 2015-08-04 18:40:23 UTC 
For me, 2012 is a bit too recent to drop the old code.
Comment 6 Quanah Gibson-Mount 2015-08-04 20:47:01 UTC 
(In reply to Kevin A. McGrail from comment #5)
> For me, 2012 is a bit too recent to drop the old code.

I agree, there are plenty of supported OSes out there that predate 2012.

Just as one example, RHEL6 released in 2010.
Comment 7 Mark Martinec 2015-08-04 23:12:21 UTC 
Thanks for the feedback.
Ok, so here goes the patch with all compatibility
measures in place (i.e. the second attachment):

trunk:
  Sending lib/Mail/SpamAssassin/Dns.pm
  Sending lib/Mail/SpamAssassin/Plugin/AskDNS.pm
  Sendinglib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Committed revision 1694122.
Comment 8 Mark Martinec 2015-08-04 23:18:27 UTC 
... and the minimalistic patch to the 3.4 branch (the first attachment)
with no future-proofing complications:

3.4:
  Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Committed revision 1694126.
Comment 9 Mark Martinec 2015-08-04 23:48:41 UTC 
trunk (detail):
  Bug 7231: simplify overlycomplicated debug-logging
    in Plugin::URIDNSBL::complete_a_lookup
  Sending lib/Mail/SpamAssassin/Plugin/URIDNSBL.pm
Committed revision 1694129.
Comment 10 Kevin A. McGrail 2015-08-05 09:35:27 UTC 
Considering resolved.  Bravo Mark.  I know what a pain dealing with the Net::DNS changes can be.
Comment 11 Bill Cole 2017-01-13 17:44:01 UTC 
*** Bug 7339 has been marked as a duplicate of this bug. ***
Comment 12 Bill Cole 2017-01-13 18:26:45 UTC 
*** Bug 7338 has been marked as a duplicate of this bug. ***
Comment 13 Sidney Markowitz 2017-04-16 07:37:09 UTC 
Ported some more of what had been committed to trunk as part of syncing up 3.4 branch with trunk in preparation for release of 3.4.2

Committed revision 1791573