[GNUnet-SVN] r37721 - in libmicrohttpd: . doc src/include src/microhttpd

[gnunet]

Author: grothoff
Date: 2016-08-15 13:13:50 +0200 (Mon, 15 Aug 2016)
New Revision: 37721

Modified:
   libmicrohttpd/ChangeLog
   libmicrohttpd/doc/libmicrohttpd.texi
   libmicrohttpd/src/include/microhttpd.h
   libmicrohttpd/src/microhttpd/connection.c
   libmicrohttpd/src/microhttpd/daemon.c
Log:
fixing crash bug, connection-limit bug and documenting connection-limit 
behavior better

Modified: libmicrohttpd/ChangeLog
===================================================================
--- libmicrohttpd/ChangeLog     2016-08-15 11:05:56 UTC (rev 37720)
+++ libmicrohttpd/ChangeLog     2016-08-15 11:13:50 UTC (rev 37721)
@@ -1,3 +1,14 @@
+Mon Aug 15 13:06:52 CEST 2016
+       Fixed possible crash due to write to read-only region of
+       memory given ill-formed HTTP request (write was otherwise
+       harmless, writing 0 to where there was already a 0).
+       Fixed issue with closed connection slots not immediately
+       being available again for new connections if we reached
+       our connection limit.
+       Avoid even accept()ing connections in certain thread modes
+       if we are at the connection limit and
+       MHD_USE_PIPE_FOR_SHUTDOWN is available.
+
 Wed Aug 10 16:42:57 MSK 2016
        Moved threads, locks and mutex abstraction to separate files,
        some minor errors fixed, added support for thread name functions

Modified: libmicrohttpd/doc/libmicrohttpd.texi
===================================================================
--- libmicrohttpd/doc/libmicrohttpd.texi        2016-08-15 11:05:56 UTC (rev 
37720)
+++ libmicrohttpd/doc/libmicrohttpd.texi        2016-08-15 11:13:50 UTC (rev 
37721)
@@ -626,6 +626,15 @@
 four for @code{stdin}, @code{stdout}, @code{stderr} and the server
 socket).  In other words, the default is as large as possible.
 
+If the connection limit is reached, MHD's behavior depends a bit on
+other options.  If @code{MHD_USE_PIPE_FOR_SHUTDOWN} was given, MHD
+will stop accepting connections on the listen socket.  This will cause
+the operating system to queue connections (up to the @code{listen()}
+limit) above the connection limit.  Those connections will be held
+until MHD is done processing at least one of the active connections.
+If @code{MHD_USE_PIPE_FOR_SHUTDOWN} is not set, then MHD will continue
+to @code{accept()} and immediately @code{close()} these connections.
+
 Note that if you set a low connection limit, you can easily get into
 trouble with browsers doing request pipelining.  For example, if your
 connection limit is ``1'', a browser may open a first connection to

Modified: libmicrohttpd/src/include/microhttpd.h
===================================================================
--- libmicrohttpd/src/include/microhttpd.h      2016-08-15 11:05:56 UTC (rev 
37720)
+++ libmicrohttpd/src/include/microhttpd.h      2016-08-15 11:13:50 UTC (rev 
37721)
@@ -131,7 +131,7 @@
  * Current version of the library.
  * 0x01093001 = 1.9.30-1.
  */
-#define MHD_VERSION 0x00095004
+#define MHD_VERSION 0x00095005
 
 /**
  * MHD-internal return code for "YES".

Modified: libmicrohttpd/src/microhttpd/connection.c
===================================================================
--- libmicrohttpd/src/microhttpd/connection.c   2016-08-15 11:05:56 UTC (rev 
37720)
+++ libmicrohttpd/src/microhttpd/connection.c   2016-08-15 11:13:50 UTC (rev 
37721)
@@ -483,6 +483,19 @@
                              &connection->client_context,
                              termination_code);
   connection->client_aware = MHD_NO;
+
+  /* if we were at the connection limit before and are in
+     thread-per-connection mode, signal the main thread
+     to resume accepting connections */
+  if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
+       (MHD_INVALID_PIPE_ != daemon->wpipe[1]) &&
+       (1 != MHD_pipe_write_ (daemon->wpipe[1], "c", 1)) )
+    {
+#ifdef HAVE_MESSAGES
+      MHD_DLOG (daemon,
+                "failed to signal end of connection via pipe");
+#endif
+    }
 }
 
 
