Merge pull 319 (trunk: r1901420, r1901446, r1901485, r1901486):

  *) mod_proxy_http: Avoid 417 responses for non forwardable 100-continue.
     PR 65666.  [Yann Ylavic]

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

diff --git a/changes-entries/proxy_no_417.txt b/changes-entries/proxy_no_417.txt
new file mode 100644 (file)
index 0000000..3bf8893
--- /dev/null
+++ b/changes-entries/proxy_no_417.txt
@@ -0,0 +1,2 @@
+  *) mod_proxy_http: Avoid 417 responses for non forwardable 100-continue.
+     PR 65666.  [Yann Ylavic]
\ No newline at end of file

diff --git a/modules/proxy/mod_proxy.h b/modules/proxy/mod_proxy.h
index 71ffa8fb77298e99497e9aa3c0c865b3564740c3..aba3bdce8af6ecf2ac8d5d34a0306dbda58f479b 100644 (file)
--- a/modules/proxy/mod_proxy.h
+++ b/modules/proxy/mod_proxy.h
@@ -392,11 +392,14 @@ do {                             \
 (w)->s->io_buffer_size_set   = (c)->io_buffer_size_set;    \
 } while (0)
 
+#define PROXY_SHOULD_PING_100_CONTINUE(w, r) \
+    ((w)->s->ping_timeout_set \
+     && (PROXYREQ_REVERSE == (r)->proxyreq) \
+     && ap_request_has_body((r)))
+
 #define PROXY_DO_100_CONTINUE(w, r) \
