Welcome! Log In Create A New Profile

Advanced

Re: Remove unnecessary check in ngx_http_stub_status_handler()

Maxim Dounin
January 19, 2021 12:30PM
Hello!

On Tue, Jan 05, 2021 at 01:24:04PM +0000, Chris Newton wrote:

> I was desk checking return codes generated in handlers following calls to
> ngx_http_send_header(), and noticed what appears to be an unnecessary test
> in ngx_http_stub_status_handler() -- or rather, I think the test should
> always evaluate as true, and if somehow it isn't odd things could occur -
> at least an additional ALERT message would be logged, as well as some
> unnecessary work performed.
>
> As such, I'd like to propose the following change:
>
> *--- a/src/http/modules/ngx_http_stub_status_module.c*
> *+++ b/src/http/modules/ngx_http_stub_status_module.c*
> @@ -106,11 +106,7 @@ ngx_http_stub_status_handler(ngx_http_request_t *r)
> if (r->method == NGX_HTTP_HEAD) {
> r->headers_out.status = NGX_HTTP_OK;
>
> - rc = ngx_http_send_header(r);
> -
> - if (rc == NGX_ERROR || rc > NGX_OK || r->header_only) {
> - return rc;
> - }
> + return ngx_http_send_header(r);
> }
>
> size = sizeof("Active connections: \n") + NGX_ATOMIC_T_LEN
>
>
> On a successful call to ngx_http_send_header() I believe that
> r->header_only will be set true and otherwise I'd expect one of those error
> checks to evaluate true, so unconditionally returning the value from
> ngx_http_send_header() seems 'cleaner'.
>
> If the test were to somehow fail, then processing would fall through and
> try the ngx_http_send_header() call again (resulting in the ALERT message),
> as well as performing other additional work that should be unnecessary when
> making a HEAD request
>
> That test seems to be SOP after calling ngx_http_send_header(), but it
> seems inappropriate when that function is called within an "r->method ==
> NGX_HTTP_HEAD" block.

After looking at this I tend to think that this optimization is
simply wrong, and, for example, image_filter or xslt filter make
this obvious. I've committed a patch which completely removes this
optimization from the stub_status, as well as similar optimization
in ngx_http_send_response() (used by "return" and "empty_gif"):

https://hg.nginx.org/nginx/rev/43a0a9e988be

Thanks for reporting this.

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

Remove unnecessary check in ngx_http_stub_status_handler()

Chris Newton 146 January 05, 2021 08:26AM

Re: Remove unnecessary check in ngx_http_stub_status_handler()

Chris Newton 20 January 14, 2021 07:44AM

RE: Re: Remove unnecessary check in ngx_http_stub_status_handler()

Anonymous User 28 January 19, 2021 12:54PM

Re: Remove unnecessary check in ngx_http_stub_status_handler()

Sergey Kandaurov 27 January 14, 2021 11:30AM

Re: Remove unnecessary check in ngx_http_stub_status_handler()

Maxim Dounin 18 January 19, 2021 12:30PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 86
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready