Remove unnecessary check in ngx_http_stub_status_handler()

Sergey Kandaurov pluknet at nginx.com
Thu Jan 14 16:28:44 UTC 2021


> On 5 Jan 2021, at 16:24, Chris Newton <cnewton at netflix.com> 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'.
> 

Your analysis looks correct to me.
Noteworthy, empty_gif have such change in fc73de3e8df0, with similar reason.

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

-- 
Sergey Kandaurov



More information about the nginx-devel mailing list