Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Received-SPF: pass (athena.apache.org: local policy)
Message-ID: <53BCD486.6040902@wanadoo.fr>
Date: Wed, 09 Jul 2014 07:35:02 +0200
From: Marion & Christophe JAILLET <christophe.jaillet@wanadoo.fr>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:24.0) Gecko/20100101 Thunderbird/24.6.0
MIME-Version: 1.0
To: dev@httpd.apache.org
Subject: Re: svn commit: r1608762 - in /httpd/httpd/branches/2.4.x: ./ CHANGES
 modules/proxy/proxy_util.c
References: <20140708131627.979A12388860@eris.apache.org>
In-Reply-To: <20140708131627.979A12388860@eris.apache.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit


Le 08/07/2014 15:16, jim@apache.org a écrit :
> Author: jim
> Date: Tue Jul  8 13:16:27 2014
> New Revision: 1608762
>
> URL: http://svn.apache.org/r1608762
> Log:
> Merge r1588519 from trunk:
>
> mod_proxy: When ping/pong is configured for a worker, don't send or forward
>             "100 Continue" (interim) response to the client if it does not
>             expect one.
>
> Submitted by: ylavic
> Reviewed/backported by: jim
>
> Modified:
>      httpd/httpd/branches/2.4.x/   (props changed)
>      httpd/httpd/branches/2.4.x/CHANGES
>      httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
>
> Propchange: httpd/httpd/branches/2.4.x/
> ------------------------------------------------------------------------------
>    Merged /httpd/httpd/trunk:r1588519
>
> Modified: httpd/httpd/branches/2.4.x/CHANGES
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1608762&r1=1608761&r2=1608762&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
> +++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Tue Jul  8 13:16:27 2014
> @@ -2,6 +2,10 @@
>   
>   Changes with Apache 2.4.10
>   
> +  *) mod_proxy: When ping/pong is configured for a worker, don't send or
> +     forward "100 Continue" (interim) response to the client if it does
> +     not expect one. [Yann Ylavic]
> +
>     *) mod_proxy_fcgi: Fix occasional high CPU when handling request bodies.
>        [Jeff Trawick]
>   
>
> Modified: httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c?rev=1608762&r1=1608761&r2=1608762&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c (original)
> +++ httpd/httpd/branches/2.4.x/modules/proxy/proxy_util.c Tue Jul  8 13:16:27 2014
> @@ -3307,8 +3307,22 @@ PROXY_DECLARE(int) ap_proxy_create_hdrbr
>        * to backend
>        */
>       if (do_100_continue) {
> -        apr_table_mergen(r->headers_in, "Expect", "100-Continue");
> -        r->expecting_100 = 1;
> +        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)
> +                || (strcasecmp(val, "100-Continue") != 0 // fast path
> +                    && !ap_find_token(r->pool, val, "100-Continue"))) {
> +            apr_table_mergen(r->headers_in, "Expect", "100-Continue");
> +        }
>       }
>   
>       /* X-Forwarded-*: handling

Just a few details :

     1) Shouldn't we use 100-continue (lowercase c) instead, to more 
closely match http://www.w3.org/Protocols/rfc2616/rfc2616-sec8.html, § 
8.2.3 ?
        This would also be consistent with the use of this string in 
protocol.c


     2) if of any use, in the fast path, strcmp could be used instead of 
strcasecmp


     3) // fast path, should be /* fast path */


     4) in protocol.c, around line 1212 there is:

     if (((expect = apr_table_get(r->headers_in, "Expect")) != NULL)
         && (expect[0] != '\0')) {
         /*
          * The Expect header field was added to HTTP/1.1 after RFC 2068
          * as a means to signal when a 100 response is desired and,
          * unfortunately, to signal a poor man's mandatory extension that
          * the server must understand or return 417 Expectation Failed.
          */
         if (strcasecmp(expect, "100-continue") == 0) {
             r->expecting_100 = 1;
         }

this is not consistent with the code below. Should this be changed to 
something like:
         if (strcasecmp(expect, "100-continue") == 0 ||
             ap_find_token(r->pool, expect, "100-Continue")) {
             r->expecting_100 = 1;
         }
?


Best regards,
CJ

---
Ce courrier électronique ne contient aucun virus ou logiciel malveillant parce que la protection avast! Antivirus est active.
http://www.avast.com