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 2005/06/28 14:17:59 UTC

svn commit: r202163 - in /spamassassin/trunk/spamc: libspamc.c spamc.c spamc.pod

Author: mss
Date: Tue Jun 28 05:17:58 2005
New Revision: 202163

URL: http://svn.apache.org/viewcvs?rev=202163&view=rev
Log:
* bug 4434: Added support for multiple hosts via spamc -d (comma separated
  list).  This is only documented for spamc and not in the libspamc API, we
  might want to change the implementation at a later point.  Most of the
  stuff stayed logically the same, more or less just added a loop over
  the hostname/hostlist.
* Fixed a possible segfault when transport_setup failed (m->outbuf wasn't
  initialized).
* Added a bunch of asserts to make NULL arguments fail a little bit saner.

TODO:
* Make MAX_CONNECT_RETRIES and CONNECT_RETRY_SLEEP configurable.
* Clean up the message initilization and documentation mess:  Why do we have
  both a m->buf and a m->outbuf when all we do with m->outbuf is allocating
  memory and then assign the pointer to m->out?  Shall m->out be freed or
  does it always point to one of the other members?  How must the members
  be initialized?  Maybe we should add a public message_setup routine which
  assigns the correct initial values?
* Add IPv6 support... doh, Google Summer Of Code submission time is over ;~)

Modified:
    spamassassin/trunk/spamc/libspamc.c
    spamassassin/trunk/spamc/spamc.c
    spamassassin/trunk/spamc/spamc.pod

Modified: spamassassin/trunk/spamc/libspamc.c
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/spamc/libspamc.c?rev=202163&r1=202162&r2=202163&view=diff
==============================================================================
--- spamassassin/trunk/spamc/libspamc.c (original)
+++ spamassassin/trunk/spamc/libspamc.c Tue Jun 28 05:17:58 2005
@@ -57,6 +57,7 @@
 #include <sys/time.h>
 #endif
 
+/* FIXME: Make this configurable */
 #define MAX_CONNECT_RETRIES 3
 #define CONNECT_RETRY_SLEEP 1
 
