Add retry option to clamd and spamd. Bug 392

4 files changed, 195 insertions, 117 deletions

diff --git a/src/src/ip.c b/src/src/ip.c
index e4a43e69a..f6072c2e8 100644
--- a/src/src/ip.c
+++ b/src/src/ip.c
@@ -337,7 +337,8 @@ for (h = &shost; h != NULL; h = h->next)
       {
       if (fd != fd6) close(fd6);
       if (fd != fd4) close(fd4);
-      if (connhost) {
+      if (connhost)
+	{
 	h->port = port;
 	*connhost = *h;
 	connhost->next = NULL;
@@ -720,3 +721,5 @@ for (i=0; i < dscp_table_size; ++i)
 
 
 /* End of ip.c */
+/* vi: aw ai sw=2
+*/
diff --git a/src/src/malware.c b/src/src/malware.c
index 2e49751c1..c13e70616 100644
--- a/src/src/malware.c
+++ b/src/src/malware.c
@@ -38,14 +38,12 @@ static struct scan
 /* The maximum number of clamd servers that are supported in the configuration */
 #define MAX_CLAMD_SERVERS 32
 #define MAX_CLAMD_SERVERS_S "32"
-/* Maximum length of the hostname that can be specified in the clamd address list */
-#define MAX_CLAMD_ADDRESS_LENGTH 64
-#define MAX_CLAMD_ADDRESS_LENGTH_S "64"
 
-typedef struct clamd_address_container {
-  uschar tcp_addr[MAX_CLAMD_ADDRESS_LENGTH+1];
-  unsigned int tcp_port;
-} clamd_address_container;
+typedef struct clamd_address {
+  uschar * hostspec;
+  unsigned tcp_port;
+  unsigned retry;
+} clamd_address;
 
 #ifndef nelements
 # define nelements(arr) (sizeof(arr) / sizeof(arr[0]))
@@ -115,21 +113,21 @@ extern uschar spooled_message_id[17];
 static inline int
 malware_errlog_defer(const uschar * str)
 {
-  log_write(0, LOG_MAIN|LOG_PANIC, "malware acl condition: %s", str);
-  return DEFER;
+log_write(0, LOG_MAIN|LOG_PANIC, "malware acl condition: %s", str);
+return DEFER;
 }
 
 static int
 m_errlog_defer(struct scan * scanent, const uschar * str)
 {
-  return malware_errlog_defer(string_sprintf("%s: %s", scanent->name, str));
+return malware_errlog_defer(string_sprintf("%s: %s", scanent->name, str));
 }
 static int
 m_errlog_defer_3(struct scan * scanent, const uschar * str,
 	int fd_to_close)
 {
-  (void) close(fd_to_close);
-  return m_errlog_defer(scanent, str);
+(void) close(fd_to_close);
+return m_errlog_defer(scanent, str);
 }
 
 /*************************************************/
@@ -141,60 +139,61 @@ static inline int
 m_tcpsocket(const uschar * hostname, unsigned int port,
 	host_item * host, uschar ** errstr)
 {
-  return ip_connectedsocket(SOCK_STREAM, hostname, port, port, 5, host, errstr);
+return ip_connectedsocket(SOCK_STREAM, hostname, port, port, 5, host, errstr);
 }
 
 static int
 m_sock_send(int sock, uschar * buf, int cnt, uschar ** errstr)
 {
-  if (send(sock, buf, cnt, 0) < 0) {
-    int err = errno;
-    (void)close(sock);
-    *errstr = string_sprintf("unable to send to socket (%s): %s",
-	   buf, strerror(err));
-    return -1;
-    }
-  return sock;
+if (send(sock, buf, cnt, 0) < 0)
+  {
+  int err = errno;
+  (void)close(sock);
+  *errstr = string_sprintf("unable to send to socket (%s): %s",
+	 buf, strerror(err));
+  return -1;
+  }
+return sock;
 }
 
 static const pcre *
 m_pcre_compile(const uschar * re, uschar ** errstr)
 {
-  const uschar * rerror;
-  int roffset;
-  const pcre * cre;
-
-  cre = pcre_compile(CS re, PCRE_COPT, (const char **)&rerror, &roffset, NULL);
-  if (!cre)
-    *errstr= string_sprintf("regular expression error in '%s': %s at offset %d",
-	re, rerror, roffset);
-  return cre;
+const uschar * rerror;
+int roffset;
+const pcre * cre;
+
+cre = pcre_compile(CS re, PCRE_COPT, (const char **)&rerror, &roffset, NULL);
+if (!cre)
+  *errstr= string_sprintf("regular expression error in '%s': %s at offset %d",
+      re, rerror, roffset);
+return cre;
 }
 
 uschar *
 m_pcre_exec(const pcre * cre, uschar * text)
 {
-  int ovector[10*3];
-  int i = pcre_exec(cre, NULL, CS text, Ustrlen(text), 0, 0,
-		ovector, nelements(ovector));
-  uschar * substr = NULL;
-  if (i >= 2)				/* Got it */
-    pcre_get_substring(CS text, ovector, i, 1, (const char **) &substr);
-  return substr;
+int ovector[10*3];
+int i = pcre_exec(cre, NULL, CS text, Ustrlen(text), 0, 0,
+	      ovector, nelements(ovector));
+uschar * substr = NULL;
+if (i >= 2)				/* Got it */
+  pcre_get_substring(CS text, ovector, i, 1, (const char **) &substr);
+return substr;
 }
 
 static const pcre *
 m_pcre_nextinlist(const uschar ** list, int * sep,
  char * listerr, uschar ** errstr)
 {
-  const uschar * list_ele;
-  const pcre * cre = NULL;
+const uschar * list_ele;
+const pcre * cre = NULL;
 
-  if (!(list_ele = string_nextinlist(list, sep, NULL, 0)))
-    *errstr = US listerr;
-  else
-    cre = m_pcre_compile(CUS list_ele, errstr);
-  return cre;
+if (!(list_ele = string_nextinlist(list, sep, NULL, 0)))
+  *errstr = US listerr;
+else
+  cre = m_pcre_compile(CUS list_ele, errstr);
+return cre;
 }
 
 /*
@@ -375,6 +374,27 @@ if (mksd_read_lines (sock, av_buffer, sizeof (av_buffer), tmo) < 0)
 return mksd_parse_line (scanent, CS av_buffer);
 }
 
+
+static int
+clamd_option(clamd_address * cd, const uschar * optstr, int * subsep)
+{
+uschar * s;
+
+cd->retry = 0;
+while ((s = string_nextinlist(&optstr, subsep, NULL, 0)))
+  {
+  if (Ustrncmp(s, "retry=", 6) == 0)
+    {
+    int sec = readconf_readtime((s += 6), '\0', FALSE);
+    if (sec < 0)
+      return FAIL;
+    cd->retry = sec;
+    }
+  else
+    return FAIL;
+  }
+}
+
 /*************************************************
 *          Scan content for malware              *
 *************************************************/
@@ -1205,7 +1225,7 @@ if (!malware_ok)
       off_t fsize;
       unsigned int fsize_uint;
       BOOL use_scan_command = FALSE;
-      clamd_address_container * clamd_address_vector[MAX_CLAMD_SERVERS];
+      clamd_address * cv[MAX_CLAMD_SERVERS];
       int num_servers = 0;
 #ifdef WITH_OLD_CLAMAV_STREAM
       unsigned int port;
@@ -1215,46 +1235,80 @@ if (!malware_ok)
       uint32_t send_size, send_final_zeroblock;
 #endif
 
+      /*XXX if unixdomain socket, only one server supported. Needs fixing;
+      there's no reason we should not mix local and remote servers */
+
       if (*scanner_options == '/')
+	{
+	clamd_address * cd;
+	const uschar * sublist;
+	int subsep = ' ';
+
 	/* Local file; so we def want to use_scan_command and don't want to try
 	 * passing IP/port combinations */
 	use_scan_command = TRUE;
+	cd = (clamd_address *) store_get(sizeof(clamd_address));
+
+	/* extract socket-path part */
+	sublist = scanner_options;
+	cd->hostspec = string_nextinlist(&sublist, &subsep, NULL, 0);
+
+	/* parse options */
+	if (clamd_option(cd, sublist, &subsep) != OK)
+	  return m_errlog_defer(scanent,
+	    string_sprintf("bad option '%s'", scanner_options));
+	cv[0] = cd;
+	}
       else
 	{
-	const uschar *address = scanner_options;
-	uschar address_buffer[MAX_CLAMD_ADDRESS_LENGTH + 20];
-
 	/* Go through the rest of the list of host/port and construct an array
 	 * of servers to try. The first one is the bit we just passed from
 	 * scanner_options so process that first and then scan the remainder of
 	 * the address buffer */
 	do
 	  {
-	  clamd_address_container *this_clamd;
+	  clamd_address * cd;
+	  const uschar * sublist;
+	  int subsep = ' ';
+	  uschar * s;
 
 	  /* The 'local' option means use the SCAN command over the network
 	   * socket (ie common file storage in use) */
-	  if (strcmpic(address,US"local") == 0)
+	  /*XXX we could accept this also as a local option? */
+	  if (strcmpic(scanner_options, US"local") == 0)
 	    {
 	    use_scan_command = TRUE;
 	    continue;
 	    }
 
-	  /* XXX: If unsuccessful we should free this memory */
-	  this_clamd =
-	      (clamd_address_container *)store_get(sizeof(clamd_address_container));
+	  cd = (clamd_address *) store_get(sizeof(clamd_address));
 
 	  /* extract host and port part */
-	  if( sscanf(CS address, "%" MAX_CLAMD_ADDRESS_LENGTH_S "s %u",
-		 this_clamd->tcp_addr, &(this_clamd->tcp_port)) != 2 )
+	  sublist = scanner_options;
+	  if (!(cd->hostspec = string_nextinlist(&sublist, &subsep, NULL, 0)))
 	    {
 	    (void) m_errlog_defer(scanent,
-			string_sprintf("invalid address '%s'", address));
+		      string_sprintf("missing address: '%s'", scanner_options));
 	    continue;
 	    }
+	  if (!(s = string_nextinlist(&sublist, &subsep, NULL, 0)))
+	    {
+	    (void) m_errlog_defer(scanent,
+		      string_sprintf("missing port: '%s'", scanner_options));
+	    continue;
+	    }
+	  cd->tcp_port = atoi(s);
 
-	  clamd_address_vector[num_servers] = this_clamd;
-	  num_servers++;
+	  /* parse options */
+	  /*XXX should these options be common over scanner types? */
+	  if (clamd_option(cd, sublist, &subsep) != OK)
+	    {
+	    return m_errlog_defer(scanent,
+	      string_sprintf("bad option '%s'", scanner_options));
+	    continue;
+	    }
+
+	  cv[num_servers++] = cd;
 	  if (num_servers >= MAX_CLAMD_SERVERS)
 	    {
 	    (void) m_errlog_defer(scanent,
@@ -1262,9 +1316,8 @@ if (!malware_ok)
 		  "specified; only using the first " MAX_CLAMD_SERVERS_S );
 	    break;
 	    }
-	  } while ((address = string_nextinlist(&av_scanner_work, &sep,
-					address_buffer,
-					sizeof(address_buffer))) != NULL);
+	  } while ((scanner_options = string_nextinlist(&av_scanner_work, &sep,
+					NULL, 0)));
 
 	/* check if we have at least one server */
 	if (!num_servers)
@@ -1288,43 +1341,53 @@ if (!malware_ok)
 
 	while (num_servers > 0)
 	  {
-	  int i;
-	  int current_server = random_number( num_servers );
+	  int i = random_number( num_servers );
+	  clamd_address * cd = cv[i];
 
-	  DEBUG(D_acl)
-	    debug_printf("trying server name %s, port %u\n",
-		       clamd_address_vector[current_server]->tcp_addr,
-		       clamd_address_vector[current_server]->tcp_port);
+	  DEBUG(D_acl) debug_printf("trying server name %s, port %u\n",
+			 cd->hostspec, cd->tcp_port);
 
 	  /* Lookup the host. This is to ensure that we connect to the same IP
 	   * on both connections (as one host could resolve to multiple ips) */
-	  sock= m_tcpsocket(clamd_address_vector[current_server]->tcp_addr,
-			      clamd_address_vector[current_server]->tcp_port,
-			      &connhost, &errstr);
-	  if (sock >= 0)
+	  for (;;)
 	    {
-	    /* Connection successfully established with a server */
-	    hostname = clamd_address_vector[current_server]->tcp_addr;
-	    break;
+	    sock= m_tcpsocket(cd->hostspec, cd->tcp_port, &connhost, &errstr);
+	    if (sock >= 0)
+	      {
+	      /* Connection successfully established with a server */
+	      hostname = cd->hostspec;
+	      break;
+	      }
+	    if (cd->retry <= 0) break;
+	    while (cd->retry > 0) cd->retry = sleep(cd->retry);
 	    }
+	  if (sock >= 0)
+	    break;
 
 	  log_write(0, LOG_MAIN, "malware acl condition: %s: %s",
 	    scanent->name, errstr);
 
 	  /* Remove the server from the list. XXX We should free the memory */
 	  num_servers--;
-	  for (i = current_server; i < num_servers; i++)
-	    clamd_address_vector[i] = clamd_address_vector[i+1];
+	  for (; i < num_servers; i++)
+	    cv[i] = cv[i+1];
 	  }
 
 	if (num_servers == 0)
 	  return m_errlog_defer(scanent, US"all servers failed");
 	}
       else
-	{
-	if ((sock = ip_unixsocket(scanner_options, &errstr)) < 0)
-	  return m_errlog_defer(scanent, errstr);
-	}
+	for (;;)
+	  {
+	  if ((sock = ip_unixsocket(cv[0]->hostspec, &errstr)) >= 0)
+	    {
+	    hostname = cv[0]->hostspec;
+	    break;
+	    }
+	  if (cv[0]->retry <= 0)
+	    return m_errlog_defer(scanent, errstr);
+	  while (cv[0]->retry > 0) cv[0]->retry = sleep(cv[0]->retry);
+	  }
 
       /* have socket in variable "sock"; command to use is semi-independent of
        * the socket protocol.  We use SCAN if is local (either Unix/local
diff --git a/src/src/spam.c b/src/src/spam.c
index 99e524d3c..96780f59e 100644
--- a/src/src/spam.c
+++ b/src/src/spam.c
@@ -21,7 +21,6 @@ int spam_ok = 0;
 int spam_rc = 0;
 uschar *prev_spamd_address_work = NULL;
 
-static int timeout_sec;
 static const uschar * loglabel = US"spam acl condition:";
 
 
@@ -29,9 +28,11 @@ static int
 spamd_param_init(spamd_address_container *spamd)
 {
 /* default spamd server weight, time and backup value */
-spamd->weight = SPAMD_WEIGHT;
 spamd->is_failed = FALSE;
 spamd->is_backup = FALSE;
+spamd->weight = SPAMD_WEIGHT;
+spamd->timeout = SPAMD_TIMEOUT;
+spamd->retry = 0;
 return 0;
 }
 