@@ -1527,6 +1540,7 @@
                             size_t line_len)
 {
   struct MHD_Daemon *daemon = connection->daemon;
+  const char *curi;
   char *uri;
   char *http_version;
   char *args;
@@ -1539,16 +1553,19 @@
   uri++;
   /* Skip any spaces. Not required by standard but allow
      to be more tolerant. */
-  while (' ' == uri[0] && (size_t)(uri - line) < line_len)
+  while ( (' ' == uri[0]) &&
+          ( (size_t)(uri - line) < line_len) )
     uri++;
   if (uri - line == line_len)
     {
-      uri = "";
+      curi = "";
+      uri = NULL;
       connection->version = "";
       args = NULL;
     }
   else
     {
+      curi = uri;
       /* Search from back to accept misformed URI with space */
       http_version = line + line_len - 1;
       /* Skip any trailing spaces */
@@ -1572,7 +1589,7 @@
   if (NULL != daemon->uri_log_callback)
     connection->client_context
       = daemon->uri_log_callback (daemon->uri_log_callback_cls,
-                                 uri,
+                                 curi,
                                  connection);
   if (NULL != args)
     {
@@ -1585,10 +1602,11 @@
                            &connection_add_header,
                            &unused_num_headers);
     }
-  daemon->unescape_callback (daemon->unescape_callback_cls,
-                            connection,
-                            uri);
-  connection->url = uri;
+  if (NULL != uri)
+    daemon->unescape_callback (daemon->unescape_callback_cls,
+                               connection,
+                               uri);
+  connection->url = curi;
   return MHD_YES;
 }
 

Modified: libmicrohttpd/src/microhttpd/daemon.c
===================================================================
--- libmicrohttpd/src/microhttpd/daemon.c       2016-08-15 11:05:56 UTC (rev 
37720)
+++ libmicrohttpd/src/microhttpd/daemon.c       2016-08-15 11:13:50 UTC (rev 
37721)
@@ -1281,6 +1281,17 @@
 
 
 /**
+ * Free resources associated with all closed connections.
+ * (destroy responses, free buffers, etc.).  All closed
+ * connections are kept in the "cleanup" doubly-linked list.
+ *
+ * @param daemon daemon to clean up
+ */
+static void
+MHD_cleanup_connections (struct MHD_Daemon *daemon);
+
+
+/**
  * Add another client connection to the set of connections
  * managed by MHD.  This API is usually not needed (since
  * MHD will accept inbound connections on the server socket).
@@ -1371,6 +1382,8 @@
             client_socket);
 #endif
 #endif
+  //if (daemon->connections == daemon->connection_limit)
+  // MHD_cleanup_connections (daemon); /* try to aggressively clean up to make 
room */
   if ( (daemon->connections == daemon->connection_limit) ||
        (MHD_NO == MHD_ip_limit_add (daemon, addr, addrlen)) )
     {
@@ -1537,7 +1550,7 @@
 
   if (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION))
   {
-    if (!MHD_mutex_lock_ (&daemon->cleanup_connection_mutex))
+    if (! MHD_mutex_lock_ (&daemon->cleanup_connection_mutex))
       MHD_PANIC ("Failed to acquire cleanup mutex\n");
   }
   else
@@ -2070,7 +2083,7 @@
   struct MHD_Connection *pos;
 
   if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
-       (!MHD_mutex_lock_ (&daemon->cleanup_connection_mutex)) )
+       (! MHD_mutex_lock_ (&daemon->cleanup_connection_mutex)) )
     MHD_PANIC ("Failed to acquire cleanup mutex\n");
   while (NULL != (pos = daemon->cleanup_head))
     {
@@ -2080,7 +2093,7 @@
       if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
           (MHD_NO == pos->thread_joined) )
        {
-         if (!MHD_join_thread_ (pos->pid))
+         if (! MHD_join_thread_ (pos->pid))
            {
              MHD_PANIC ("Failed to join a thread\n");
            }
@@ -2092,6 +2105,8 @@
 #endif
       daemon->connections--;
       daemon->at_limit = MHD_NO;
+
+      /* clean up the connection */
       if (NULL != daemon->notify_connection)
         daemon->notify_connection (daemon->notify_connection_cls,
                                    pos,
@@ -2141,7 +2156,7 @@
       free (pos);
     }
   if ( (0 != (daemon->options & MHD_USE_THREAD_PER_CONNECTION)) &&
-       (!MHD_mutex_unlock_ (&daemon->cleanup_connection_mutex)) )
+       (! MHD_mutex_unlock_ (&daemon->cleanup_connection_mutex)) )
     MHD_PANIC ("Failed to release cleanup mutex\n");
 }
 
