fixing 1296 and other bugs

6 files changed, 124 insertions, 46 deletions

diff --git a/ChangeLog b/ChangeLog
index 79d2c275..2089a4b1 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+Wed Dec  5 21:39:35 MST 2007
+         Fixed race in multi-threaded server mode.
+         Fixed handling of POST data when receiving a
+         "Connection: close" header (#1296).
+         Releasing libmicrohttpd 0.1.2. - CG
+
 Sat Nov 17 00:55:24 MST 2007
          Fixed off-by-one in error message string matching.
          Added code to avoid generating SIGPIPE on platforms
diff --git a/configure.ac b/configure.ac
index f7110260..adc1e3ca 100644
--- a/configure.ac
+++ b/configure.ac
@@ -21,8 +21,8 @@
 #
 #
 AC_PREREQ(2.57)
-AC_INIT([libmicrohttpd], [0.1.1],[libmicrohttpd@gnunet.org])
-AM_INIT_AUTOMAKE([libmicrohttpd], [0.1.1])
+AC_INIT([libmicrohttpd], [0.1.2],[libmicrohttpd@gnunet.org])
+AM_INIT_AUTOMAKE([libmicrohttpd], [0.1.2])
 AM_CONFIG_HEADER([config.h])
 
 AH_TOP([#define _GNU_SOURCE  1])
diff --git a/src/daemon/Makefile.am b/src/daemon/Makefile.am
index b35b45d7..3ac9f312 100644
--- a/src/daemon/Makefile.am
+++ b/src/daemon/Makefile.am
@@ -3,7 +3,7 @@ SUBDIRS  = .
 INCLUDES = -I$(top_srcdir)/src/include
 
 if HAVE_GNU_LD
- retaincommand=-Wl,--retain-symbols-file -Wl,SYMBOLS
+ retaincommand=-Wl,--retain-symbols-file -Wl,$(srcdir)/SYMBOLS
 endif
 
 EXTRA_DIST = SYMBOLS
@@ -12,7 +12,7 @@ lib_LTLIBRARIES = \
   libmicrohttpd.la
 
 libmicrohttpd_la_LDFLAGS = \
-  -export-dynamic -version-info 2:0:0 $(retaincommand)
+  -export-dynamic -version-info 2:1:1 $(retaincommand)
 libmicrohttpd_la_SOURCES = \
   connection.c connection.h \
   reason_phrase.c reason_phrase.h \
@@ -46,11 +46,13 @@ check_PROGRAMS = \
   daemontest_get \
   daemontest_post \
   daemontest_postform \
+  daemontest_post_loop \
   daemontest_put \
   daemontest_large_put \
   daemontest_get11 \
   daemontest_post11 \
   daemontest_postform11 \
+  daemontest_post_loop11 \
   daemontest_put11 \
   daemontest_large_put11 \
   daemontest_long_header
@@ -80,6 +82,12 @@ daemontest_postform_LDADD = \
   $(top_builddir)/src/daemon/libmicrohttpd.la \
   @LIBCURL@ 
 
+daemontest_post_loop_SOURCES = \
+  daemontest_post_loop.c
+daemontest_post_loop_LDADD = \
+  $(top_builddir)/src/daemon/libmicrohttpd.la \
+  @LIBCURL@ 
+
 daemontest_put_SOURCES = \
   daemontest_put.c
 daemontest_put_LDADD = \
@@ -104,6 +112,12 @@ daemontest_postform11_LDADD = \
   $(top_builddir)/src/daemon/libmicrohttpd.la \
   @LIBCURL@ 
 
+daemontest_post_loop11_SOURCES = \
+  daemontest_post_loop.c
+daemontest_post_loop11_LDADD = \
+  $(top_builddir)/src/daemon/libmicrohttpd.la \
+  @LIBCURL@ 
+
 daemontest_put11_SOURCES = \
   daemontest_put.c
 daemontest_put11_LDADD = \
diff --git a/src/daemon/connection.c b/src/daemon/connection.c
index 8217932e..39e00dd7 100644
--- a/src/daemon/connection.c
+++ b/src/daemon/connection.c
@@ -66,13 +66,12 @@
  * Add extra debug messages with reasons for closing connections
  * (non-error reasons).
  */
-#define DEBUG_CLOSE 0
-
+#define DEBUG_CLOSE MHD_NO
 
 /**
  * Should all data send be printed to stderr?
  */
-#define DEBUG_SEND_DATA 0
+#define DEBUG_SEND_DATA MHD_NO
 
 
 /**
@@ -604,11 +603,11 @@ MHD_parse_connection_headers (struct MHD_Connection *connection)
   char *colon;
   char *tmp;
   const char *clen;
-  const char *end;
   unsigned long long cval;
   struct MHD_Response *response;
 
-  if (connection->bodyReceived == MHD_YES)
+  if ( (connection->bodyReceived == MHD_YES) ||
+       (connection->headersReceived == MHD_YES) )
     abort ();
   colon = NULL;                 /* make gcc happy */
   last = NULL;
@@ -691,17 +690,6 @@ MHD_parse_connection_headers (struct MHD_Connection *connection)
                   connection->bodyReceived = MHD_NO;
                 }
             }
-          end = MHD_lookup_connection_value (connection,
-                                             MHD_HEADER_KIND,
-                                             MHD_HTTP_HEADER_CONNECTION);
-          if ((end != NULL) && (0 == strcasecmp (end, "close")))
-            {
-              /* other side explicitly requested
-                 that we close the connection after
-                 this request */
-              connection->read_close = MHD_YES;
-            }
-
           if ((0 != (MHD_USE_PEDANTIC_CHECKS & connection->daemon->options))
               && (NULL != connection->version)
               && (0 == strcasecmp (MHD_HTTP_VERSION_1_1, connection->version))
@@ -724,7 +712,6 @@ MHD_parse_connection_headers (struct MHD_Connection *connection)
               MHD_queue_response (connection, MHD_HTTP_BAD_REQUEST, response);
               MHD_destroy_response (response);
             }
-
           break;
         }
       /* line should be normal header line, find colon */
