On Po, 2020-09-21 at 22:07 +0300, Maxim Dounin wrote:
> 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).

Hello,

Yes. This is one of the use-case. I am also trying to cover X-Accel-
Redirect header in a response from upstream.


> > 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.

Thank you, that's a good point.


Best, Jan Prachař


_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel