Upstream error handling issue

[Aviram Cohen]

Hello!

Regarding the patch - it seems that if ngx_http_upstream_send_request()
fails (the call to the function is in line 1342),
ngx_http_run_posted_requests() is still not called. Should a call to this
function be added there as well?

Regards


On Mon, Aug 19, 2013 at 6:17 PM, Maxim Dounin <mdounin at mdounin.ru> wrote:

> Hello!
>
> On Mon, Aug 19, 2013 at 05:17:24PM +0300, Aviram Cohen wrote:
>
> > Hello!
> >
> > I have encountered a potential bug in Nginx's upstream module -
> > When the upstream server is an SSL server, if an error occurs in
> > ngx_http_upstream_ssl_handshake() - the
> > function ngx_http_run_posted_requests() is never called.
> > This happens when initiating an SSL connection, the SSL module handles
> the
> > handshake, and not the upstream module (meaning
> ngx_http_upstream_handler()
> > is not involved in the process), and so if an error occurs, there's no
> one
> > who calls ngx_http_run_posted_requests().
> >
> > The effect of this issue is the requests that "spawn" subrequests that
> use
> > the upstream error get stuck in case of an SSL error.
> > I can suggest two possible fixes (in the file ngx_http_upstream.c):
> >   - Add a call to ngx_http_run_posted_requests() to the end
> > of ngx_http_upstream_finalize_request().
> >   - Add a call to ngx_http_run_posted_requests() after calling
> > ngx_http_upstream_finalize_request() during error handling of the SSL
> > connection establishment.
> >
> > Can anyone verify this issue and the suggested solution? If so, I'll be
> > more than happy to submit a patch.
>
> Yes, it looks like there is a problem.
>
> The ngx_http_run_posted_requests() is usually called by an event
> handler, so adding a call to ngx_http_upstream_ssl_handshake()
> might be more appropriate.  Something like this should fix the
> problem:
>
> diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c
> +++ b/src/http/ngx_http_upstream.c
> @@ -1343,8 +1343,11 @@ ngx_http_upstream_ssl_handshake(ngx_conn
>          return;
>      }
>
> +    c = r->connection;
> +
>      ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);
>
> +    ngx_http_run_posted_requests(c);
>  }
>
>  #endif
>
>
> --
> Maxim Dounin
> http://nginx.org/en/donation.html
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20130819/ca9e0aa9/attachment.html>