@@ -808,7 +795,7 @@ MHD_call_connection_handler (struct MHD_Connection *connection)
                         connection->read_buffer, &processed,
                         &connection->client_context))
     {
-      /* serios internal error, close connection */
+      /* serious internal error, close connection */
 #if HAVE_MESSAGES
       MHD_DLOG (connection->daemon,
                 "Internal application error, closing connection.\n");
@@ -820,8 +807,8 @@ MHD_call_connection_handler (struct MHD_Connection *connection)
   memmove (connection->read_buffer,
            &connection->read_buffer[connection->readLoc - processed],
            processed);
-  if (connection->uploadSize != -1)
-    connection->uploadSize -= (connection->readLoc - processed);
+  if (connection->uploadSize != -1) 
+    connection->uploadSize -= (connection->readLoc - processed);  
   connection->readLoc = processed;
   if ((connection->uploadSize == 0) ||
       ((connection->readLoc == 0) &&
@@ -921,6 +908,12 @@ MHD_connection_handle_read (struct MHD_Connection *connection)
 #endif
 #endif
       shutdown (connection->socket_fd, SHUT_RD);
+      if ( (connection->headersReceived == MHD_NO) ||
+           (connection->bodyReceived == MHD_NO) ) {
+        /* no request => no response! */
+        CLOSE (connection->socket_fd);
+        connection->socket_fd = -1;
+      }
       return MHD_YES;
     }
   connection->readLoc += bytes_read;
@@ -1059,6 +1052,7 @@ MHD_connection_handle_write (struct MHD_Connection *connection)
 {
   struct MHD_Response *response;
   int ret;
+  const char * end;
 
   if (MHD_need_100_continue (connection))
     {
@@ -1194,6 +1188,9 @@ MHD_connection_handle_write (struct MHD_Connection *connection)
                                               connection,
                                               &connection->client_context,
                                               MHD_REQUEST_TERMINATED_COMPLETED_OK);
+      end = MHD_lookup_connection_value (connection,
+                                         MHD_HEADER_KIND,
+                                         MHD_HTTP_HEADER_CONNECTION);
       connection->client_context = NULL;
       connection->continuePos = 0;
       connection->responseCode = 0;
@@ -1204,7 +1201,14 @@ MHD_connection_handle_write (struct MHD_Connection *connection)
       connection->bodyReceived = MHD_NO;
       connection->messagePos = 0;
       connection->method = NULL;
-      connection->url = NULL;
+      connection->url = NULL;      
+      if ((end != NULL) && (0 == strcasecmp (end, "close")))
+        {
+          /* other side explicitly requested
+             that we close the connection after
+             this request */
+          connection->read_close = MHD_YES;
+        }
       if ((connection->read_close == MHD_YES) ||
           (0 != strcasecmp (MHD_HTTP_VERSION_1_1, connection->version)))
         {
diff --git a/src/daemon/daemon.c b/src/daemon/daemon.c
index e5317c04..b7c66c8d 100644
--- a/src/daemon/daemon.c
+++ b/src/daemon/daemon.c
@@ -44,7 +44,13 @@
  * Print extra messages with reasons for closing
  * sockets? (only adds non-error messages). 
  */
-#define DEBUG_CLOSE 0
+#define DEBUG_CLOSE MHD_NO
+
+/**
+ * Print extra messages when establishing
+ * connections? (only adds non-error messages). 
+ */
+#define DEBUG_CONNECT MHD_NO
 
 /**
  * Register an access handler for all URIs beginning with uri_prefix.
@@ -163,6 +169,9 @@ MHD_get_fdset (struct MHD_Daemon *daemon,
         return MHD_NO;
       pos = pos->next;
     }
+#if DEBUG_CONNECT
+  MHD_DLOG (daemon, "Maximum socket in select set: %d\n", *max_fd);
+#endif  
   return MHD_YES;
 }
 
@@ -276,6 +285,9 @@ MHD_accept_connection (struct MHD_Daemon *daemon)
         }
       return MHD_NO;
     }
+#if DEBUG_CONNECT
+  MHD_DLOG (daemon, "Accepted connection on socket %d\n", s);
+#endif  
   if (daemon->max_connections == 0)
     {
       /* above connection limit - reject */
@@ -425,7 +437,8 @@ MHD_cleanup_connections (struct MHD_Daemon *daemon)
           continue;
         }
 
-      if ((pos->headersReceived == MHD_YES) && (pos->response == NULL))
+      if ( (0 == (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+           ((pos->headersReceived == MHD_YES) && (pos->response == NULL)) )
         MHD_call_connection_handler (pos);
 
       prev = pos;
diff --git a/src/daemon/daemontest_post_loop.c b/src/daemon/daemontest_post_loop.c
index c24dcf52..8fbc1f70 100644
--- a/src/daemon/daemontest_post_loop.c
+++ b/src/daemon/daemontest_post_loop.c
@@ -37,7 +37,7 @@
 
 #define POST_DATA "<?xml version='1.0' ?>\n<xml>\n<data-id>1</data-id>\n</xml>\n"
 
-#define LOOPCOUNT 100
+#define LOOPCOUNT 10
 
 static int oneone;
 
@@ -67,8 +67,9 @@ ahc_echo (void *cls,
           const char *method,
           const char *version,
           const char *upload_data, unsigned int *upload_data_size,
-          void **unused)
+          void **mptr)
 {
+  static int marker;
   struct MHD_Response *response;
   int ret;
 
@@ -77,15 +78,23 @@ ahc_echo (void *cls,
       printf ("METHOD: %s\n", method);
       return MHD_NO;            /* unexpected method */
     }
+  if ( (*mptr != NULL) &&
+       (0 == *upload_data_size) ) {
+    if (*mptr != &marker)
+      abort();
+    response = MHD_create_response_from_data (2,
+                                              "OK",
+                                              MHD_NO, MHD_NO);
+    ret = MHD_queue_response (connection, MHD_HTTP_OK, response);
+    MHD_destroy_response (response);
+    *mptr = NULL;
+    return ret;
+  }
   if (strlen(POST_DATA) != *upload_data_size)
-    return MHD_NO;
+    return MHD_YES;
   *upload_data_size = 0;
-  response = MHD_create_response_from_data (2,
-                                            "OK",
-                                            MHD_NO, MHD_NO);
-  ret = MHD_queue_response (connection, MHD_HTTP_OK, response);
-  MHD_destroy_response (response);
-  return ret;
+  *mptr = &marker;
+  return MHD_YES;
 }
 
 
@@ -98,18 +107,22 @@ testInternalPost ()
   struct CBC cbc;
   CURLcode errornum;
   int i;
+  char url[1024];
 
   cbc.buf = buf;
   cbc.size = 2048;
   d = MHD_start_daemon (MHD_USE_SELECT_INTERNALLY | MHD_USE_DEBUG,
-                        8008, NULL, NULL, &ahc_echo, NULL, MHD_OPTION_END);
+                        1080, NULL, NULL, &ahc_echo, NULL, MHD_OPTION_END);
   if (d == NULL)
     return 1;
   for (i=0;i<LOOPCOUNT;i++) {
+    if (0 == i % 100)
+      fprintf(stderr, ".");
     c = curl_easy_init ();
     cbc.pos = 0;
     buf[0] = '\0';
-    curl_easy_setopt (c, CURLOPT_URL, "http://localhost:1080/hello_world");
+    sprintf(url, "http://localhost:1080/hw%d", i);
+    curl_easy_setopt (c, CURLOPT_URL, url);
     curl_easy_setopt (c, CURLOPT_WRITEFUNCTION, &copyBuffer);
     curl_easy_setopt (c, CURLOPT_WRITEDATA, &cbc);
     curl_easy_setopt (c, CURLOPT_POSTFIELDS, POST_DATA);
@@ -143,6 +156,7 @@ testInternalPost ()
     }
   }
   MHD_stop_daemon (d);
+  fprintf(stderr, "\n");
   return 0;
 }
 
@@ -155,18 +169,22 @@ testMultithreadedPost ()
   struct CBC cbc;
   CURLcode errornum;
   int i;
+  char url[1024];
 
   cbc.buf = buf;
   cbc.size = 2048;
   d = MHD_start_daemon (MHD_USE_THREAD_PER_CONNECTION | MHD_USE_DEBUG,
-                        8009, NULL, NULL, &ahc_echo, NULL, MHD_OPTION_END);
+                        1081, NULL, NULL, &ahc_echo, NULL, MHD_OPTION_END);
   if (d == NULL)
     return 16;
   for (i=0;i<LOOPCOUNT;i++) {
+    if (0 == i % 100)
+      fprintf(stderr, ".");
     c = curl_easy_init ();
     cbc.pos = 0;
     buf[0] = '\0';
-    curl_easy_setopt (c, CURLOPT_URL, "http://localhost:1081/hello_world");
+    sprintf(url, "http://localhost:1081/hw%d", i);
+    curl_easy_setopt (c, CURLOPT_URL, url);
     curl_easy_setopt (c, CURLOPT_WRITEFUNCTION, &copyBuffer);
     curl_easy_setopt (c, CURLOPT_WRITEDATA, &cbc);
     curl_easy_setopt (c, CURLOPT_POSTFIELDS, POST_DATA);
@@ -200,6 +218,7 @@ testMultithreadedPost ()
     }
   }
   MHD_stop_daemon (d);
