Mailing List Archive

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>