httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Yann Ylavic <ylavic....@gmail.com>
Subject Re: svn commit: r1754732 - /httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
Date Mon, 01 Aug 2016 23:03:01 GMT
On Mon, Aug 1, 2016 at 12:55 PM,  <elukey@apache.org> wrote:
>
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c?rev=1754732&r1=1754731&r2=1754732&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_fcgi.c Mon Aug  1 10:55:03 2016
> @@ -661,14 +661,26 @@ recv_again:
>                                      break;
>                                  }
>                                  else if (status == HTTP_NOT_MODIFIED) {
> -                                    /* The 304 response MUST NOT contain
> -                                     * a message-body, ignore it.
> -                                     * The break is not added since there might
> -                                     * be more bytes to read from the FCGI
> -                                     * connection. Even if the message-body is
> -                                     * ignored we want to avoid subsequent
> -                                     * bogus reads. */
> +                                    /* A 304 response MUST NOT contain
> +                                     * a message-body, so we must ignore it but
> +                                     * some extra steps needs to be taken to
> +                                     * avoid inconsistencies.
> +                                     * The break is not added with connection
> +                                     * reuse set since there might be more bytes
> +                                     * to read from the FCGI connection,
> +                                     * like the message-body,

There is no more to read for *this* request, it has its response: 304
(header only).
If we read something that's for the next request (some response for no
request), so we can discard it (conn->close=1 and done+break).
But that'd better be checked just before reusing the connection
(before sending the next request), the later detected the better; if
anything read do not reuse (close) and establish a new one.

> +                                     * subsequent bogus reads (for example
> +                                     * the start of the message-body
> +                                     * interpreted as FCGI header).
> +                                     * With connecton reuse disabled (default)
> +                                     * we can safely break and force the end
> +                                     * of the FCGI processing phase since the
> +                                     * connection will be cleaned up later on. */
>                                      ignore_body = 1;
> +                                    if (conn->close) {
> +                                        done = 1;
> +                                        break;
> +                                    }

So this does not look correct to me, disablereuse or "connection:
close" shouldn't control the behaviour, unless we close+break below by
reading data with ignore_body == 1 (but that's too early IHMO).


Regards,
Yann.

Mime
View raw message