Debugging fixes for features recently added.

3 files changed, 68 insertions, 43 deletions

diff --git a/common/smtppass.c b/common/smtppass.c
index 57bdf27..2604a85 100644
--- a/common/smtppass.c
+++ b/common/smtppass.c
@@ -176,7 +176,7 @@ static int connect_out(spctx_t* ctx);
 static int read_server_response(spctx_t* ctx);
 static int parse_config_file(const char* configfile);
 static char* parse_address(char* line);
-static int is_successful_rsp(const char* line);
+static const char* get_successful_rsp(const char* line);
 
 /* Used externally in some cases */
 int sp_parse_option(const char* name, const char* option);
@@ -649,8 +649,8 @@ static void* thread_main(void* arg)
         fd = thread->fd;
     sp_unlock();
 
-    ctx = init_thread(fd);
-    if(!ctx)
+    /* Sometimes we get to this point and then quit is noted */
+    if(sp_is_quit() || (ctx = init_thread(fd)) == NULL)
     {
         /* Special case. We don't have a context so clean up descriptor */
         close(fd);
@@ -776,6 +776,7 @@ static int connect_out(spctx_t* ctx)
 static int smtp_passthru(spctx_t* ctx)
 {
     char* t;
+    const char* p;
     int r, ret = 0;
     unsigned int mask;
     int neterror = 0;
@@ -789,7 +790,7 @@ static int smtp_passthru(spctx_t* ctx)
     #define C_LINE  ctx->client.line
     #define S_LINE  ctx->server.line
 
-    for(;;)
+    while(!sp_is_quit())
     {
         mask = spio_select(ctx, &(ctx->client), &(ctx->server), NULL);
 
@@ -962,7 +963,7 @@ static int smtp_passthru(spctx_t* ctx)
                 }
             }
 
-            if(is_successful_rsp(S_LINE))
+            if((p = get_successful_rsp(S_LINE)) != NULL)
             {
                 /*
                  * Filter out any EHLO responses that we can't or don't want
@@ -970,14 +971,13 @@ static int smtp_passthru(spctx_t* ctx)
                  */
                 if(is_first_word(C_LINE, EHLO_CMD, KL(EHLO_CMD)))
                 {
-                    char* p = S_LINE + r;
                     if(is_first_word(p, ESMTP_PIPELINE, KL(ESMTP_PIPELINE)) ||
                        is_first_word(p, ESMTP_TLS, KL(ESMTP_TLS)) ||
                        is_first_word(p, ESMTP_CHUNK, KL(ESMTP_CHUNK)) ||
                        is_first_word(p, ESMTP_BINARY, KL(ESMTP_BINARY)) ||
                        is_first_word(p, ESMTP_CHECK, KL(ESMTP_CHECK)))
                     {
-                        sp_messagex(ctx, LOG_DEBUG, "filtered ESMTP feature: %s", trim_space(p));
+                        sp_messagex(ctx, LOG_DEBUG, "filtered ESMTP feature: %s", trim_space((char*)p));
                         continue;
                     }
                 }
@@ -1008,6 +1008,9 @@ static int smtp_passthru(spctx_t* ctx)
                         /* Recipients are separated by lines */
                         if(r != 0)
                             strcat(ctx->recipients, "\n");
+                        else
+                            ctx->recipients[0] = 0;
+
                         strcat(ctx->recipients, t);
                     }
                 }
@@ -1060,7 +1063,7 @@ static char* parse_address(char* line)
     return trim_end(line);
 }
 
-static int is_successful_rsp(const char* line)
+static const char* get_successful_rsp(const char* line)
 {
     /*
      * We check for both '250 xxx' type replies
@@ -1071,9 +1074,9 @@ static int is_successful_rsp(const char* line)
 
     if(line[0] == '2' && isdigit(line[1]) && isdigit(line[2]) &&
        (line[3] == ' ' || line[3] == '-'))
-        return 1;
+        return line + 4;
 
-    return 0;
+    return NULL;
 }
 
 void sp_add_log(spctx_t* ctx, char* prefix, char* line)
diff --git a/common/spio.c b/common/spio.c
index 2d42eac..8cd315b 100644
--- a/common/spio.c
+++ b/common/spio.c
@@ -164,6 +164,7 @@ unsigned int spio_select(spctx_t* ctx, ...)
     fd_set mask;
     spio_t* io;
     int ret = 0;
+    int have = 0;
     int i = 0;
     va_list ap;
 
@@ -174,16 +175,20 @@ unsigned int spio_select(spctx_t* ctx, ...)
 
     while((io = va_arg(ap, spio_t*)) != NULL)
     {
-        /* We can't handle more than 31 args */
-        if(i > (sizeof(int) * 8) - 2)
-            break;
+        if(spio_valid(io))
+        {
+            /* We can't handle more than 31 args */
+            if(i > (sizeof(int) * 8) - 2)
+                break;
 
-        /* Check if the buffer has something in it */
-        if(HAS_EXTRA(io))
-            ret |= (1 << i);
+            /* Check if the buffer has something in it */
+            if(HAS_EXTRA(io))
+                ret |= (1 << i);
 
-        /* Mark for select */
-        FD_SET(io->fd, &mask);
+            /* Mark for select */
+            FD_SET(io->fd, &mask);
+            have = 1;
+        }
 
         i++;
     }
