[Patch 2.0.x] Re: svn commit: r307031 - in /httpd/httpd/branches/2.2.x: CHANGES include/ap_mmn.h include/http_core.h modules/http/http_core.c server/core.c server/core_filters.c server/protocol.c

[William A. Rowe, Jr.]

Folks,

the patch below cannot be literally backported to 2.0.x.  Everyone would
agree that the echo filter is inherently a DoS attack even when it's
working as expected.  But other protocol module authors have based their
code on this module; and it was dangerous precident to never establish
any timeout.

We could, obviously, throw the same hack as mod_ftp's code in mod_echo,
prior to the first get_brigade invocation;

     client_socket = ap_get_module_config(cdata->conn_config,
                                          &core_module);
     apr_socket_timeout_set(client_socket, cdata->base_server->timeout);

but that's hackish, and truly does nothing for any existing borked
protocol module out there, written by unsuspecting authors.

Here's a proposed patch for 2.0 that would move the -initial- timeout
setting from underneath NET_TIME.  Yet it would retain the NET_TIME
filter for managing keep-alive time outs, thus maintaining all binary
and nearly identical behavior compatibility in the 2.0.x branch.

Comments?


Index: server/core.c
===================================================================
--- server/core.c	(revision 307178)
+++ server/core.c	(working copy)
@@ -3671,11 +3671,9 @@
     }
 
     if (mode != AP_MODE_INIT && mode != AP_MODE_EATCRLF) {
-        if (ctx->first_line) {
-            apr_socket_timeout_set(ctx->csd, 
-                                   keptalive
-                                      ? f->c->base_server->keep_alive_timeout
-                                      : f->c->base_server->timeout);
+        if (keptalive && ctx->first_line) {
+            apr_socket_timeout_set(ctx->csd,
+                                   f->c->base_server->keep_alive_timeout);
             ctx->first_line = 0;
         }
         else {
@@ -4485,10 +4483,9 @@
 static int core_pre_connection(conn_rec *c, void *csd)
 {
     core_net_rec *net = apr_palloc(c->pool, sizeof(*net));
+    apr_status_t rv;
 
 #ifdef AP_MPM_DISABLE_NAGLE_ACCEPTED_SOCK
-    apr_status_t rv;
-
     /* BillS says perhaps this should be moved to the MPMs. Some OSes
      * allow listening socket attributes to be inherited by the
      * accept sockets which means this call only needs to be made
@@ -4510,6 +4507,20 @@
                       "apr_socket_opt_set(APR_TCP_NODELAY)");
     }
 #endif
+
+    /* The core filter requires the timeout mode to be set, which
+     * incidentally sets the socket to be nonblocking.  If this
+     * is not initialized correctly, Linux - for example - will
+     * be initially blocking, while Solaris will be non blocking
+     * and any initial read will fail.
+     */
+    rv = apr_socket_timeout_set(csd, c->base_server->timeout);
+    if (rv != APR_SUCCESS) {
+        /* expected cause is that the client disconnected already */
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, c,
+                     "apr_socket_timeout_set");
+    }
+
     net->c = c;
     net->in_ctx = NULL;
     net->out_ctx = NULL;