Commit 129a412b authored by Yann Ylavic's avatar Yann Ylavic
Browse files

mod_proxy_http: follow up to r1836588: avoid 100-continue responses from core. 

When mod_proxy_http handles end-to-end "100 continue", it can't let
ap_http_filter() send its own interim response whenever the body is read.

So save/restore r->expecting_100 before/after handling the request, and use
req->expecting_100 internally (including to restore r->expecting appropriately).

While at it, add comments and debug logs about 100 continue handling, and
fill in missing APLOGNO()s from r1836588.


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

docs/log-message-tags/next-number

+1 −1
Changes for docs/log-message-tags/next-number: 1 added line, 1 removed line.
Original line number Diff line number Diff line 
10153
 
10155

modules/proxy/mod_proxy_http.c

+48 −15
Changes for modules/proxy/mod_proxy_http.c: 48 added lines, 15 removed lines.
Original line number Diff line number Diff line
@@ -254,6 +254,7 @@ typedef struct { 
    apr_off_t cl_val, bytes_spooled;



    int do_100_continue;

    int expecting_100;

    int flushall;

} proxy_http_req_t;
@@ -669,7 +670,7 @@ static int ap_proxy_http_prefetch(proxy_http_req_t *req, 
    apr_read_type_e block;



    if (apr_table_get(r->subprocess_env, "force-proxy-request-1.0")) {

        if (r->expecting_100) {

        if (req->expecting_100) {

            return HTTP_EXPECTATION_FAILED;

        }

        force10 = 1;
@@ -1613,11 +1614,12 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) 
            ap_log_rerror(APLOG_MARK, APLOG_TRACE2, 0, r,

                          "HTTP: received interim %d response", r->status);

            if (!policy

                    || (proxy_status == HTTP_CONTINUE

                        && r->expecting_100)

                    || (!strcasecmp(policy, "RFC")

                        && (proxy_status != HTTP_CONTINUE

                            || (r->expecting_100 = 1)))) {

                    || !strcasecmp(policy, "RFC")

                    || (proxy_status == HTTP_CONTINUE && req->expecting_100)) {

                if (proxy_status == HTTP_CONTINUE) {

                    req->expecting_100 = 0;

                    r->expecting_100 = 1;

                }

                ap_send_interim_response(r, 1);

            }

            /* FIXME: refine this to be able to specify per-response-status
@@ -1639,12 +1641,16 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) 
         * for the next one, or this is a final response and hence the backend

         * did not honor our expectation.

         */

        if (do_100_continue

                && (proxy_status == HTTP_CONTINUE || !interim_response)) {

        if (do_100_continue && (!interim_response

                                || proxy_status == HTTP_CONTINUE)) {

            int do_send_body = (proxy_status == HTTP_CONTINUE

                                || (!toclose && major > 0 && minor > 0));



            /* Reset to old timeout iff we've adjusted it. */

            if (worker->s->ping_timeout_set) {

                apr_socket_timeout_set(backend->sock, old_timeout);

            }



            /* RFC 7231 - Section 5.1.1 - Expect - Requirement for servers

             *   A server that responds with a final status code before

             *   reading the entire message body SHOULD indicate in that
@@ -1657,8 +1663,17 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) 
             * discard it or the caller try another balancer member with the

             * same body (given status 503, though not implemented yet).

             */

            if (proxy_status == HTTP_CONTINUE

                    || (major > 0 && minor > 0 && !toclose)) {

            if (proxy_status != HTTP_CONTINUE) {

                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(10153)

                              "HTTP: no 100-Continue sent by %pI (%s): "

                              "%ssending body%s (response: %s)",

                              backend->addr,

                              backend->hostname ? backend->hostname : "",

                              do_send_body ? "" : "not ",

                              do_send_body ? " anyway" : "",

                              proxy_status_line);

            }

            if (do_send_body) {

                int status;



                /* Send the request body (fully). */
@@ -1682,7 +1697,7 @@ int ap_proxy_http_process_response(proxy_http_req_t *req) 
                }

                if (status != OK) {

                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,

                            APLOGNO() "pass request body failed "

                            APLOGNO(10154) "pass request body failed "

                            "to %pI (%s) from %s (%s) with status %i",

                            backend->addr,

                            backend->hostname ? backend->hostname : "",
@@ -2082,8 +2097,9 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, 


    /* create space for state information */

    if ((status = ap_proxy_acquire_connection(proxy_function, &backend,

                                              worker, r->server)) != OK)

        goto cleanup;

                                              worker, r->server)) != OK) {

        return status;

    }



    backend->is_ssl = is_ssl;
@@ -2094,13 +2110,29 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, 
    req->worker = worker;

    req->backend = backend;

    req->bucket_alloc = c->bucket_alloc;

    req->rb_method = RB_INIT;



    /* Should we handle end-to-end or ping 100-continue? */

    if (r->expecting_100 || PROXY_DO_100_CONTINUE(worker, r)) {

        /* We need to reset r->expecting_100 or prefetching will cause

         * ap_http_filter() to send "100 Continue" response by itself. So

         * we'll use req->expecting_100 in mod_proxy_http to determine whether

         * the client should be forwarded "100 continue", and r->expecting_100

         * will be restored at the end of the function with the actual value of

         * req->expecting_100 (i.e. cleared only if mod_proxy_http sent the

         * "100 Continue" according to its policy).

         */

        req->do_100_continue = 1;

        req->expecting_100 = r->expecting_100;

        r->expecting_100 = 0;

    }



    /* Should we block while prefetching the body or try nonblocking and flush

     * data to the backend ASAP?

     */

    if (apr_table_get(r->subprocess_env, "proxy-flushall")) {

        req->flushall = 1;

    }

    req->rb_method = RB_INIT;



    /*

     * In the case that we are handling a reverse proxy connection and this
@@ -2217,7 +2249,8 @@ static int proxy_http_handler(request_rec *r, proxy_worker *worker, 


    /* Step Six: Clean Up */

cleanup:

    if (req && req->backend) {

    r->expecting_100 = req->expecting_100;

    if (req->backend) {

        if (status != OK)

            req->backend->close = 1;

        ap_proxy_http_cleanup(proxy_function, r, req->backend);

modules/proxy/proxy_util.c

+0 −8
Changes for modules/proxy/proxy_util.c: 0 added lines, 8 removed lines.
Original line number Diff line number Diff line
@@ -3738,14 +3738,6 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbrgd(apr_pool_t *p, 
    if (do_100_continue) {

        const char *val;



        if (!r->expecting_100) {

            /* Don't forward any "100 Continue" response if the client is

             * not expecting it.

             */

            apr_table_setn(r->subprocess_env, "proxy-interim-response",

                                              "Suppress");

        }



        /* 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 */