From: Colm MacCarthaigh Date: Tue, 24 Jan 2006 22:57:33 +0000 (+0000) Subject: Merge r327590 from trunk: X-Git-Tag: 2.0.56~77 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=27326063b72fd302d5ba3447de08b2ff9b6e8e98;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. Submitted by: rpluem git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.0.x@372046 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index b3aad3fbb3a..ae22fdf8acb 100644 --- a/CHANGES +++ b/CHANGES @@ -1,6 +1,10 @@ -*- coding: utf-8 -*- Changes with Apache 2.0.56 + *) mod_proxy_http: Prevent data corruption of POST request bodies when + client accesses proxied resources with SSL. PR 37145. + [Ruediger Pluem, William Rowe] + *) Elimiated the NET_TIME filter, restructuring the timeout logic. This provides a working mod_echo on all platforms, and ensures any custom protocol module is at least given an initial timeout value diff --git a/STATUS b/STATUS index be0a40c56b9..97d87e98eb9 100644 --- a/STATUS +++ b/STATUS @@ -106,15 +106,6 @@ CURRENT RELEASE NOTES: RELEASE SHOWSTOPPERS: - *) mod_proxy: Fix PR37145 (data loss with httpd-2.0.55 reverse proxy - method=post). - Ruediger says: This is a regression against 2.0.54 and causes - severe trouble with reverse proxy configurations - in conjunction with SSL. Many users have complained - about this problem in bugzilla and on the dev list. - Plus: It is only missing one vote :-) - - PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] @@ -147,14 +138,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK: as well. colm: another +1 on the later patch too. - *) mod_proxy: Fix PR37145 - (data loss with httpd-2.0.55 reverse proxy method=post). - Trunk version of patch: - http://svn.apache.org/viewcvs.cgi?rev=327590&view=rev - Backport version for 2.0.x of patch: - http://issues.apache.org/bugzilla/attachment.cgi?id=16744 - +1: rpluem, trawick, jim - *) mod_cache: Fix PR37347 (mod_disk_cache replaces HTTP Status 301 with 200) Trunk version of patch: diff --git a/modules/proxy/proxy_http.c b/modules/proxy/proxy_http.c index 225b7011b4e..d4906fcaa6f 100644 --- a/modules/proxy/proxy_http.c +++ b/modules/proxy/proxy_http.c @@ -504,7 +504,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 { @@ -611,7 +625,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 { @@ -735,7 +763,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; @@ -1081,9 +1123,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 %s from %s (%s)", + p_conn->name ? p_conn->name: "", + 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