You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by ms...@apache.org on 2004/09/14 20:39:56 UTC

svn commit: rev 46033 - spamassassin/trunk/lib/Mail/SpamAssassin

Author: mss
Date: Tue Sep 14 11:39:55 2004
New Revision: 46033

Modified:
   spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm
Log:
bug 3649: In case of an error the truncated Pyzor output missed valuable information from stderr.
 * Now all output from Pyzor is captured (but only the first line used for parsing).
 * In case of a Traceback, an "internal error" is reported.
 * Replaced two unnecessary ~= compares with eqs.


Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm	(original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Dns.pm	Tue Sep 14 11:39:55 2004
@@ -893,7 +893,7 @@
 
 sub pyzor_lookup {
   my ($self, $fulltext) = @_;
-  my $response = undef;
+  my @response;
   my $pyzor_count;
   my $pyzor_whitelisted;
   my $timeout=$self->{conf}->{pyzor_timeout};
@@ -932,16 +932,18 @@
                 $tmpf, 1, $path, split(' ', $opts), "check");
     $pid or die "$!\n";
 
-    $response = <PYZOR>;
+    @response = <PYZOR>;
     close PYZOR;
 
-    unless (defined($response)) {
+    unless (@response) {
       die ("no response\n");	# yes, this is possible
     }
+    map { chomp } @response;
+    dbg("Pyzor: got response: " . join("\\n", @response));
 
-    chomp $response;
-
-    dbg("Pyzor: got response: $response");
+    if ($response[0] =~ /^Traceback/) {
+      die ("internal error\n");
+    }
 
     alarm(0);
     $self->cleanup_kids($pid);
@@ -951,11 +953,12 @@
   $self->leave_helper_run_mode();
 
   if ($@) {
-    if ($@ =~ /^__alarm__$/) {
+    chomp $@;
+    if ($@ eq "__alarm__") {
       dbg ("Pyzor -> check timed out after $timeout secs.");
-    } elsif ($@ =~ /^__brokenpipe__$/) {
+    } elsif ($@ eq "__brokenpipe__") {
       dbg ("Pyzor -> check failed: Broken pipe.");
-    } elsif ($@ eq "no response\n") {
+    } elsif ($@ eq "no response") {
       dbg ("Pyzor -> check failed: no response");
     } else {
       warn ("Pyzor -> check failed: $@\n");
@@ -964,7 +967,7 @@
   }
 
   # made regexp a little more forgiving (jm)
-  if ($response =~ /^\S+\t.*?\t(\d+)\t(\d+)\s*$/) {
+  if ($response[0] =~ /^\S+\t.*?\t(\d+)\t(\d+)\s*$/) {
     $pyzor_whitelisted = $2+0;
     if ($pyzor_whitelisted == 0) {
       $pyzor_count = $1+0;
@@ -972,7 +975,7 @@
 
   } else {
     # warn on failures to parse (jm)
-    dbg ("Pyzor: couldn't grok response \"$response\"");
+    dbg ("Pyzor: couldn't grok response \"$response[0]\"");
   }
 
   # moved this around a bit; no point in testing RE twice (jm)