+  fprintf(stderr, "\n");
   return 0;
 }
 
@@ -222,6 +241,9 @@ testExternalPost ()
   time_t start;
   struct timeval tv;
   int i;
+  unsigned long long timeout;
+  long ctimeout;
+  char url[1024];
 
   multi = NULL;
   cbc.buf = buf;
@@ -238,10 +260,13 @@ testExternalPost ()
       return 512;
     }
   for (i=0;i<LOOPCOUNT;i++) {
+    if (0 == i % 100)
+      fprintf(stderr, ".");
     c = curl_easy_init ();
     cbc.pos = 0;
     buf[0] = '\0';
-    curl_easy_setopt (c, CURLOPT_URL, "http://localhost:1082/hello_world");
+    sprintf(url, "http://localhost:1082/hw%d", i);
+    curl_easy_setopt (c, CURLOPT_URL, url);
     curl_easy_setopt (c, CURLOPT_WRITEFUNCTION, &copyBuffer);
     curl_easy_setopt (c, CURLOPT_WRITEDATA, &cbc);
     curl_easy_setopt (c, CURLOPT_POSTFIELDS, POST_DATA);
@@ -291,8 +316,23 @@ testExternalPost ()
             MHD_stop_daemon (d);
             return 4096;
           }
-        tv.tv_sec = 0;
-        tv.tv_usec = 1000;
+        if (MHD_NO == MHD_get_timeout(d, &timeout))
+          timeout = 1000000; /* 1000s == INFTY */
+        if ( (CURLM_OK == curl_multi_timeout(multi, &ctimeout)) &&
+             (ctimeout < timeout) &&
+             (ctimeout >= 0) )
+          timeout = ctimeout;
+        if (timeout > 0)
+          timeout = 0; /* this line is needed since CURL seems to have
+                           a bug -- it returns a timeout of -1 even though
+                           it should be attempting to connect -- and the
+                           socket for doing the connection is not yet
+                           in the write-set!  If the timeout is too high,
+                           no progress would happen! Even a timeout of
+                           1 would cause my system to be mostly idle instead
+                           of processing at maximum speed... */
+        tv.tv_sec = timeout / 1000;
+        tv.tv_usec = (timeout % 1000) * 1000;
         select (max + 1, &rs, &ws, &es, &tv);
         curl_multi_perform (multi, &running);
         if (running == 0)
@@ -308,25 +348,26 @@ testExternalPost ()
                           __FILE__,
                           __LINE__, curl_easy_strerror (msg->data.result));
                 curl_multi_remove_handle (multi, c);
-                curl_multi_cleanup (multi);
                 curl_easy_cleanup (c);
                 c = NULL;
-                multi = NULL;
               }
           }
         MHD_run (d);
       }
+    if (c != NULL) {
       curl_multi_remove_handle (multi, c);
       curl_easy_cleanup (c);
+    }
     if ( (buf[0] != 'O') ||
          (buf[1] != 'K') ) {
       curl_multi_cleanup (multi);  
       MHD_stop_daemon (d);
       return 8192;
-    }
+    }  
   }
   curl_multi_cleanup (multi);  
   MHD_stop_daemon (d);
+  fprintf(stderr, "\n");
   return 0;
 }