@@ -2364,17 +2379,6 @@
 #endif
           err_state = MHD_YES;
         }
-
-      /* If we're at the connection limit, no need to
-         accept new connections; however, make sure
-         we do not miss the shutdown, so only do this
-         optimization if we have a shutdown signaling
-         pipe. */
-      if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
-           ( ( (daemon->connections == daemon->connection_limit) &&
-             (0 != (daemon->options & MHD_USE_PIPE_FOR_SHUTDOWN)) ) ||
-             (MHD_YES == daemon->at_limit) ) )
-        FD_CLR (daemon->socket_fd, &rs);
     }
   else
     {
@@ -2420,7 +2424,21 @@
         }
 #endif /* MHD_WINSOCK_SOCKETS */
     }
-
+  /* Stop listening if we are at the configured connection limit */
+  /* If we're at the connection limit, no point in really
+     accepting new connections; however, make sure we do not miss
+     the shutdown OR the termination of an existing connection; so
+     only do this optimization if we have a signaling pipe in
+     place. */
+  if ( (MHD_INVALID_SOCKET != daemon->socket_fd) &&
+       (MHD_INVALID_PIPE_ != daemon->wpipe[0]) &&
+       (0 != (daemon->options & MHD_USE_PIPE_FOR_SHUTDOWN)) &&
+       ( (daemon->connections == daemon->connection_limit) ||
+         (MHD_YES == daemon->at_limit) ) )
+    {
+      FD_CLR (daemon->socket_fd,
+              &rs);
+    }
   tv = NULL;
   if (MHD_YES == err_state)
     may_block = MHD_NO;
@@ -4185,7 +4203,7 @@
     }
 #endif
 
-  if (!MHD_mutex_init_ (&daemon->per_ip_connection_mutex))
+  if (! MHD_mutex_init_ (&daemon->per_ip_connection_mutex))
     {
 #ifdef HAVE_MESSAGES
       MHD_DLOG (daemon,
@@ -4196,7 +4214,7 @@
        MHD_PANIC ("close failed\n");
       goto free_and_fail;
     }
-  if (!MHD_mutex_init_ (&daemon->cleanup_connection_mutex))
+  if (! MHD_mutex_init_ (&daemon->cleanup_connection_mutex))
     {
 #ifdef HAVE_MESSAGES
       MHD_DLOG (daemon,
@@ -4229,12 +4247,12 @@
         ( (0 != (flags & MHD_USE_SELECT_INTERNALLY)) &&
           (0 == daemon->worker_pool_size)) ) &&
        (0 == (daemon->options & MHD_USE_NO_LISTEN_SOCKET)) &&
-       (!MHD_create_named_thread_ (&daemon->pid,
-                                   (flags & MHD_USE_THREAD_PER_CONNECTION) ?
-                                       "MHD-listen" : "MHD-single",
-                                   daemon->thread_stack_size,
-                                   &MHD_select_thread,
-                                   daemon) ) )
+       (! MHD_create_named_thread_ (&daemon->pid,
+                                    (flags & MHD_USE_THREAD_PER_CONNECTION) ?
+                                    "MHD-listen" : "MHD-single",
+                                    daemon->thread_stack_size,
+                                    &MHD_select_thread,
+                                    daemon) ) )
     {
 #ifdef HAVE_MESSAGES
       MHD_DLOG (daemon,
@@ -4342,11 +4360,11 @@
             }
 
           /* Spawn the worker thread */
-          if (!MHD_create_named_thread_(&d->pid,
-                                        "MHD-worker",
-                                        daemon->thread_stack_size,
-                                        &MHD_select_thread,
-                                        d))
+          if (! MHD_create_named_thread_(&d->pid,
+                                         "MHD-worker",
+                                         daemon->thread_stack_size,
+                                         &MHD_select_thread,
+                                         d))
             {
 #ifdef HAVE_MESSAGES
               MHD_DLOG (daemon,