<div dir="ltr">any thoughts on this?<div><br>TIA</div><div><br></div><div>Chris</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 5, 2021 at 1:24 PM Chris Newton <<a href="mailto:cnewton@netflix.com">cnewton@netflix.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div><br></div>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. <div>





<div><br></div><div>As such, I'd like to propose the following change:</div><div><br></div><div>





<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><b>--- a/src/http/modules/ngx_http_stub_status_module.c</b></span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><b>+++ b/src/http/modules/ngx_http_stub_status_module.c</b></span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures;color:rgb(46,174,187)">@@ -106,11 +106,7 @@</span><span style="font-variant-ligatures:no-common-ligatures"> ngx_http_stub_status_handler(ngx_http_request_t *r)</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>     </span>if (r->method == NGX_HTTP_HEAD) {</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>         </span>r->headers_out.status = NGX_HTTP_OK;</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0);min-height:13px"><span style="font-variant-ligatures:no-common-ligatures"><span> </span></span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(180,36,25)"><span style="font-variant-ligatures:no-common-ligatures">-<span>        </span>rc = ngx_http_send_header(r);</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(180,36,25)"><span style="font-variant-ligatures:no-common-ligatures">-</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(180,36,25)"><span style="font-variant-ligatures:no-common-ligatures">-<span>        </span>if (rc == NGX_ERROR || rc > NGX_OK || r->header_only) {</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(180,36,25)"><span style="font-variant-ligatures:no-common-ligatures">-<span>            </span>return rc;</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(180,36,25)"><span style="font-variant-ligatures:no-common-ligatures">-<span>        </span>}</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(47,180,29)"><span style="font-variant-ligatures:no-common-ligatures">+<span>        </span>return ngx_http_send_header(r);</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures"><span>     </span>}</span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0);min-height:13px"><span style="font-variant-ligatures:no-common-ligatures"><span> </span></span></p>
<p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-variant-ligatures:no-common-ligatures">     </span><span style="font-variant-ligatures:no-common-ligatures">size = sizeof("Active connections:</span><span style="font-variant-ligatures:no-common-ligatures">  </span><span style="font-variant-ligatures:no-common-ligatures">\n") + NGX_ATOMIC_T_LEN</span></p><p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-family:Arial,Helvetica,sans-serif;font-size:small;color:rgb(34,34,34)"><br></span></p><p style="margin:0px;font-variant-numeric:normal;font-variant-east-asian:normal;font-stretch:normal;font-size:11px;line-height:normal;font-family:Menlo;color:rgb(0,0,0)"><span style="font-family:Arial,Helvetica,sans-serif;font-size:small;color:rgb(34,34,34)">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'.</span><br></p></div><div><br></div><div>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 </div><div><br></div></div><div>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.</div><div><br></div>TIA<div><br></div><div>Chris<br><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><br></div></div></div></div></div></div></div>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div dir="ltr"><br></div><div dir="ltr"><b style="color:rgb(0,0,0);font-family:Arial;font-size:x-small;line-height:1.38;white-space:pre-wrap">Chris Newton</b></div><div dir="ltr"><div dir="ltr"><span style="color:rgb(102,102,102);font-size:x-small;line-height:1.38"><font face="Arial"><span style="white-space:pre-wrap">Director Of Engineering | Content Delivery Architecture
</span></font></span><span style="color:rgb(102,102,102);font-size:x-small;line-height:1.38">M: 805.444.0573</span><span style="color:rgb(102,102,102);font-size:x-small;line-height:1.38;font-family:tahoma,sans-serif;white-space:pre-wrap"> </span><span style="color:rgb(102,102,102);font-family:tahoma,sans-serif;font-size:x-small;line-height:13.8px;white-space:pre-wrap">| </span><a href="mailto:cnewton@netflix.com" style="color:rgb(17,85,204);font-size:x-small;line-height:1.38;font-family:tahoma,sans-serif;white-space:pre-wrap" target="_blank">cnewton@netflix.com</a><br style="font-size:12.8px"><span style="color:rgb(102,102,102);font-family:Arial;font-size:x-small;line-height:1.38;white-space:pre-wrap"><a href="https://maps.google.com/?q=111+Albright+Way%C2%A0%7C++Los+Gatos,+CA+95032&entry=gmail&source=g" style="color:rgb(17,85,204)" target="_blank">111 Albright Way |  Los Gatos, CA 95032</a></span><br></div><div dir="ltr"><br></div><div dir="ltr"><img src="https://lh6.googleusercontent.com/KDmG8yFfKZ_1dwToSfKjPTq9jRylYSYv_Um56GHw4OW0J5iaaZthlmRRU664lyRgOSYnYOLei8XS0NQmaewP-fU115x5CTb3omHWEkBCii4gLmLrFY92wpAS6-il-Gs32FUJTJ9y" width="73" height="44" style="color: rgb(136, 136, 136); font-family: Arial; font-size: 10.6667px; white-space: pre-wrap; border: none;"></div></div></div></div></div></div>