@@ -41,6 +42,7 @@ spamd_param(const uschar *param, spamd_address_container *spamd)
 {
 static int timesinceday = -1;
 const uschar * s;
+const uschar * name;
 
 /* check backup parameter */
 if (Ustrcmp(param, "backup") == 0)
@@ -67,6 +69,7 @@ if (Ustrncmp(param, "time=", 5) == 0)
   unsigned int time_start, time_end;
   const uschar * end_string;
 
+  name = US"time";
   s = param+5;
   if ((end_string = Ustrchr(s, '-')))
     {
@@ -74,18 +77,10 @@ if (Ustrncmp(param, "time=", 5) == 0)
     if (  sscanf(CS end_string, "%u.%u.%u", &end_h,   &end_m,   &end_s)   == 0
        || sscanf(CS s,          "%u.%u.%u", &start_h, &start_m, &start_s) == 0
        )
-      {
-      log_write(0, LOG_MAIN,
-	"%s warning - invalid spamd time value: '%s'", loglabel, s);
-      return -1; /* syntax error */
-      }
+      goto badval;
     }
   else
-    {
-    log_write(0, LOG_MAIN,
-      "%s warning - invalid spamd time value: '%s'", loglabel, s);
-    return -1; /* syntax error */
-    }
+    goto badval;
 
   if (timesinceday < 0)
     {
@@ -112,19 +107,31 @@ if (Ustrcmp(param, "variant=rspamd") == 0)
 if (Ustrncmp(param, "tmo=", 4) == 0)
   {
   int sec = readconf_readtime((s = param+4), '\0', FALSE);
+  name = US"timeout";
   if (sec < 0)
-    {
-    log_write(0, LOG_MAIN,
-      "%s warning - invalid spamd timeout value: '%s'", loglabel, s);
-    return -1; /* syntax error */
-    }
-  timeout_sec = sec;
+    goto badval;
+  spamd->timeout = sec;
+  return 0;
+  }
+
+if (Ustrncmp(param, "retry=", 6) == 0)
+  {
+  int sec = readconf_readtime((s = param+6), '\0', FALSE);
+  name = US"retry";
+  if (sec < 0)
+    goto badval;
+  spamd->retry = sec;
   return 0;
   }
 
 log_write(0, LOG_MAIN, "%s warning - invalid spamd parameter: '%s'",
   loglabel, param);
 return -1; /* syntax error */
+
+badval:
+  log_write(0, LOG_MAIN,
+    "%s warning - invalid spamd %s value: '%s'", loglabel, name, s);
+  return -1; /* syntax error */
 }
 
 
@@ -188,7 +195,6 @@ FILE *mbox_file;
 int spamd_sock = -1;
 uschar spamd_buffer[32600];
 int i, j, offset, result;
-BOOL is_rspamd;
 uschar spamd_version[8];
 uschar spamd_short_result[8];
 uschar spamd_score_char;
@@ -206,6 +212,7 @@ struct timeval select_tv;         /* and applied by PH */
 fd_set select_fd;
 #endif
 uschar *spamd_address_work;
+spamd_address_container * sd;
 
 /* stop compiler warning */
 result = 0;
@@ -224,8 +231,6 @@ if ( (Ustrcmp(user_name,"0") == 0) ||
      (strcmpic(user_name,US"false") == 0) )
   return FAIL;
 
-timeout_sec = SPAMD_TIMEOUT;
-
 /* if there is an additional option, check if it is "true" */
 if (strcmpic(list,US"true") == 0)
   /* in that case, always return true later */
@@ -278,7 +283,6 @@ start = time(NULL);
   uschar *address;
   const uschar *spamd_address_list_ptr = spamd_address_work;
   spamd_address_container * spamd_address_vector[32];
-  spamd_address_container * sd;
 
   /* Check how many spamd servers we have
      and register their addresses */
@@ -298,7 +302,7 @@ start = time(NULL);
 	 args++
 	 )
       {
-	HDEBUG(D_acl) debug_printf("spamd: addr parm '%s'\n", s);
+	HDEBUG(D_acl) debug_printf("spamd:  addr parm '%s'\n", s);
 	switch (args)
 	{
 	case 0:   sd->hostspec = s;
@@ -331,17 +335,24 @@ start = time(NULL);
     goto defer;
     }
 
-  while (1)
+  current_server = spamd_get_server(spamd_address_vector, num_servers);
+  sd = spamd_address_vector[current_server];
+  for(;;)
     {
     uschar * errstr;
 
-    current_server = spamd_get_server(spamd_address_vector, num_servers);
-    sd = spamd_address_vector[current_server];
-
     debug_printf("trying server %s\n", sd->hostspec);
 
-    /* contact a spamd */
-    if ((spamd_sock = ip_streamsocket(sd->hostspec, &errstr, 5)) >= 0)
+    for (;;)
+      {
+      if (  (spamd_sock = ip_streamsocket(sd->hostspec, &errstr, 5)) >= 0
+         || sd->retry <= 0
+	 )
+	break;
+      debug_printf("server %s: retry conn\n", sd->hostspec);
+      while (sd->retry > 0) sd->retry = sleep(sd->retry);
+      }
+    if (spamd_sock >= 0)
       break;
 
     log_write(0, LOG_MAIN, "%s spamd: %s", loglabel, errstr);
@@ -350,12 +361,11 @@ start = time(NULL);
     current_server = spamd_get_server(spamd_address_vector, num_servers);
     if (current_server < 0)
       {
-      log_write(0, LOG_MAIN|LOG_PANIC, "%s all spamd servers failed",
-	loglabel);
+      log_write(0, LOG_MAIN|LOG_PANIC, "%s all spamd servers failed", loglabel);
       goto defer;
       }
+    sd = spamd_address_vector[current_server];
     }
-    is_rspamd = sd->is_rspamd;
   }
 
 if (spamd_sock == -1)