-((w)->s->ping_timeout_set \
- && (PROXYREQ_REVERSE == (r)->proxyreq) \
- && !(apr_table_get((r)->subprocess_env, "force-proxy-request-1.0")) \
- && ap_request_has_body((r)))
+    (PROXY_SHOULD_PING_100_CONTINUE(w, r) \
+     && !apr_table_get((r)->subprocess_env, "force-proxy-request-1.0"))
 
 /* use 2 hashes */
 typedef struct {

diff --git a/modules/proxy/mod_proxy_http.c b/modules/proxy/mod_proxy_http.c
index 10b555c74a8690ef8ddf42fc7cf3a99e66312be5..d74ae054ac9a423a8036fb3aeba4b2dc02581c0b 100644 (file)
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -463,10 +463,6 @@ static int ap_proxy_http_prefetch(proxy_http_req_t *req,
     apr_off_t bytes;
     int rv;
 
-    if (req->force10 && r->expecting_100) {
-        return HTTP_EXPECTATION_FAILED;
-    }
-
     rv = ap_proxy_create_hdrbrgd(p, header_brigade, r, p_conn,
                                  req->worker, req->sconf,
                                  uri, url, req->server_portstr,
@@ -1930,8 +1926,11 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker,
     apr_pool_userdata_get((void **)&input_brigade, "proxy-req-input", p);
 
     /* Should we handle end-to-end or ping 100-continue? */
-    if ((r->expecting_100 && (dconf->forward_100_continue || input_brigade))
-            || PROXY_DO_100_CONTINUE(worker, r)) {
+    if (!req->force10
+        && ((r->expecting_100 && (dconf->forward_100_continue || input_brigade))
+            || PROXY_SHOULD_PING_100_CONTINUE(worker, r))) {
+        /* Tell ap_proxy_create_hdrbrgd() to preserve/add the Expect header */
+        apr_table_setn(r->notes, "proxy-100-continue", "1");
         req->do_100_continue = 1;
     }
 

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index c88af92814e4850ec623862c65db615ff0c2d82d..e488aa6c00ebf8d8e7a56789e8d5a1092847adee 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -3876,8 +3876,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     const apr_array_header_t *headers_in_array;
     const apr_table_entry_t *headers_in;
     apr_bucket *e;
-    int do_100_continue;
+    int force10 = 0, do_100_continue = 0;
     conn_rec *origin = p_conn->connection;
+    const char *host, *val;
     proxy_dir_conf *dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
 
     /*
@@ -3885,28 +3886,26 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
      * To be compliant, we only use 100-Continue for requests with bodies.
      * We also make sure we won't be talking HTTP/1.0 as well.
      */
-    do_100_continue = PROXY_DO_100_CONTINUE(worker, r);
-
     if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {
-        /*
-         * According to RFC 2616 8.2.3 we are not allowed to forward an
-         * Expect: 100-continue to an HTTP/1.0 server. Instead we MUST return
-         * a HTTP_EXPECTATION_FAILED
-         */
-        if (r->expecting_100) {
-            return HTTP_EXPECTATION_FAILED;
-        }
-        buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.0" CRLF, NULL);
-        p_conn->close = 1;
-    } else {
-        buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" CRLF, NULL);
+        force10 = 1;
     }
-    if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) {
+    else if (apr_table_get(r->notes, "proxy-100-continue")
+             || PROXY_SHOULD_PING_100_CONTINUE(worker, r)) {
+        do_100_continue = 1;
+    }
+    if (force10 || apr_table_get(r->subprocess_env, "proxy-nokeepalive")) {
         if (origin) {
             origin->keepalive = AP_CONN_CLOSE;
         }
         p_conn->close = 1;
     }
+
+    if (force10) {
+        buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.0" CRLF, NULL);
+    }
+    else {
+        buf = apr_pstrcat(p, r->method, " ", url, " HTTP/1.1" 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);
@@ -3953,45 +3952,40 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     apr_table_unset(r->headers_in, "Trailer");
     apr_table_unset(r->headers_in, "TE");
 
-    /* We used to send `Host: ` always first, so let's keep it that
-     * way. No telling which legacy backend is relying no this.
-     */
+    /* Compute Host header */
     if (dconf->preserve_host == 0) {
         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);
+                host = apr_pstrcat(r->pool, "[", uri->hostname, "]:",
+                                   uri->port_str, NULL);
             } else {
-                buf = apr_pstrcat(p, "Host: [", uri->hostname, "]", CRLF, NULL);
+                host = 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);
+                host = apr_pstrcat(r->pool, uri->hostname, ":",
+                                   uri->port_str, NULL);
             } else {
-                buf = apr_pstrcat(p, "Host: ", uri->hostname, CRLF, NULL);
+                host = uri->hostname;
             }
         }
+        apr_table_setn(r->headers_in, "Host", host);
     }
     else {
-        /* don't want to use r->hostname, as the incoming header might have a
-         * port attached
+        /* don't want to use r->hostname as the incoming header might have a
+         * port attached, let's use the original header.
          */
-        const char* hostname = saved_host;
-        if (!hostname) {
-            hostname =  r->server->server_hostname;
+        host = saved_host;
+        if (!host) {
+            host =  r->server->server_hostname;
             ap_log_rerror(APLOG_MARK, APLOG_WARNING, 0, r, APLOGNO(01092)
                           "no HTTP 0.9 request (with no host line) "
                           "on incoming request and preserve host set "
                           "forcing hostname to be %s for uri %s",
-                          hostname, r->uri);
+                          host, r->uri);
+            apr_table_setn(r->headers_in, "Host", host);
         }
-        buf = apr_pstrcat(p, "Host: ", hostname, 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);
-    apr_table_unset(r->headers_in, "Host");
 
     /* handle Via */
     if (conf->viaopt == via_block) {
@@ -4026,15 +4020,19 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
      * to backend
      */
     if (do_100_continue) {
-        const char *val;
-
         /* Add the Expect header if not already there. */
-        if (((val = apr_table_get(r->headers_in, "Expect")) == NULL)
-                || (ap_cstr_casecmp(val, "100-Continue") != 0 /* fast path */
-                    && !ap_find_token(r->pool, val, "100-Continue"))) {
+        if (!(val = apr_table_get(r->headers_in, "Expect"))
+            || (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");
         }
     }
+    else {
+        /* XXX: we should strip the 100-continue token only from the
+         * Expect header, but are there others actually used anywhere?
+         */
+        apr_table_unset(r->headers_in, "Expect");
+    }
 
     /* X-Forwarded-*: handling
      *
@@ -4103,7 +4101,22 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     /* run hook to fixup the request we are about to send */
     proxy_run_fixups(r);
 
-    /* send request headers */
+    /* We used to send `Host: ` always first, so let's keep it that
+     * way. No telling which legacy backend is relying on this.
+     * If proxy_run_fixups() changed the value, use it (though removal
+     * is ignored).
+     */
+    val = apr_table_get(r->headers_in, "Host");
+    if (val) {
+        apr_table_unset(r->headers_in, "Host");
+        host = val;
+    }
+    buf = apr_pstrcat(p, "Host: ", host, 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);
+
+    /* Append the (remaining) headers to the brigade */
     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++) {