Hello!

On Mon, Sep 21, 2020 at 08:09:29PM +0200, Jan Prachař wrote:

> # HG changeset patch
> # User Jan Prachař <jan.prachar@gmail.com>
> # Date 1600710589 -7200
> # Mon Sep 21 19:49:49 2020 +0200
> # Node ID f211684e1acee34eabfdd9dd39283bcff8dc7087
> # Parent 052ecc68d35038b1b4adde12efe6249a92055f09
> Proxy: set u->keepalive also if the whole body has already been read.
>
> HTTP redirection responses often contain a small few hundred bytes
> body. This
> allows to left keep alive connection open, if the body fits into the
> upstream
> buffer.

HTTP redirection responses are expected to be read from the
connection and returned to the client. The code to do it will
properly read the response body, check it, and will set
u->keepalive as appropriate.

Most likely, you are trying to cover a specific case when
redirections responses are intercepted using
proxy_intercept_errors and error_page, as recently reported in
ticket #2033 (https://trac.nginx.org/nginx/ticket/2033).

> diff -r 052ecc68d350 -r f211684e1ace
> src/http/modules/ngx_http_proxy_module.c
> --- a/src/http/modules/ngx_http_proxy_module.c Wed Sep 16 18:26:25
> 2020 +0300
> +++ b/src/http/modules/ngx_http_proxy_module.c Mon Sep 21 19:49:49
> 2020 +0200
> @@ -1905,7 +1905,8 @@
> }
>
> /*
> - * set u->keepalive if response has no body; this allows
> to keep
> + * set u->keepalive if response has no body or if the
> whole body
> + * has been already read to u->buffer; this allows to keep
> * connections alive in case of r->header_only or X-Accel-
> Redirect
> */
>
> @@ -1915,7 +1916,7 @@
> || u->headers_in.status_n == NGX_HTTP_NOT_MODIFIED
> || ctx->head
> || (!u->headers_in.chunked
> - && u->headers_in.content_length_n == 0))
> + && u->headers_in.content_length_n <= u-
> >buffer.last-u->buffer.pos))
> {
> u->keepalive = !u->headers_in.connection_close;
> }

I can't say I like the idea of connection cacheability to depend
on timing and buffer size factors.

Further, the "<=" comparison looks wrong: we shouldn't cache
connections if there are more data than expected.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel