*) 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.



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

5 files changed, 120 insertions, 120 deletions

diff --git a/include/ap_mmn.h b/include/ap_mmn.h
index 04fb184048..25fa16e963 100644
--- a/include/ap_mmn.h
+++ b/include/ap_mmn.h
@@ -709,6 +709,7 @@
  *                         Add field `body_indeterminate` in request_rec
  *                         Add new http/1.x formatting helpers
  *                         Add ap_assign_request()
+ * 20211221.7 (2.5.1-dev)  Add ap_h1_append_header()
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
@@ -716,7 +717,7 @@
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
 #define MODULE_MAGIC_NUMBER_MAJOR 20211221
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 6             /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 7             /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
diff --git a/include/http_protocol.h b/include/http_protocol.h
index b522f70928..589517f7d2 100644
--- a/include/http_protocol.h
+++ b/include/http_protocol.h
@@ -1320,6 +1320,16 @@ AP_DECLARE(void) ap_set_std_response_headers(request_rec *r);
 AP_DECLARE(void) ap_send_interim_response(request_rec *r, int send_headers);
 
 /**
+ * Append a header in HTTP/1.1 format to the brigade.
+ * @param b the brigade to append to
+ * @param p the pool to use
+ * @param name the name of the header field
+ * @param value the value of the header field
+ */
+AP_DECLARE(apr_status_t) ap_h1_append_header(apr_bucket_brigade *b,
+                                             apr_pool_t *pool,
+                                             const char *name, const char *value);
+/**
  * Append the headers in HTTP/1.1 format to the brigade.
  * @param b the brigade to append to
  * @param r the reqeust this is done for (pool and logging)
diff --git a/modules/http/http_protocol.c b/modules/http/http_protocol.c
index 59fef62756..993462826a 100644
--- a/modules/http/http_protocol.c
+++ b/modules/http/http_protocol.c
@@ -1485,8 +1485,22 @@ AP_DECLARE(void) ap_clear_method_list(ap_method_list_t *l)
     l->method_list->nelts = 0;
 }
 
-/* Send a request's HTTP response headers to the client.
- */
+AP_DECLARE(apr_status_t) ap_h1_append_header(apr_bucket_brigade *b,
+                                             apr_pool_t *p,
+                                             const char *name, const char *value)
+{
+    char *buf;
+    apr_size_t len;
+
+    if (!name || !*name || !value || !*value) {
+        return APR_SUCCESS;
+    }
+    buf = apr_pstrcat(p, name, ": ", value, CRLF, NULL);
+    len = strlen(buf);
+    ap_xlate_proto_to_ascii(buf, len);
+    return apr_brigade_write(b, NULL, NULL, buf, len);
+}
+
 AP_DECLARE(apr_status_t) ap_h1_append_headers(apr_bucket_brigade *bb,
                                               request_rec *r,
                                               apr_table_t *headers)
diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 91be06fc12..b617c8e728 100644
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -516,10 +516,6 @@ static int stream_reqbody(proxy_http_req_t *req)
 
 static void terminate_headers(proxy_http_req_t *req)
 {
-    apr_bucket_alloc_t *bucket_alloc = req->bucket_alloc;
-    apr_bucket *e;
-    char *buf;
-
     /*
      * Handle Connection: header if we do HTTP/1.1 request:
      * If we plan to close the backend connection sent Connection: close
@@ -527,28 +523,20 @@ static void terminate_headers(proxy_http_req_t *req)
      */
     if (!req->force10) {
         if (req->upgrade) {
-            buf = apr_pstrdup(req->p, "Connection: Upgrade" CRLF);
-            ap_xlate_proto_to_ascii(buf, strlen(buf));
-            e = apr_bucket_pool_create(buf, strlen(buf), req->p, bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(req->header_brigade, e);
-
             /* Tell the backend that it can upgrade the connection. */
-            buf = apr_pstrcat(req->p, "Upgrade: ", req->upgrade, CRLF, NULL);
+            ap_h1_append_header(req->header_brigade, req->p, "Connection", "Upgrade");
+            ap_h1_append_header(req->header_brigade, req->p, "Upgrade", req->upgrade);
         }
         else if (ap_proxy_connection_reusable(req->backend)) {
-            buf = apr_pstrdup(req->p, "Connection: Keep-Alive" CRLF);
+            ap_h1_append_header(req->header_brigade, req->p, "Connection", "Keep-Alive");
         }
         else {
-            buf = apr_pstrdup(req->p, "Connection: close" CRLF);
+            ap_h1_append_header(req->header_brigade, req->p, "Connection", "close");
         }
-        ap_xlate_proto_to_ascii(buf, strlen(buf));
-        e = apr_bucket_pool_create(buf, strlen(buf), req->p, bucket_alloc);
-        APR_BRIGADE_INSERT_TAIL(req->header_brigade, e);
     }
 
     /* add empty line at the end of the headers */
-    e = apr_bucket_immortal_create(CRLF_ASCII, 2, bucket_alloc);
-    APR_BRIGADE_INSERT_TAIL(req->header_brigade, e);
+    ap_h1_terminate_header(req->header_brigade);
 }
 
 static int ap_proxy_http_prefetch(proxy_http_req_t *req,
diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index d8739e8a7f..748c8fa5c8 100644
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -816,7 +816,7 @@ PROXY_DECLARE(int) ap_proxy_pre_http_request(conn_rec *c, request_rec *r)
 {
     ap_add_input_filter("HTTP_IN", NULL, r, c);
     ap_add_input_filter("HTTP1_BODY_IN", NULL, r, c);
-   return OK;
+    return OK;
 }
 
 PROXY_DECLARE(const char *) ap_proxy_location_reverse_map(request_rec *r,
@@ -3871,11 +3871,8 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
                                             char **old_te_val)
 {
     conn_rec *c = r->connection;
-    int counter;
     char *buf;
-    const apr_array_header_t *headers_in_array;
-    const apr_table_entry_t *headers_in;
-    apr_table_t *saved_headers_in;
+    apr_table_t *saved_headers_in, *request_headers;
     apr_bucket *e;
     int do_100_continue;
     conn_rec *origin = p_conn->connection;
@@ -3913,28 +3910,51 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     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.
+     * 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)
@@ -3943,31 +3963,14 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
                           "forcing hostname to be %s for uri %s",
                           hostname, r->uri);
         }
-        buf = apr_pstrcat(p, "Host: ", hostname, CRLF, NULL);
+        ap_h1_append_header(header_brigade, r->pool, "Host", hostname);
+        apr_table_unset(request_headers, "Host");
     }