@@ -559,6 +560,8 @@
 
 int message_read(int fd, int flags, struct message *m)
 {
+    assert(m != NULL);
+
     libspamc_timeout = 0;
 
     /* create the "private" part of the struct message */
@@ -590,6 +593,8 @@
     off_t jlimit;
     char buffer[1024];
 
+    assert(m != NULL);
+
     if (m->priv->flags & SPAMC_CHECK_ONLY) {
 	if (m->is_spam == EX_ISSPAM || m->is_spam == EX_NOTSPAM) {
 	    return full_write(fd, 1, m->out, m->out_len);
@@ -856,6 +861,9 @@
     SSL *ssl = NULL;
     SSL_METHOD *meth;
 
+    assert(tp != NULL);
+    assert(m != NULL);
+
     if (flags & SPAMC_USE_SSL) {
 #ifdef SPAMC_SSL
 	SSLeay_add_ssl_algorithms();
@@ -1079,6 +1087,8 @@
     int ret;
     struct message m;
 
+    assert(trans != NULL);
+
     m.type = MESSAGE_NONE;
 
     m.max_len = max_size;
@@ -1127,6 +1137,9 @@
     SSL *ssl = NULL;
     SSL_METHOD *meth;
 
+    assert(tp != NULL);
+    assert(m != NULL);
+
     if (flags & SPAMC_USE_SSL) {
 #ifdef SPAMC_SSL
 	SSLeay_add_ssl_algorithms();
@@ -1327,7 +1340,8 @@
 
 void message_cleanup(struct message *m)
 {
-    if (m->outbuf)
+    assert(m != NULL);
+    if (m->outbuf != NULL)
         free(m->outbuf);
     if (m->raw != NULL)
         free(m->raw);
@@ -1418,7 +1432,9 @@
 */
 int transport_setup(struct transport *tp, int flags)
 {
-    struct hostent *hp = 0;
+    struct hostent *hp;
+    char *hostlist, *hostname;
+    int errbits;
     char **addrp;
 
 #ifdef _WIN32
@@ -1432,10 +1448,9 @@
 
 #endif
 
+    assert(tp != NULL);
     tp->flags = flags;
 
-    assert(tp != 0);
-
     switch (tp->type) {
 #ifndef _WIN32
     case TRANSPORT_UNIX:
@@ -1448,82 +1463,124 @@
         return EX_OK;
 
     case TRANSPORT_TCP:
-        if (NULL == (hp = gethostbyname(tp->hostname))) {
-            int origherr = h_errno;	/* take a copy before syslog() */
-
-            libspamc_log(flags, LOG_ERR, "gethostbyname(%s) failed: h_errno=%d",
-                    tp->hostname, origherr);
-            switch (origherr) {
-            case HOST_NOT_FOUND:
-            case NO_ADDRESS:
-            case NO_RECOVERY:
-                return EX_NOHOST;
-            case TRY_AGAIN:
-                return EX_TEMPFAIL;
-            default:
-                return EX_OSERR;
-            }
-        }
-
-                /*--------------------------------------------------------
-                * If we have no hosts at all, or if they are some other
-                * kind of address family besides IPv4, then we really
-                * just have no hosts at all.
-                */
-        if (hp->h_addr_list[0] == 0) {
-            /* no hosts in this list */
-            return EX_NOHOST;
-        }
+        if ((hostlist = strdup(tp->hostname)) == NULL)
+            return EX_OSERR;
 
-        if (hp->h_length != sizeof tp->hosts[0]
-            || hp->h_addrtype != AF_INET) {
-            /* FAIL - bad size/protocol/family? */
-            return EX_NOHOST;
-        }
-
-                /*--------------------------------------------------------
-                * Copy all the IP addresses into our private structure.
-                * This gets them out of the resolver's static area and
-                * means we won't ever walk all over the list with other
-                * calls.
-                */
+        /* We want to return the least permanent error, in this bitmask we
+         * record the errors seen with:
+         *  0: no error
+         *  1: EX_TEMPFAIL
+         *  2: EX_NOHOST
+         * EX_OSERR will return immediately.
+         * Bits aren't reset so a check against nhosts is needed to determine
+         * if something went wrong.
+         */
+        errbits = 0;
         tp->nhosts = 0;
-
-        for (addrp = hp->h_addr_list; *addrp; addrp++) {
-            if (tp->nhosts >= TRANSPORT_MAX_HOSTS - 1) {
-                libspamc_log(flags, LOG_ERR, "hit limit of %d hosts, ignoring remainder",
-                      TRANSPORT_MAX_HOSTS - 1);
-                break;
+        /* Start with char offset in front of the string because we'll add 
+         * one in the loop
+         */
+        hostname = hostlist - 1;
+        do {
+            char *hostend;
+            
+            hostname += 1;
+            hostend = strchr(hostname, ',');
+            if (hostend != NULL) {
+                *hostend = '\0';
+            }
+            
+            if ((hp = gethostbyname(hostname)) == NULL) {
+                int origerr = h_errno; /* take a copy before syslog() */
+                libspamc_log(flags, LOG_DEBUG, "gethostbyname(%s) failed: h_errno=%d",
+                    hostname, origerr);
+                switch (origerr) {
+                case TRY_AGAIN:
+                    errbits |= 1;
+                    break;
+                case HOST_NOT_FOUND:
+                case NO_ADDRESS:
+                case NO_RECOVERY:
+                    errbits |= 2;
+                    break;
+                default:
+                    /* should not happen, all errors are checked above */
+                    free(hostlist);
+                    return EX_OSERR;
+                }
+                goto nexthost; /* try next host in list */
+            }
+            
+            /* If we have no hosts at all, or if they are some other
+             * kind of address family besides IPv4, then we really
+             * just have no hosts at all. TODO: IPv6
+             */
+            if (hp->h_addr_list[0] == NULL
+             || hp->h_length != sizeof tp->hosts[0]
+             || hp->h_addrtype != AF_INET) {
+                /* no hosts/bad size/wrong family */
+                errbits |= 1;
+                goto nexthost; /* try next host in list */
             }
 
-            memcpy(&tp->hosts[tp->nhosts], *addrp, sizeof tp->hosts[0]);
-
-            tp->nhosts++;
+            /* Copy all the IP addresses into our private structure.
+             * This gets them out of the resolver's static area and
+             * means we won't ever walk all over the list with other
+             * calls.
+             */
+            for (addrp = hp->h_addr_list; *addrp; addrp++) {
+                if (tp->nhosts == TRANSPORT_MAX_HOSTS) {
+                    libspamc_log(flags, LOG_NOTICE, "hit limit of %d hosts, ignoring remainder",
+                        TRANSPORT_MAX_HOSTS);
+                    break;
+                }
+                memcpy(&tp->hosts[tp->nhosts], *addrp, hp->h_length);
+                tp->nhosts++;
+            }
+            
+nexthost:
+            hostname = hostend;
+        } while (hostname != NULL);
+        free(hostlist);
+        
+        if (tp->nhosts == 0) {
+            if (errbits & 1) {
+                libspamc_log(flags, LOG_ERR, "could not resolve any hosts (%s): a temporary error occurred",
+                    tp->hostname); 
+                return EX_TEMPFAIL;
+            }
+            else {
+                libspamc_log(flags, LOG_ERR, "could not resolve any hosts (%s): no such host",
+                    tp->hostname); 
+                return EX_NOHOST;
+            }
         }
-
-                /*--------------------------------------------------------
-                * QUASI-LOAD-BALANCING
-                *
-                * If the user wants to do quasi load balancing, "rotate"
-                * the list by a random amount based on the current time.
-                * This may later be truncated to a single item. This is
-                * meaningful only if we have more than one host.
-                */
+        
+        /* QUASI-LOAD-BALANCING
+         *
+         * If the user wants to do quasi load balancing, "rotate"
+         * the list by a random amount based on the current time.
+         * This may later be truncated to a single item. This is
+         * meaningful only if we have more than one host.
+         */
         if ((flags & SPAMC_RANDOMIZE_HOSTS) && tp->nhosts > 1) {
             _randomize_hosts(tp);
         }
 
-                /*--------------------------------------------------------
-                * If the user wants no fallback, simply truncate the host
-                * list to just one - this pretends that this is the extent
-                * of our connection list - then it's not a special case.
-                */
+        /* If the user wants no fallback, simply truncate the host
+         * list to just one - this pretends that this is the extent
+         * of our connection list - then it's not a special case.
+         */
         if (!(flags & SPAMC_SAFE_FALLBACK) && tp->nhosts > 1) {
             /* truncating list */
             tp->nhosts = 1;
         }
+        
+        return EX_OK;
     }
-    return EX_OK;
+    
+    /* oops, unknown transport type */
+    return EX_OSERR;
 }
 
 /* --------------------------------------------------------------------------- */

Modified: spamassassin/trunk/spamc/spamc.c
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/spamc/spamc.c?rev=202163&r1=202162&r2=202163&view=diff
==============================================================================
--- spamassassin/trunk/spamc/spamc.c (original)
+++ spamassassin/trunk/spamc/spamc.c Tue Jun 28 05:17:58 2005
@@ -134,7 +134,7 @@
     usg("\n");
     usg("Options:\n");
 
-    usg("  -d host             Specify host to connect to.\n"
+    usg("  -d host[,host2]     Specify one or more hosts to connect to.\n"
         "                      [default: localhost]\n");
     usg("  -H                  Randomize IP addresses for the looked-up\n"
         "                      hostname.\n");
@@ -683,6 +683,7 @@
      */
     m.type = MESSAGE_NONE;
     m.out = NULL;
+    m.outbuf = NULL;
     m.raw = NULL;
     m.priv = NULL;
     m.max_len = max_size;

Modified: spamassassin/trunk/spamc/spamc.pod
URL: http://svn.apache.org/viewcvs/spamassassin/trunk/spamc/spamc.pod?rev=202163&r1=202162&r2=202163&view=diff
==============================================================================
--- spamassassin/trunk/spamc/spamc.pod (original)
+++ spamassassin/trunk/spamc/spamc.pod Tue Jun 28 05:17:58 2005
@@ -62,12 +62,14 @@
 Combining B<-c> and B<-E> is a no-op, since B<-c> implies the behaviour
 of B<-E>.
 
-=item B<-d> I<host>
+=item B<-d> I<host[,host2]>
 
-In TCP/IP mode, connect to spamd server on given host (default: localhost). 
+In TCP/IP mode, connect to spamd server on given host (default: localhost).
+Several hosts can be specified if separated by commas.
 
 If I<host> resolves to multiple addresses, then spamc will fail-over to the 
-other addresses, if the first one cannot be connected to.
+other addresses, if the first one cannot be connected to.  It will first try
+all addresses of one host before it tries the next one in the list.
 
 =item B<-e> I<command> I<[args]>
 
@@ -96,9 +98,9 @@
 
 =item B<-H>
 
-For TCP/IP sockets, randomize the IP addresses returned from a DNS name
-lookup (when more than one IP is returned). This provides for a kind of
-hostname-base load balancing.
+For TCP/IP sockets, randomize the IP addresses returned for the hosts given
+by the B<-d> switch. This provides for a simple kind of load balancing.  It
+will try only three times though.
 
 =item B<-l>