On Sat, Aug 2, 2014 at 10:24 AM, <rj...@apache.org> wrote:
> Author: rjung
> Date: Sat Aug 2 08:24:35 2014
> New Revision: 1615289
>
> URL: http://svn.apache.org/r1615289
> Log:
> PR53420: Proxy responses with error status and
> "ProxyErrorOverride On" hang until proxy timeout.
>
> Regression from 2.2. It was introduced by r912063
> in order to fix PR41646.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
>
[...]
> Modified: httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
> URL:
> http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c?rev=1615289&r1=1615288&r2=1615289&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/proxy/mod_proxy_http.c (original)
> +++ httpd/httpd/trunk/modules/proxy/mod_proxy_http.c Sat Aug 2 08:24:35 2014
> @@ -1637,6 +1637,18 @@ int ap_proxy_http_process_response(apr_p
> if (!r->header_only && /* not HEAD request */
> (proxy_status != HTTP_NO_CONTENT) && /* not 204 */
> (proxy_status != HTTP_NOT_MODIFIED)) { /* not 304 */
> + const char *tmp;
> + /* Add minimal headers needed to allow http_in filter
> + * detecting end of body without waiting for a timeout. */
> + if ((tmp = apr_table_get(r->headers_out, "Content-Length")))
> {
> + apr_table_set(backend->r->headers_in, "Content-Length",
> tmp);
> + }
> + else if ((tmp = apr_table_get(r->headers_out,
> "Transfer-Encoding"))) {
> + apr_table_set(backend->r->headers_in,
> "Transfer-Encoding", tmp);
> + }
> + else if (te) {
> + apr_table_set(backend->r->headers_in,
> "Transfer-Encoding", te);
> + }
Shouldn't these tests be in reverse order, ie. :
if ((tmp = apr_table_get(r->headers_out, "Transfer-Encoding"))) {
...
}
else if ((tmp = apr_table_get(r->headers_out, "Content-Length"))) {
...
}
else if (te) {
...
}
?
This is what http_in filter does, since according to the RFC, when
both TE and CL are there, the former wins.
I'm not sure it can happen because mod_proxy_http removes the CL above
in this case, but OTOH the code and comment below suggest
r->headers_out may change in between, so we probably want to be safe
about possible response splitting...
Regards,
Yann.
