Thanks! Now fixed I suppose:

trunk:
  $ svn ci -m 'Bug 6573: Report for IPv6 IP hitting DNS BL/WL is broken'
  Sending lib/Mail/SpamAssassin/Dns.pm
Committed revision 1099506.

(In reply to comment #1)
> Thanks! Now fixed I suppose:
> 
> trunk:
>   $ svn ci -m 'Bug 6573: Report for IPv6 IP hitting DNS BL/WL is broken'
>   Sending lib/Mail/SpamAssassin/Dns.pm
> Committed revision 1099506.

We are talking about this patch, yes?  If so, I don't use IPv6 but I really liked the empty if comment change.

I don't use IPv6 but I was looking for a type eq 'AA' so if this works, I see the logic as good but it confuses me a bit.

Index: Dns.pm
===================================================================
--- Dns.pm      (revision 1040623)
+++ Dns.pm      (working copy)
@@ -182,15 +182,26 @@
   my ($self, $rule, $question, $answer) = @_;
 
   my $log = "";
-  if (substr($rule, 0, 2) ne "__") {
-    if ($answer->type eq 'TXT') {
-      $log = $answer->rdatastr;
-      $log =~ s/^"(.*)"$/$1/;
-      $log =~ s/(?<![<([])(https?:\/\/\S+)/<$1>/gi;
+  if (substr($rule, 0, 2) eq "__") {
+    # don't bother with meta rules
+  } elsif ($answer->type eq 'TXT') {
+    local $1;
+    $log = $answer->rdatastr;
+    $log =~ s/^"(.*)"$/$1/;
+    $log =~ s/(?<![<([])(https?:\/\/\S+)/<$1>/gi;
+  } else {  # assuming $answer->type eq 'A'
+    local($1,$2,$3,$4,$5);
+    if ($question->string =~ m/^((?:[0-9a-fA-F]\.){32})(\S+\w)/) {
+      $log = ' listed in ' . lc($2);
+      my $ipv6addr = join('', reverse split(/\./, lc $1));
+      $ipv6addr =~ s/\G(....)/$1:/g;  chop $ipv6addr;
+      $ipv6addr =~ s/:0{1,3}/:/g;
+      $log = $ipv6addr . $log;
+    } elsif ($question->string =~ m/^(\d+)\.(\d+)\.(\d+)\.(\d+)\.(\S+\w)/) {
+      $log = "$4.$3.$2.$1 listed in " . lc($5);
+    } else {
+      $log = 'listed in ' . $question->string;
     }
-    elsif ($question->string =~ m/^(\d+)\.(\d+)\.(\d+)\.(\d+)\.(\S+\w)/) {
-      $log = "$4.$3.$2.$1 listed in ".lc($5);
-    }
   }
 
   # TODO: this may result in some log messages appearing under the
@@ -281,9 +292,9 @@
   while (my ($subtest, $rule) = each %{ $self->{dnspost}->{$set} }) {
     next if $self->{tests_already_hit}->{$rule};
 
-    # exact substr (usually IP address)
-    if ($subtest eq $rdatastr) {
-      $self->dnsbl_hit($rule, $question, $answer);
+    if ($subtest =~ /^\d+\.\d+\.\d+\.\d+$/) {
+      # test for exact equality, not a regexp (usually IP address)
+      $self->dnsbl_hit($rule, $question, $answer)  if $subtest eq $rdatastr;
     }
     # senderbase
     elsif ($subtest =~ s/^sb://) {

> We are talking about this patch, yes?  If so, I don't use IPv6 but I really
> liked the empty if comment change. 
> I don't use IPv6 but I was looking for a type eq 'AA' so if this works, I see
> the logic as good but it confuses me a bit.

Sorry, I don't think I understand what you are trying to say.

> --- Dns.pm      (revision 1040623)
> +++ Dns.pm      (working copy)
> @@ -182,15 +182,26 @@
[...]

Yes, this patch chunk is what we are taking about, a change
in sub dnsbl_hit. As shown by: svn diff -c1099506

> @@ -281,9 +292,9 @@
[...]

Not this one. This chunk is from Bug 6565, unrelated.

(In reply to comment #3)
> > We are talking about this patch, yes?  If so, I don't use IPv6 but I really
> > liked the empty if comment change. 
> > I don't use IPv6 but I was looking for a type eq 'AA' so if this works, I see
> > the logic as good but it confuses me a bit.
> 
> Sorry, I don't think I understand what you are trying to say.

That's because I MEANT to write 'AAAA'.  If we get an IPv6 answer, won't it have a type of AAAA and not A?

So maybe an elseif $answer->type eq 'AAAA' is needed rather than assuming A?

> Yes, this patch chunk is what we are taking about, a change
> in sub dnsbl_hit. As shown by: svn diff -c1099506

Thanks.  I was using svn diff Dns.pm -r 1040623

> If we get an IPv6 answer, won't it
> have a type of AAAA and not A?
> So maybe an elseif $answer->type eq 'AAAA' is needed rather than assuming A?

Not in case of querying DNS WBL servers. The query is for an A record
(expecting an answer like 127.0.0.x), even if a domain in a question
section is composed similar to a PTR query for an IPv6 address.

We are currently never issuing a DNS query of an AAAA type.
I just left the assumption as it was, not strengthening it
(for fear of potentially breaking some unanticipated case).

OK, so this code really does nothing more than formatting changes.  I'm +1 though I can't test.  Even if it's wrong, it's only cleaning up textual output and the logic looks sound.

Works for me:

 pts rule name              description
---- ---------------------- --------------------------------------------------
-0.1 RCVD_IN_IPREPDNS_100   RBL: Sender listed at
                            http://www.chaosreigns.com/iprep/, 100% ham
                            [2001:470:1f0f:201:0:0:0:2 listed in]
                            [iprep.chaosreigns.com]

ok looked fine for me.. +1 3.4

Closing. Already in trunk.