Hi Olivier,
On Wed, Apr 11, 2018 at 05:29:15PM +0200, Olivier Houchard wrote:
> From 7c9f06727cf60acf873353ac71283ff9c562aeee Mon Sep 17 00:00:00 2001
> From: Olivier Houchard <[email protected]>
> Date: Wed, 11 Apr 2018 17:23:17 +0200
> Subject: [PATCH] BUG/MINOR: connection: Setup a mux when in proxy mode.
>
> We were allocating a new connection when in proxy mode, but did not provide
> it a mux, thus crashing later.
>
> This should be backported to 1.8.
> ---
> src/proto_http.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/src/proto_http.c b/src/proto_http.c
> index 80e001d69..817692c48 100644
> --- a/src/proto_http.c
> +++ b/src/proto_http.c
> @@ -62,6 +62,7 @@
> #include <proto/h1.h>
> #include <proto/log.h>
> #include <proto/hdr_idx.h>
> +#include <proto/mux_pt.h>
> #include <proto/pattern.h>
> #include <proto/proto_tcp.h>
> #include <proto/proto_http.h>
> @@ -3718,6 +3719,8 @@ int http_process_request(struct stream *s, struct
> channel *req, int an_bit)
>
> return 0;
> }
> + /* XXX: We probably need a better mux */
> + conn_install_mux(conn, &mux_pt_ops, objt_cs(s->si[1].end));
>
> path = http_get_path(txn);
> url2sa(req->buf->p + msg->sl.rq.u,
While I can understand how missing this can have caused trouble, I'm not
sure it's the best solution, and I'm a bit worried about having various
code places choose a mux and install it. I suspect we're "simply" missing
a test in the backend code to cover the case where the connection already
exists but a mux was not yet installed (a situation that didn't exist
prior to muxes which is why we didn't notice this). I think that this
happens in connect_server() around line 1177 (return SF_ERR_INTERNAL)
when conn_xprt_ready() indicates the transport was not yet initialized.
It's likely that the error unrolling fails to consider the case where
the mux wasn't initialized, leading to the crash Praveen experienced.
If this is right, it would mean two fixes :
- one in the error unrolling path to ensure we don't destroy a non
allocated mux ;
- one above the return SF_ERR_INTERNAL above in connect_server() to
handle the case where the connection already exists but not the mux.
What do you think ?
Willy