@@ -194,17 +199,35 @@ unsigned int spio_select(spctx_t* ctx, ...)
     if(ret != 0)
         return ret;
 
-    /* Otherwise wait on more data */
-    switch(select(FD_SETSIZE, &mask, NULL, NULL,
-           (struct timeval*)&(g_state.timeout)))
-    {
-    case 0:
-        sp_messagex(ctx, LOG_ERR, "network operation timed out");
-        return ~0;
-    case -1:
-        sp_message(ctx, LOG_ERR, "couldn't select on sockets");
+    /* No valid file descriptors */
+    if(!have)
         return ~0;
-    };
+
+    for(;;)
+    {
+        /* Otherwise wait on more data */
+        switch(select(FD_SETSIZE, &mask, NULL, NULL,
+               (struct timeval*)&(g_state.timeout)))
+        {
+        case 0:
+            sp_messagex(ctx, LOG_ERR, "network operation timed out");
+            return ~0;
+
+        case -1:
+            if(errno == EINTR)
+            {
+                if(!sp_is_quit())
+                    continue;
+            }
+
+            else
+                sp_message(ctx, LOG_ERR, "couldn't select on sockets");
+
+            return ~0;
+        };
+
+        break;
+    }
 
     /* See what came in */
     i = 0;
diff --git a/src/clamsmtpd.c b/src/clamsmtpd.c
index f1d6e75..34a53a3 100644
--- a/src/clamsmtpd.c
+++ b/src/clamsmtpd.c
@@ -37,6 +37,7 @@
 
 #include <sys/types.h>
 #include <sys/param.h>
+#include <sys/wait.h>
 
 #include <paths.h>
 #include <ctype.h>
@@ -327,14 +328,16 @@ int cb_check_data(spctx_t* sp)
     const char* virus;
     clctx_t* ctx = (clctx_t*)sp;
 
-    /* Connect to clamav */
-    if(!spio_valid(&(ctx->clam)))
-        r = connect_clam(ctx);
-
-    if(r != -1 && (r = sp_cache_data(sp)) > 0)
+    /* ClamAV doesn't like empty files */
+    if((r = sp_cache_data(sp)) > 0)
+    {
+        /* Connect to clamav */
+        if(!spio_valid(&(ctx->clam)))
+            r = connect_clam(ctx);
 
-        /* ClamAV doesn't like empty files */
-        r = clam_scan_file(ctx, &virus);
+        if(r != -1)
+            r = clam_scan_file(ctx, &virus);
+    }
 
     switch(r)
     {
@@ -659,6 +662,9 @@ static int virus_action(clctx_t* ctx, const char* virus)
 
     if(g_clstate.virusaction != NULL)
     {
+        /* Cleanup any old actions */
+        while(waitpid(-1, &i, WNOHANG) > 0)
+            ;
 
         sp_messagex(sp, LOG_DEBUG, "executing virus action: %s", g_clstate.virusaction);
 
@@ -670,13 +676,6 @@ static int virus_action(clctx_t* ctx, const char* virus)
 
         /* The child */
         case 0:
-            /*
-             * New process group because we don't care about this child,
-             * it's return value, waiting for it to exit or any of that.
-             * Apparently this can't fail when done like this.
-             */
-            setsid();
-
             /* Close all descriptors */
             for(i = 0; i <= 2; i++)
                 close(i);
@@ -696,7 +695,7 @@ static int virus_action(clctx_t* ctx, const char* virus)
 
             /* If that returned then there was an error, but there's
              * not much we can do about it. */
-            exit(1);
+            _exit(1);
             break;
         };
     }