You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spamassassin.apache.org by mm...@apache.org on 2009/11/27 13:49:44 UTC

svn commit: r884863 - in /spamassassin/trunk: build/announcements/3.3.0-beta1.txt lib/Mail/SpamAssassin/Client.pm spamc/libspamc.c spamd/PROTOCOL spamd/spamd.raw

Author: mmartinec
Date: Fri Nov 27 12:49:43 2009
New Revision: 884863

URL: http://svn.apache.org/viewvc?rev=884863&view=rev
Log:
Bug 6187 - Mail::SpamAssasin::Client ping may erronesously
result in broken pipe. Bumps spamc protocol vesion to 1.5.

Modified:
    spamassassin/trunk/build/announcements/3.3.0-beta1.txt
    spamassassin/trunk/lib/Mail/SpamAssassin/Client.pm
    spamassassin/trunk/spamc/libspamc.c
    spamassassin/trunk/spamd/PROTOCOL
    spamassassin/trunk/spamd/spamd.raw

Modified: spamassassin/trunk/build/announcements/3.3.0-beta1.txt
URL: http://svn.apache.org/viewvc/spamassassin/trunk/build/announcements/3.3.0-beta1.txt?rev=884863&r1=884862&r2=884863&view=diff
==============================================================================
--- spamassassin/trunk/build/announcements/3.3.0-beta1.txt (original)
+++ spamassassin/trunk/build/announcements/3.3.0-beta1.txt Fri Nov 27 12:49:43 2009
@@ -57,6 +57,11 @@
   See sql/README.awl for details. The change need not be undone even if
   downgrading back to 3.2.* for some reason;
 
+- fixing a protocol implementation error regarding a PING command required
+  bumping up the SPAMC protocol version to 1.5.  Spamd retains compatibility
+  with older spamc clients. Combining new spamc clients with pre-3.3 versions
+  of a spamd daemon is not supported.
+
 - support for versions of perl 5.6.* is being gradually revoked
   (may still work, but no promises and no support)
 

Modified: spamassassin/trunk/lib/Mail/SpamAssassin/Client.pm
URL: http://svn.apache.org/viewvc/spamassassin/trunk/lib/Mail/SpamAssassin/Client.pm?rev=884863&r1=884862&r2=884863&view=diff
==============================================================================
--- spamassassin/trunk/lib/Mail/SpamAssassin/Client.pm (original)
+++ spamassassin/trunk/lib/Mail/SpamAssassin/Client.pm Fri Nov 27 12:49:43 2009
@@ -62,7 +62,7 @@
 
 my $EOL = "\015\012";
 my $BLANK = $EOL x 2;
-my $PROTOVERSION = 'SPAMC/1.3';
+my $PROTOVERSION = 'SPAMC/1.5';
 
 =head1 PUBLIC METHODS
 
@@ -408,7 +408,7 @@
   return 0 unless ($remote);
 
   print $remote "PING $PROTOVERSION$EOL";
-  print $remote "$EOL";
+  print $remote "$EOL";  # bug 6187, bumps protocol version to 1.5
 
   $! = 0; my $line = <$remote>;
   defined $line || $!==0  or

Modified: spamassassin/trunk/spamc/libspamc.c
URL: http://svn.apache.org/viewvc/spamassassin/trunk/spamc/libspamc.c?rev=884863&r1=884862&r2=884863&view=diff
==============================================================================
--- spamassassin/trunk/spamc/libspamc.c (original)
+++ spamassassin/trunk/spamc/libspamc.c Fri Nov 27 12:49:43 2009
@@ -140,7 +140,7 @@
  */
 
 /* Set the protocol version that this spamc speaks */
-static const char *PROTOCOL_VERSION = "SPAMC/1.4";
+static const char *PROTOCOL_VERSION = "SPAMC/1.5";
 
 /* "private" part of struct message.
  * we use this instead of the struct message directly, so that we
@@ -1341,8 +1341,10 @@
               }
               return EX_DATAERR;
           }
-          len += snprintf(buf + len, 8192-len, "Content-length: %d\r\n\r\n", (int) towrite_len);
+          len += snprintf(buf + len, 8192-len, "Content-length: %d\r\n", (int) towrite_len);
         }
+        /* bug 6187, PING needs empty line too, bumps protocol version to 1.5 */
+        len += snprintf(buf + len, 8192-len, "\r\n");
     
         libspamc_timeout = m->timeout;
         libspamc_connect_timeout = m->connect_timeout;	/* Sep 8, 2008 mrgus: separate connect timeout */

Modified: spamassassin/trunk/spamd/PROTOCOL
URL: http://svn.apache.org/viewvc/spamassassin/trunk/spamd/PROTOCOL?rev=884863&r1=884862&r2=884863&view=diff
==============================================================================
--- spamassassin/trunk/spamd/PROTOCOL (original)
+++ spamassassin/trunk/spamd/PROTOCOL Fri Nov 27 12:49:43 2009
@@ -25,7 +25,7 @@
 After each side is done writing, it shuts down its side of the connection.
 
 The first line from spamc is the command for spamd to execute (PROCESS a
-message is the command in protocol<=1.3) followed by the protocol version.
+message is the command in protocol<=1.5) followed by the protocol version.
 
 There may be additional headers following the command, which are as yet
 undefined.  Servers should ignore these, and keep looking for headers which
@@ -45,7 +45,7 @@
 Commands
 --------
 
-The following commands are defined as of protocol 1.4:
+The following commands are defined as of protocol 1.5:
 
 CHECK         --  Just check if the passed message is spam or not and reply as
                   described below
@@ -122,16 +122,22 @@
 REPORT_IFSPAM returns the same as REPORT if the message is spam, or nothing at
 all if the message is non-spam.
 
+
 The PING command does not actually trigger any spam checking, and (as with
-SKIP) no additional input is expected. It returns a simple confirmation
+SKIP) no additional headers are expected. It returns a simple confirmation
 response, like this:
 
-	SPAMD/1.4 0 PONG\r\n
+	SPAMD/1.5 0 PONG\r\n
 
 This facility may be useful for monitoring programs which wish to check that
 the daemon is alive and providing at least a basic response within a reasonable
 time frame.
 
+Note that since the protocol version 1.5, a client sending a PING command
+is required to follow the command (and a null header) with an empty line,
+for consistency with other commands (fixes bug 6187).
+
+
 TELL accepts three new headers, Message-class, Set and Remove and will return
 two possible headers, DidSet and DidRemove which indicate which action was
 taken.  It is up to the caller to determine if the proper action happened.  Here

Modified: spamassassin/trunk/spamd/spamd.raw
URL: http://svn.apache.org/viewvc/spamassassin/trunk/spamd/spamd.raw?rev=884863&r1=884862&r2=884863&view=diff
==============================================================================
--- spamassassin/trunk/spamd/spamd.raw (original)
+++ spamassassin/trunk/spamd/spamd.raw Fri Nov 27 12:49:43 2009
@@ -1251,20 +1251,12 @@
 
   s/\r?\n//;
 
-  # It may be a SKIP message, meaning that the client (spamc)
-  # thinks it is too big to check.  So we don't do any real work
-  # in that case.
-
-  if (/SKIP SPAMC\/(.*)/) {
-    info(sprintf("spamd: skipped large message in %3d seconds", time - $start));
-  }
-
   # It might be a CHECK message, meaning that we should just check
   # if it's spam or not, then return the appropriate response.
   # If we get the PROCESS command, the client is going to send a
   # message that we need to filter.
 
-  elsif (/(PROCESS|CHECK|SYMBOLS|REPORT|HEADERS|REPORT_IFSPAM) SPAMC\/(.*)/) {
+  if (/(PROCESS|CHECK|SYMBOLS|REPORT|HEADERS|REPORT_IFSPAM) SPAMC\/(.*)/) {
     my $method = $1;
     my $version = $2;
     eval {
@@ -1310,10 +1302,20 @@
     }
   }
 
-  # Looks like a client is just seeing if we're alive.
+  # Looks like a client is just seeing if we're alive or changed its mind
+
+  elsif (/(SKIP|PING) SPAMC\/(.*)/) {
+    my $method = $1;
+    my $version = $2;
 
-  elsif (/PING SPAMC\/(.*)/) {
-    syswrite( $client, "SPAMD/1.4 $resphash{EX_OK} PONG\r\n" );
+    if ($method eq 'SKIP') {
+      # It may be a SKIP message, meaning that the client (spamc)
+      # thinks it is too big to check.  So we don't do any real work
+      # in that case.
+      info("spamd: skipped large message in %3.1f seconds", time - $start);
+    }
+    doskip_or_ping($method, $version,
+                   $start, $remote_hostname, $remote_hostaddr);
   }
 
   # If it was none of the above, then we don't know what it was.
@@ -1790,6 +1792,25 @@
   return 1;
 }
 
+sub doskip_or_ping {
+  my ($method, $version, $start_time, $remote_hostname, $remote_hostaddr) = @_;
+
+  if ( $version >= 1.5 ) {
+    # Spamc protocol 1.5 means client is expected to send a protocol header
+    # (usually just a null header), followed by an empty line
+    # Fixes Bug 6187.
+
+    my $hdrs = {};
+    return 0 unless (parse_headers($hdrs, $client));
+  }
+
+  if ($method eq 'PING') {
+    print $client "SPAMD/1.5 $resphash{EX_OK} PONG\r\n";
+  }
+
+  return 1;
+}
+
 ###########################################################################
 
 sub do_user_handling {