<div dir="ltr">Apologies, I referenced a git commit hash in the first sentence, the corresponding mercurial commit is 5746:35990c69.</div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Mar 7, 2016 at 1:16 PM, Justin Li <span dir="ltr"><<a href="mailto:jli.justinli@gmail.com" target="_blank">jli.justinli@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"># HG changeset patch<br>
# User Justin Li <<a href="mailto:jli.justinli@gmail.com">jli.justinli@gmail.com</a>><br>
# Date 1457369412 18000<br>
#      Mon Mar 07 11:50:12 2016 -0500<br>
# Node ID e84a844f3c2634488285fb9677d3254a032440a2<br>
# Parent  c5f81dcf97a79576138917501c9a6cc6f53ee930<br>
Upstream: avoid closing connection when no client body needed<br>
<br>
Despite 4a75e1a6, it was previously possible for the connection to be closed in<br>
cases where a response body from the upstream should not be sent to the client.<br>
This is due to the conditional in ngx_http_upstream_process_request that may<br>
call ngx_http_upstream_finalize_request in certain cases.<br>
<br>
Example configuration:<br>
<br>
proxy_cache foo;<br>
proxy_cache_bypass 1;<br>
proxy_no_cache 1;<br>
<br>
If a client sends If-None-Match, and the upstream server returns 200 with a<br>
matching ETag, no body should be returned to the client. At the start of<br>
ngx_http_upstream_send_response proxy_no_cache is not yet tested, thus cacheable<br>
is still 1 and downstream_error is set.<br>
<br>
However, by the time the downstream_error check is done in process_request,<br>
proxy_no_cache has been tested and cacheable is set to 0. The client connection<br>
is then closed, regardless of keepalive.<br>
<br>
The fix is to avoid using the p->downstream_error flag, and instead repurpose<br>
p->downstream_done to indicate the event pipe should be drained. Additionally,<br>
a check is added for this flag in ngx_http_upstream_check_broken_connection to<br>
prevent a 499 if the client validly closes the connection after receiving the<br>
headers.<br>
<br>
diff -r c5f81dcf97a7 -r e84a844f3c26 src/event/ngx_event_pipe.c<br>
--- a/src/event/ngx_event_pipe.c        Thu Mar 03 21:14:19 2016 +0300<br>
+++ b/src/event/ngx_event_pipe.c        Mon Mar 07 11:50:12 2016 -0500<br>
@@ -502,7 +502,7 @@<br>
     flushed = 0;<br>
<br>
     for ( ;; ) {<br>
-        if (p->downstream_error) {<br>
+        if (p->downstream_error || p->downstream_done) {<br>
             return ngx_event_pipe_drain_chains(p);<br>
         }<br>
<br>
diff -r c5f81dcf97a7 -r e84a844f3c26 src/http/ngx_http_upstream.c<br>
--- a/src/http/ngx_http_upstream.c      Thu Mar 03 21:14:19 2016 +0300<br>
+++ b/src/http/ngx_http_upstream.c      Mon Mar 07 11:50:12 2016 -0500<br>
@@ -1172,6 +1172,10 @@<br>
     }<br>
 #endif<br>
<br>
+    if (u->pipe->downstream_done) {<br>
+        return;<br>
+    }<br>
+<br>
 #if (NGX_HAVE_KQUEUE)<br>
<br>
     if (ngx_event_flags & NGX_USE_KQUEUE_EVENT) {<br>
@@ -2697,7 +2701,7 @@<br>
             return;<br>
         }<br>
<br>
-        u->pipe->downstream_error = 1;<br>
+        u->pipe->downstream_done = 1;<br>
     }<br>
<br>
     if (r->request_body && r->request_body->temp_file) {<br>
</blockquote></div><br></div>