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

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@219533 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index fa595797a55d391c88505622dce63435097029c2..f9134d96bec77f5ace7d2659692b31427e244777 100644 (file)
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -584,11 +584,6 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
     }
     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 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
 
         /* 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_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
         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 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
         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 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
      * 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 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
     } 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 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
         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);