httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wr...@apache.org
Subject svn commit: r219533 - /httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Date Mon, 18 Jul 2005 16:52:40 GMT
Author: wrowe
Date: Mon Jul 18 09:48:25 2005
New Revision: 219533

URL: http://svn.apache.org/viewcvs?rev=219533&view=rev
Log:

  Yet another snafu in body handling.  We need to clearly avoid any
  ap_get_brigade or request body processing in every *subrequest* 
  proxy action.  The new code introduced more chaos because we read
  the request body irrespective of any bogus header handling bugs.

  This requires a goto, and yes, that sucks :)  But this is one of those
  oddball cases where jumping away makes more sense than tons of indented
  code, IMHO.  And if you count the number of goto's I've committed to
  httpd, you know I avoid them like the plague.

  I woulda' suggestd to jorton to take a flying carnal act, except that 
  each time he points me back to the 2.0 patch, I catch another entirely 
  bogus choice within the old/new httpd-2.x request body code :)

  I've bumped the 2.0 patch to correspond; see
  http://people.apache.org/~wrowe/httpd-2.0-proxy-request-4.patch

Modified:
    httpd/httpd/trunk/modules/proxy/mod_proxy_http.c

Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=219533&r1=219532&r2=219533&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
+++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Mon Jul 18 09:48:25 2005
@@ -584,11 +584,6 @@
     }
     ap_proxy_clear_connection(p, r->headers_in);
 
-    /* sub-requests never use keepalives */
-    if (r->main) {
-        p_conn->close++;
-    }
-
     if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {
         buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.0" CRLF, NULL);
         force10 = 1;
@@ -752,9 +747,7 @@
 
         /* for sub-requests, ignore freshness/expiry headers */
         if (r->main) {
-            if (headers_in[counter].key == NULL
-                 || headers_in[counter].val == NULL
-                 || !strcasecmp(headers_in[counter].key, "If-Match")
+            if (    !strcasecmp(headers_in[counter].key, "If-Match")
                  || !strcasecmp(headers_in[counter].key, "If-Modified-Since")
                  || !strcasecmp(headers_in[counter].key, "If-Range")
                  || !strcasecmp(headers_in[counter].key, "If-Unmodified-Since")
@@ -771,6 +764,32 @@
         APR_BRIGADE_INSERT_TAIL(header_brigade, e);
     }
 
+    /* We have headers, let's figure out our request body... */
+    input_brigade = apr_brigade_create(p, bucket_alloc);
+
+    /* sub-requests never use keepalives, and mustn't pass request bodies.
+     * Because the new logic looks at input_brigade, we will self-terminate
+     * input_brigade and jump past all of the request body logic...
+     * Reading anything with ap_get_brigade is likely to consume the
+     * main request's body or read beyond EOS - which would be unplesant.
+     */
+    if (r->main) {
+        /* XXX: Why DON'T sub-requests use keepalives? */
+        p_conn->close++;
+        if (old_cl_val) {
+            old_cl_val = NULL;
+            apr_table_unset(r->headers_in, "Content-Length");
+        }
+        if (old_te_val) {
+            old_te_val = NULL;
+            apr_table_unset(r->headers_in, "Transfer-Encoding");
+        }
+        rb_method = RB_STREAM_CL;
+        e = apr_bucket_eos_create(input_brigade->bucket_alloc);
+        APR_BRIGADE_INSERT_TAIL(input_brigade, e);
+        goto skip_body;
+    }
+
     /* WE only understand chunked.  Other modules might inject
      * (and therefore, decode) other flavors but we don't know
      * that the can and have done so unless they they remove
@@ -786,6 +805,17 @@
         return APR_EINVAL;
     }
 
+    if (old_cl_val && old_te_val) {
+        ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_ENOTIMPL, r->server,
+                     "proxy: client %s (%s) requested Transfer-Encoding "
+                     "chunked body with Content-Length (C-L ignored)",
+                     c->remote_ip, c->remote_host ? c->remote_host: "");
+        apr_table_unset(r->headers_in, "Content-Length");
+        old_cl_val = NULL;
+        origin->keepalive = AP_CONN_CLOSE;
+        p_conn->close++;
+    }
+
     /* Prefetch MAX_MEM_SPOOL bytes
      *
      * This helps us avoid any election of C-L v.s. T-E
@@ -794,7 +824,6 @@
      * us an instant C-L election if the body is of some
      * reasonable size.
      */
-    input_brigade = apr_brigade_create(p, bucket_alloc);
     temp_brigade = apr_brigade_create(p, bucket_alloc);
     do {
         status = ap_get_brigade(r->input_filters, temp_brigade,
@@ -821,15 +850,6 @@
     } while ((bytes_read < MAX_MEM_SPOOL - 80) 
               && !APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade)));
 
-    if (old_cl_val && old_te_val) {
-        ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_ENOTIMPL, r->server,
-                     "proxy: client %s (%s) requested Transfer-Encoding "
-                     "chunked body with Content-Length (C-L ignored)",
-                     c->remote_ip, c->remote_host ? c->remote_host: "");
-        origin->keepalive = AP_CONN_CLOSE;
-        p_conn->close++;
-    }
-
     /* Use chunked request body encoding or send a content-length body?
      *
      * Prefer C-L when:
@@ -909,6 +929,8 @@
         rb_method = RB_SPOOL_CL;
     }
 
+/* Yes I hate gotos.  This is the subrequest shortcut */
+skip_body:
     /* Handle Connection: header */
     if (!force10 && p_conn->close) {
         buf = apr_pstrdup(p, "Connection: close" CRLF);



Mime
View raw message