@@ -367,7 +377,7 @@ if (spamd_sock == -1)
 
 (void)fcntl(spamd_sock, F_SETFL, O_NONBLOCK);
 /* now we are connected to spamd on spamd_sock */
-if (is_rspamd)
+if (sd->is_rspamd)
   {				/* rspamd variant */
   uschar *req_str;
   const char *helo;
@@ -449,7 +459,7 @@ again:
 	  "%s %s on spamd socket", loglabel, strerror(errno));
       else
 	{
-	if (time(NULL) - start < timeout_sec)
+	if (time(NULL) - start < sd->timeout)
 	  goto again;
 	log_write(0, LOG_MAIN|LOG_PANIC,
 	  "%s timed out writing spamd socket", loglabel);
@@ -494,7 +504,7 @@ offset = 0;
 while ((i = ip_recv(spamd_sock,
 		   spamd_buffer + offset,
 		   sizeof(spamd_buffer) - offset - 1,
-		   timeout_sec - time(NULL) + start)) > 0 )
+		   sd->timeout - time(NULL) + start)) > 0 )
   offset += i;
 
 /* error handling */
@@ -509,7 +519,7 @@ if (i <= 0 && errno != 0)
 /* reading done */
 (void)close(spamd_sock);
 
-if (is_rspamd)
+if (sd->is_rspamd)
   {				/* rspamd variant of reply */
   int r;
   if ((r = sscanf(CS spamd_buffer,
diff --git a/src/src/spam.h b/src/src/spam.h
index 7ab6d2a58..0e3acfc25 100644
--- a/src/src/spam.h
+++ b/src/src/spam.h
@@ -30,6 +30,8 @@ typedef struct spamd_address_container
   int is_failed:1;
   int is_backup:1;
   unsigned int weight;
+  unsigned int timeout;
+  unsigned int retry;
 } spamd_address_container;
 
 #endif