-    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);
-
-    /*
-     * Save the original headers in here and restore them when leaving, since
-     * we will apply proxy purpose only modifications (eg. clearing hop-by-hop
-     * headers, add Via or X-Forwarded-* or Expect...), whereas the originals
-     * will be needed later to prepare the correct response and logging.
-     *
-     * 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.
-     * This would trigger the bad pool ancestry abort in apr_table_copy if
-     * apr is compiled with APR_POOL_DEBUG.
-     */
-    saved_headers_in = r->headers_in;
-    r->headers_in = apr_table_copy(r->pool, saved_headers_in);
 
     /* handle Via */
     if (conf->viaopt == via_block) {
         /* Block all outgoing Via: headers */
-        apr_table_unset(r->headers_in, "Via");
+        apr_table_unset(request_headers, "Via");
     } else if (conf->viaopt != via_off) {
         const char *server_name = ap_get_server_name(r);
         /* If USE_CANONICAL_NAME_OFF was configured for the proxy virtual host,
@@ -3979,14 +3982,14 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
             server_name = r->server->server_hostname;
         /* Create a "Via:" request header entry and merge it */
         /* Generate outgoing Via: header with/without server comment: */
-        apr_table_mergen(r->headers_in, "Via",
+        apr_table_mergen(request_headers, "Via",
                          (conf->viaopt == via_full)
-                         ? apr_psprintf(p, "%d.%d %s%s (%s)",
+                         ? apr_psprintf(r->pool, "%d.%d %s%s (%s)",
                                         HTTP_VERSION_MAJOR(r->proto_num),
                                         HTTP_VERSION_MINOR(r->proto_num),
                                         server_name, server_portstr,
                                         AP_SERVER_BASEVERSION)
-                         : apr_psprintf(p, "%d.%d %s%s",
+                         : apr_psprintf(r->pool, "%d.%d %s%s",
                                         HTTP_VERSION_MAJOR(r->proto_num),
                                         HTTP_VERSION_MINOR(r->proto_num),
                                         server_name, server_portstr)
@@ -4000,10 +4003,10 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
         const char *val;
 
         /* Add the Expect header if not already there. */
-        if (((val = apr_table_get(r->headers_in, "Expect")) == NULL)
+        if (((val = apr_table_get(request_headers, "Expect")) == NULL)
                 || (ap_cstr_casecmp(val, "100-Continue") != 0 /* fast path */
                     && !ap_find_token(r->pool, val, "100-Continue"))) {
-            apr_table_mergen(r->headers_in, "Expect", "100-Continue");
+            apr_table_mergen(request_headers, "Expect", "100-Continue");
         }
     }
 
@@ -4034,103 +4037,87 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
             /* Add X-Forwarded-For: so that the upstream has a chance to
              * determine, where the original request came from.
              */
-            apr_table_mergen(r->headers_in, "X-Forwarded-For",
+            apr_table_mergen(request_headers, "X-Forwarded-For",
                              r->useragent_ip);
 
             /* Add X-Forwarded-Host: so that upstream knows what the
              * original request hostname was.
              */
             if ((buf = apr_table_get(r->headers_in, "Host"))) {
-                apr_table_mergen(r->headers_in, "X-Forwarded-Host", buf);
+                apr_table_mergen(request_headers, "X-Forwarded-Host", buf);
             }
 
             /* Add X-Forwarded-Server: so that upstream knows what the
              * name of this proxy server is (if there are more than one)
              * XXX: This duplicates Via: - do we strictly need it?
              */
-            apr_table_mergen(r->headers_in, "X-Forwarded-Server",
+            apr_table_mergen(request_headers, "X-Forwarded-Server",
                              r->server->server_hostname);
         }
     }
 
+    /* run hook to fixup the request we are about to send,
+     * this will modify r->headers_in, so give it our request_headers
+     * and restore afterwards.
+     */
+    saved_headers_in = r->headers_in;
+    r->headers_in = request_headers;
     proxy_run_fixups(r);
     if (ap_proxy_clear_connection(r, r->headers_in) < 0) {
+        r->headers_in = saved_headers_in;
         return HTTP_BAD_REQUEST;
     }
+    r->headers_in = saved_headers_in;
 
     creds = apr_table_get(r->notes, "proxy-basic-creds");
     if (creds) {
-        apr_table_mergen(r->headers_in, "Proxy-Authorization", creds);
+        apr_table_mergen(request_headers, "Proxy-Authorization", creds);
     }
 
-    /* send request headers */
-    headers_in_array = apr_table_elts(r->headers_in);
-    headers_in = (const apr_table_entry_t *) headers_in_array->elts;
-    for (counter = 0; counter < headers_in_array->nelts; counter++) {
-        if (headers_in[counter].key == NULL
-            || headers_in[counter].val == NULL
+    /* Clear out hop-by-hop request headers not to send
+     * RFC2616 13.5.1 says we should strip these headers
+     */
+    apr_table_unset(request_headers, "Keep-Alive");
+    apr_table_unset(request_headers, "TE");
 
-            /* Already sent */
-            || !ap_cstr_casecmp(headers_in[counter].key, "Host")
+    /* FIXME: since we now handle r->trailers_in on forwarding
+     * request bodies, it seems unwise to clear any Trailer
+     * header present. Is this the correct thing now?
+     */
+    if (fpr1) apr_table_unset(request_headers, "Trailer");
 
-            /* Clear out hop-by-hop request headers not to send
-             * RFC2616 13.5.1 says we should strip these headers
-             */
-            || !ap_cstr_casecmp(headers_in[counter].key, "Keep-Alive")
-            || !ap_cstr_casecmp(headers_in[counter].key, "TE")
-            || !ap_cstr_casecmp(headers_in[counter].key, "Trailer")
-            || !ap_cstr_casecmp(headers_in[counter].key, "Upgrade")
+    apr_table_unset(request_headers, "Upgrade");
 
-            ) {
-            continue;
-        }
-        /* Do we want to strip Proxy-Authorization ?
-         * If we haven't used it, then NO
-         * If we have used it then MAYBE: RFC2616 says we MAY propagate it.
-         * So let's make it configurable by env.
-         */
-        if (!ap_cstr_casecmp(headers_in[counter].key,"Proxy-Authorization")) {
-            if (r->user != NULL) { /* we've authenticated */
-                if (!apr_table_get(r->subprocess_env, "Proxy-Chain-Auth")) {
-                    continue;
-                }
-            }
-        }
+    /* Do we want to strip Proxy-Authorization ?
+     * If we haven't used it, then NO
+     * If we have used it then MAYBE: RFC2616 says we MAY propagate it.
+     * So let's make it configurable by env.
+     */
+    if (r->user != NULL /* we've authenticated */
+        && !apr_table_get(r->subprocess_env, "Proxy-Chain-Auth")) {
+        apr_table_unset(request_headers, "Proxy-Authorization");
+    }
 
         /* Skip Transfer-Encoding and Content-Length for now.
          */
-        if (!ap_cstr_casecmp(headers_in[counter].key, "Transfer-Encoding")) {
-            *old_te_val = headers_in[counter].val;
-            continue;
-        }
-        if (!ap_cstr_casecmp(headers_in[counter].key, "Content-Length")) {
-            *old_cl_val = headers_in[counter].val;
-            continue;
-        }
-
-        /* for sub-requests, ignore freshness/expiry headers */
-        if (r->main) {
-            if (   !ap_cstr_casecmp(headers_in[counter].key, "If-Match")
-                || !ap_cstr_casecmp(headers_in[counter].key, "If-Modified-Since")
-                || !ap_cstr_casecmp(headers_in[counter].key, "If-Range")
-                || !ap_cstr_casecmp(headers_in[counter].key, "If-Unmodified-Since")
-                || !ap_cstr_casecmp(headers_in[counter].key, "If-None-Match")) {
-                continue;
-            }
-        }
+    if ((*old_te_val = (char *)apr_table_get(request_headers, "Transfer-Encoding"))) {
+        apr_table_unset(request_headers, "Transfer-Encoding");
+    }
+    if ((*old_cl_val = (char *)apr_table_get(request_headers, "Content-Length"))) {
+        apr_table_unset(request_headers, "Content-Length");
+    }
 
-        buf = apr_pstrcat(p, headers_in[counter].key, ": ",
-                          headers_in[counter].val, CRLF,
-                          NULL);
-        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);
+    /* for sub-requests, ignore freshness/expiry headers */
+    if (r->main) {
+        apr_table_unset(request_headers, "If-Match");
+        apr_table_unset(request_headers, "If-Modified-Since");
+        apr_table_unset(request_headers, "If-Range");
+        apr_table_unset(request_headers, "If-Unmodified-Since");
+        apr_table_unset(request_headers, "If-None-Match");
     }
 
-    /* Restore the original headers in (see comment above),
-     * we won't modify them anymore.
-     */
-    r->headers_in = saved_headers_in;
+    ap_h1_append_headers(header_brigade, r, request_headers);
+
     return OK;
 }