<div dir="ltr">Hello!<div><br></div><div>Regarding the patch - it seems that if ngx_http_upstream_send_request() fails (the call to the function is in line 1342),</div><div>ngx_http_run_posted_requests() is still not called. Should a call to this function be added there as well?</div>
<div><br></div><div>Regards</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Aug 19, 2013 at 6:17 PM, Maxim Dounin <span dir="ltr"><<a href="mailto:mdounin@mdounin.ru" target="_blank">mdounin@mdounin.ru</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Hello!<br>
<div><div class="h5"><br>
On Mon, Aug 19, 2013 at 05:17:24PM +0300, Aviram Cohen wrote:<br>
<br>
> Hello!<br>
><br>
> I have encountered a potential bug in Nginx's upstream module -<br>
> When the upstream server is an SSL server, if an error occurs in<br>
> ngx_http_upstream_ssl_handshake() - the<br>
> function ngx_http_run_posted_requests() is never called.<br>
> This happens when initiating an SSL connection, the SSL module handles the<br>
> handshake, and not the upstream module (meaning ngx_http_upstream_handler()<br>
> is not involved in the process), and so if an error occurs, there's no one<br>
> who calls ngx_http_run_posted_requests().<br>
><br>
> The effect of this issue is the requests that "spawn" subrequests that use<br>
> the upstream error get stuck in case of an SSL error.<br>
> I can suggest two possible fixes (in the file ngx_http_upstream.c):<br>
>   - Add a call to ngx_http_run_posted_requests() to the end<br>
> of ngx_http_upstream_finalize_request().<br>
>   - Add a call to ngx_http_run_posted_requests() after calling<br>
> ngx_http_upstream_finalize_request() during error handling of the SSL<br>
> connection establishment.<br>
><br>
> Can anyone verify this issue and the suggested solution? If so, I'll be<br>
> more than happy to submit a patch.<br>
<br>
</div></div>Yes, it looks like there is a problem.<br>
<br>
The ngx_http_run_posted_requests() is usually called by an event<br>
handler, so adding a call to ngx_http_upstream_ssl_handshake()<br>
might be more appropriate.  Something like this should fix the<br>
problem:<br>
<br>
diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c<br>
--- a/src/http/ngx_http_upstream.c<br>
+++ b/src/http/ngx_http_upstream.c<br>
@@ -1343,8 +1343,11 @@ ngx_http_upstream_ssl_handshake(ngx_conn<br>
         return;<br>
     }<br>
<br>
+    c = r->connection;<br>
+<br>
     ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);<br>
<br>
+    ngx_http_run_posted_requests(c);<br>
 }<br>
<br>
 #endif<br>
<span class=""><font color="#888888"><br>
<br>
--<br>
Maxim Dounin<br>
<a href="http://nginx.org/en/donation.html" target="_blank">http://nginx.org/en/donation.html</a><br>
<br>
_______________________________________________<br>
nginx-devel mailing list<br>
<a href="mailto:nginx-devel@nginx.org">nginx-devel@nginx.org</a><br>
<a href="http://mailman.nginx.org/mailman/listinfo/nginx-devel" target="_blank">http://mailman.nginx.org/mailman/listinfo/nginx-devel</a><br>
</font></span></blockquote></div><br></div></div>