mod_proxy_http: Avoid 417 responses for non forwardable 100-continue. PR 65666.

Stop returning 417 when mod_proxy has to forward an HTTP/1.1 request with both
"Expect: 100-continue" and "force-proxy-request-1.0" set, mod_proxy can instead
handle the 100-continue by itself before forwarding the request, like in the
"Proxy100Continue Off" case.

Note that this does not change the behaviour of httpd receiving an HTTP/1.0
request with an Expect header, ap_check_request_header() will still correctly
return 417 in this case.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1901420 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 2eac740dac5962aa9e762b5cedf02b91fe804652..630184571278d63b7aa973a2122e8795d8120f72 100644 (file)
--- a/modules/proxy/mod_proxy.h
+++ b/modules/proxy/mod_proxy.h
@@ -398,11 +398,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 d86f29afb477797566142cf33df163e9a1365d39..c0b20c2462dec5c0fb915d7a88a65b7638926128 100644 (file)
--- a/modules/proxy/mod_proxy_http.c
+++ b/modules/proxy/mod_proxy_http.c
@@ -547,10 +547,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,
@@ -2008,8 +2004,9 @@ 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))) {
         req->do_100_continue = 1;
     }
 

diff --git a/modules/proxy/proxy_util.c b/modules/proxy/proxy_util.c
index 5c55531ee425ccba6cadcb2ce99610eec393b10d..c0c6b994864635f3b692314e8e8ef08e230740e2 100644 (file)
--- a/modules/proxy/proxy_util.c
+++ b/modules/proxy/proxy_util.c
@@ -3891,9 +3891,9 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     char *buf;
     apr_table_t *saved_headers_in, *request_headers;
     apr_bucket *e;
-    int do_100_continue;
+    int force10 = 0, ping100 = 0;
     conn_rec *origin = p_conn->connection;
-    const char *fpr1, *creds;
+    const char *creds;
     proxy_dir_conf *dconf = ap_get_module_config(r->per_dir_config, &proxy_module);
 
     /*
@@ -3901,29 +3901,25 @@ 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.
      */
-    fpr1 = apr_table_get(r->subprocess_env, "force-proxy-request-1.0");
-    do_100_continue = PROXY_DO_100_CONTINUE(worker, r);
-
-    if (fpr1) {
-        /*
-         * 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);
+    if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {
+        force10 = 1;
+    }
+    else if (PROXY_SHOULD_PING_100_CONTINUE(worker, r)) {
+        ping100 = 1;
     }
-    if (apr_table_get(r->subprocess_env, "proxy-nokeepalive")) {
+    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);
@@ -4016,16 +4012,17 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
     /* Use HTTP/1.1 100-Continue as quick "HTTP ping" test
      * to backend
      */
-    if (do_100_continue) {
-        const char *val;
-
+    if (ping100) {
         /* Add the Expect header if not already there. */
-        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"))) {
+        const char *val = apr_table_get(request_headers, "Expect");
+        if (!val || (ap_cstr_casecmp(val, "100-Continue") != 0 /* fast path */
+                     && !ap_find_token(r->pool, val, "100-Continue"))) {
             apr_table_mergen(request_headers, "Expect", "100-Continue");
         }
     }
+    else if (force10 || !dconf->forward_100_continue || !r->expecting_100) {
+        apr_table_unset(request_headers, "Expect");
+    }
 
     /* X-Forwarded-*: handling
      *
@@ -4101,7 +4098,8 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p,
      * 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");
+    if (force10)
+        apr_table_unset(request_headers, "Trailer");
 
     apr_table_unset(request_headers, "Upgrade");