Remove unnecessary check in ngx_http_stub_status_handler()

[Chris Newton]

any thoughts on this?

TIA

Chris

On Tue, Jan 5, 2021 at 1:24 PM 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'.
>
> 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.
>
> TIA
>
> Chris
>
>

-- 

*Chris Newton*
Director Of Engineering | Content Delivery Architecture M: 805.444.0573 |
cnewton at netflix.com
111 Albright Way | Los Gatos, CA 95032
<https://maps.google.com/?q=111+Albright+Way%C2%A0%7C++Los+Gatos,+CA+95032&entry=gmail&source=g>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20210114/d161bbce/attachment.htm>