From: Ruediger Pluem <rpluem@apache.org>
Date: Fri, 21 Oct 2005 21:54:27 +0000 (+0000)
Subject: Merge r327590 from trunk:
X-Git-Tag: 2.1.9~29
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=96f197db57e7f71221197ad99a50359822a17417;p=thirdparty%2Fapache%2Fhttpd.git

Merge r327590 from trunk:

* Fix PR37145 (data loss with httpd-2.0.55 reverse proxy method=post) by
  exchanging APR_BRIGADE_CONCAT with ap_save_brigade to ensure that
  transient buckets get setaside correctly between various iterations of
  ap_get_brigade calls.

Reviewed by: Joe Orton, William Rowe, Jim Jagielski, Jeff Trawick


git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@327592 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/CHANGES b/CHANGES
index 037f038c884..78d5c616bef 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.1.9
 
+  *) mod_proxy_http: Prevent data corruption of POST request bodies when
+     client accesses proxied resources with SSL. PR37145.
+     [Ruediger Pluem, William Rowe]
+
   *) mod_ssl: Fix issue which could cause spurious warnings about use
      of name-based vhosts.  PR 37051.  [Joe Orton]
 
diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 2f39d62676a..449cb9fe203 100644
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -247,7 +247,21 @@ static apr_status_t stream_reqbody_chunked(apr_pool_t *p,
              * take care of that now
              */
             bb = header_brigade;
-            APR_BRIGADE_CONCAT(bb, input_brigade);
+
+            /*
+             * Save input_brigade in bb brigade. (At least) in the SSL case
+             * input_brigade contains transient buckets whose data would get
+             * overwritten during the next call of ap_get_brigade in the loop.
+             * ap_save_brigade ensures these buckets to be set aside.
+             * Calling ap_save_brigade with NULL as filter is OK, because
+             * bb brigade already has been created and does not need to get
+             * created by ap_save_brigade.
+             */
+            status = ap_save_brigade(NULL, &bb, &input_brigade, p);
+            if (status != APR_SUCCESS) {
+                return status;
+            }
+
             header_brigade = NULL;
         }
         else {
@@ -354,7 +368,21 @@ static apr_status_t stream_reqbody_cl(apr_pool_t *p,
              * take care of that now
              */
             bb = header_brigade;
-            APR_BRIGADE_CONCAT(bb, input_brigade);
+
+            /*
+             * Save input_brigade in bb brigade. (At least) in the SSL case
+             * input_brigade contains transient buckets whose data would get
+             * overwritten during the next call of ap_get_brigade in the loop.
+             * ap_save_brigade ensures these buckets to be set aside.
+             * Calling ap_save_brigade with NULL as filter is OK, because
+             * bb brigade already has been created and does not need to get
+             * created by ap_save_brigade.
+             */
+            status = ap_save_brigade(NULL, &bb, &input_brigade, p);
+            if (status != APR_SUCCESS) {
+                return status;
+            }
+
             header_brigade = NULL;
         }
         else {
@@ -476,7 +504,21 @@ static apr_status_t spool_reqbody_cl(apr_pool_t *p,
             apr_brigade_cleanup(input_brigade);
         }
         else {
-            APR_BRIGADE_CONCAT(body_brigade, input_brigade);
+
+            /*
+             * Save input_brigade in body_brigade. (At least) in the SSL case
+             * input_brigade contains transient buckets whose data would get
+             * overwritten during the next call of ap_get_brigade in the loop.
+             * ap_save_brigade ensures these buckets to be set aside.
+             * Calling ap_save_brigade with NULL as filter is OK, because
+             * body_brigade already has been created and does not need to get
+             * created by ap_save_brigade.
+             */
+            status = ap_save_brigade(NULL, &body_brigade, &input_brigade, p);
+            if (status != APR_SUCCESS) {
+                return status;
+            }
+
         }
         
         bytes_spooled += bytes;
@@ -832,9 +874,27 @@ apr_status_t ap_proxy_http_request(apr_pool_t *p, request_rec *r,
         }
 
         apr_brigade_length(temp_brigade, 1, &bytes);
-        APR_BRIGADE_CONCAT(input_brigade, temp_brigade);
         bytes_read += bytes;
 
+        /*
+         * Save temp_brigade in input_brigade. (At least) in the SSL case
+         * temp_brigade contains transient buckets whose data would get
+         * overwritten during the next call of ap_get_brigade in the loop.
+         * ap_save_brigade ensures these buckets to be set aside.
+         * Calling ap_save_brigade with NULL as filter is OK, because
+         * input_brigade already has been created and does not need to get
+         * created by ap_save_brigade.
+         */
+        status = ap_save_brigade(NULL, &input_brigade, &temp_brigade, p);
+        if (status != APR_SUCCESS) {
+            ap_log_error(APLOG_MARK, APLOG_ERR, status, r->server,
+                         "proxy: processing prefetched request body failed"
+                         " to %pI (%s) from %s (%s)",
+                         p_conn->addr, p_conn->hostname ? p_conn->hostname: "",
+                         c->remote_ip, c->remote_host ? c->remote_host: "");
+            return status;
+        }
+
     /* Ensure we don't hit a wall where we have a buffer too small
      * for ap_get_brigade's filters to fetch us another bucket,
      * surrender once we hit 80 bytes less than MAX_MEM_SPOOL