Mailing List Archive

On 4/4/22 11:41 AM, icing@apache.org wrote:
> Author: icing
> Date: Mon Apr 4 09:41:25 2022
> New Revision: 1899550
>
> URL: http://svn.apache.org/viewvc?rev=1899550&view=rev
> Log:
> *) core: add ap_h1_append_header() for single header values.
> *) mod_proxy: use of new ap_h1_header(s) functions for
> formatting HTTP/1.1 requests.
>
>
> Modified:
> httpd/httpd/trunk/include/ap_mmn.h
> httpd/httpd/trunk/include/http_protocol.h
> httpd/httpd/trunk/modules/http/http_protocol.c
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> httpd/httpd/trunk/modules/proxy/proxy_util.c
>

> Modified: httpd/httpd/trunk/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c?rev=1899550&r1=1899549&r2=1899550&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/trunk/modules/proxy/proxy_util.c Mon Apr 4 09:41:25 2022

> @@ -3913,28 +3910,51 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
> ap_xlate_proto_to_ascii(buf, strlen(buf));
> e = apr_bucket_pool_create(buf, strlen(buf), p, c->bucket_alloc);
> APR_BRIGADE_INSERT_TAIL(header_brigade, e);
> +
> + /*
> + * Make a copy on r->headers_in for the request we make to the backend.
> + * This we modify according to our configuration and connection handling.
> + * Leave the original headers we received from the client untouched.
> + *
> + * Note: We need to take r->pool for apr_table_copy as the key / value
> + * pairs in r->headers_in have been created out of r->pool and
> + * p might be (and actually is) a longer living pool.

Hm. I found two calls to ap_proxy_create_hdrbr in mod_proxy_http.c and mod_proxy_wstunnel.c and both seem to pass r->pool as pool p.
There is currently no documentation on how p relates to r->pool.
But I agree with your further comments that we should allocate further headers from r->pool to be consistent in the table and
to use r->pool for the table copy.


> + * This would trigger the bad pool ancestry abort in apr_table_copy if
> + * apr is compiled with APR_POOL_DEBUG.
> + *
> + * icing: if p indeed lives longer than r->pool, we should allocate
> + * all new header values from r->pool as well and avoid leakage.
> + */
> + request_headers = apr_table_copy(r->pool, r->headers_in);
> +
> + /* We used to send `Host: ` always first, so let's keep it that
> + * way. No telling which legacy backend is relying no this.
> + */
> if (dconf->preserve_host == 0) {
> + const char *nhost;
> if (ap_strchr_c(uri->hostname, ':')) { /* if literal IPv6 address */
> if (uri->port_str && uri->port != DEFAULT_HTTP_PORT) {
> - buf = apr_pstrcat(p, "Host: [", uri->hostname, "]:",
> - uri->port_str, CRLF, NULL);
> + nhost = apr_pstrcat(r->pool, "[", uri->hostname, "]:",
> + uri->port_str, NULL);
> } else {
> - buf = apr_pstrcat(p, "Host: [", uri->hostname, "]", CRLF, NULL);
> + nhost = apr_pstrcat(r->pool, "[", uri->hostname, "]", NULL);
> }
> } else {
> if (uri->port_str && uri->port != DEFAULT_HTTP_PORT) {
> - buf = apr_pstrcat(p, "Host: ", uri->hostname, ":",
> - uri->port_str, CRLF, NULL);
> + nhost = apr_pstrcat(r->pool, uri->hostname, ":",
> + uri->port_str, NULL);
> } else {
> - buf = apr_pstrcat(p, "Host: ", uri->hostname, CRLF, NULL);
> + nhost = uri->hostname;
> }
> }
> + ap_h1_append_header(header_brigade, r->pool, "Host", nhost);
> + apr_table_unset(request_headers, "Host");
> }
> else {
> /* don't want to use r->hostname, as the incoming header might have a
> * port attached
> */
> - const char* hostname = apr_table_get(r->headers_in,"Host");
> + const char* hostname = apr_table_get(request_headers, "Host");
> if (!hostname) {
> hostname = r->server->server_hostname;
> ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01092)

Regards

Rüdiger