[PATCH] Chunked filter: check if ctx is null
jan.prachar at gmail.com
Fri Jan 5 12:20:03 UTC 2018
On Fri, 2018-01-05 at 08:41 +0300, Maxim Dounin wrote:
> On Thu, Jan 04, 2018 at 11:08:11PM +0100, Jan Prachar wrote:
> > Hello, thank you for response!
> > On Thu, 2018-01-04 at 03:42 +0300, Maxim Dounin wrote:
> > > Hello!
> > >
> > > On Wed, Jan 03, 2018 at 07:53:00PM +0100, Jan Pracha=C5=99 wrote:
> > >
> > > To catch cases when a duplicate response is returned after the
> > > header was already sent we have a dedicated check in the
> > > ngx_http_send_header() function, see this commit for details:
> > >
> > > http://hg.nginx.org/nginx/rev/03ff14058272
> > >
> > > Trying to bypass this check is a bad idea. The same applies to
> > > conditionally sending headers based on the r->headers_sent flag,
> > > as it will mean that the check will be bypassed. This is what
> > > the
> > > lua module seems to be doing, and it should be fixed to avoid
> > > doing this.
> > Lua module checks r->header_sent in function
> > ngx_http_lua_send_header_if_needed(), which is called with every
> > output. See
> > https://github.com/openresty/lua-nginx-module/commit/235875b5c6afd4
> > 9611
> > 81fa9ead9c167dc865e737
> > So you suggest, that they should have their own flag (like they
> > already
> > had - ctx->headers_sent) and always call ngx_http_send_header()
> > function, if this flag is not set?
Thanks. I will report it to lua module developers.
> > > The other part of the equation is how and why error_page is
> > > called
> > > after the header as already sent. If you know a scenario where
> > > error_page can be called with the header already sent, you may
> > > want focus on reproducing and fixing this. Normally this is
> > > expected to result in the "header already sent" alerts produced
> > > by
> > > the check discussed above.
> > On the nginx side it is cause by this:
> > http://hg.nginx.org/nginx/rev/ad3f342f14ba046c
> > If writing to client returns an error and thus u->pipe-
> > > downstream_error is 1 and then reading from upstream fails and
> > > thus u-
> > > pipe->upstream_error is 1. ngx_http_upstream_finalize_request()
> > > is
> > then called with rc=NGX_HTTP_BAD_GATEWAY, where thanks to the above
> > commit the ngx_http_finalize_request() function is called also with
> > rc=NGX_HTTP_BAD_GATEWAY and thus error_page is called (if it is
> > configured for 502 status).
> > I think, that the ngx_http_finalize_request() function should be
> > called
> > with rc=NGX_ERROR in this case.
> I agree, the code in question looks incorrect. As long as header
> is already sent, it shouldn't call ngx_http_finalize_request()
> with NGX_HTTP_BAD_GATEWAY or any other special response code
> (except may be NGX_HTTP_REQUEST_TIME_OUT and
> NGX_HTTP_CLIENT_CLOSED_REQUEST, which are explicitly handled in
> The exact scenario described won't work though. If writing to the
> client returns an error, c->error will be set, and
> ngx_http_finalize_request() won't call error_page processing.
> I was able to trigger the alert using a HEAD request, which
> results in u->pipe->downstream_error being set in
> ngx_http_upstream_send_response() without an actual connection
> error (http://hg.nginx.org/nginx-tests/rev/b17f27fa9081). The
> same situation might also happen due to various other errors - for
> example, if writing to the client times out.
You are right, I checked it and in my scenario actually the client
> Please try the following patch:
The patch works for me (request is terminated without an internal
More information about the nginx-devel