BUG/MEDIUM: stream: Properly handle destructive client connection upgrades

When the protocol is changed for a client connection at the stream level
(from TCP to H1/H2), there are two cases. The stream may be reused or
not. The first case, when the stream is reused is working. The second one is
buggy since the conn-stream refactoring and leads to a crash.

In this case, the new mux don't reuse the stream. It must be silently
aborted. However, its front stream connector is still referencing the
connection. So it must be detached. But it must be performed in two stages,
to be sure to not loose the context for the upgrade and to be able to
rollback on error. So now, before the upgrade, we prepare to detach the
stconn and it is finally detached if the upgrade succeeds. There is a trick
here. Because we pretend the stconn is detached but its state is preserved.

This patch must be backported to 2.6.

diff --git a/include/haproxy/stconn.h b/include/haproxy/stconn.h
index 9c7be5032b2ba89805479042a279a9a9d4f944af..ff0de4354dbdfd741f95f41aef4f5c79de7de724 100644 (file)
--- a/include/haproxy/stconn.h
+++ b/include/haproxy/stconn.h
@@ -51,6 +51,10 @@ int sc_reset_endp(struct stconn *sc);
 
 struct appctx *sc_applet_create(struct stconn *sc, struct applet *app);
 
+void sc_conn_prepare_endp_upgrade(struct stconn *sc);
+void sc_conn_abort_endp_upgrade(struct stconn *sc);
+void sc_conn_commit_endp_upgrade(struct stconn *sc);
+
 /* The se_fl_*() set of functions manipulate the stream endpoint flags from
  * the stream endpoint itself. The sc_ep_*() set of functions manipulate the
  * stream endpoint flags from the the stream connector (ex. stconn).

diff --git a/src/stconn.c b/src/stconn.c
index 5d4ed3ed47c5c523312d290770a1ac5c61a0e025..02a8dc452d0ce20ab6fea79e4acb83ad7e1e351b 100644 (file)
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1943,3 +1943,35 @@ static int sc_applet_process(struct stconn *sc)
                appctx_wakeup(__sc_appctx(sc));
        return 0;
 }
+
+
+/* Prepares an endpoint upgrade. We don't now at this stage if the upgrade will
+ * succeed or not and if the stconn will be reused by the new endpoint. Thus,
+ * for now, only pretend the stconn is detached.
+ */
+void sc_conn_prepare_endp_upgrade(struct stconn *sc)
+{
+       BUG_ON(!sc_conn(sc) || !sc->app);
+       sc_ep_clr(sc, SE_FL_T_MUX);
+       sc_ep_set(sc, SE_FL_DETACHED);
+}
+
+/* Endpoint upgrade failed. Retore the stconn state. */
+void sc_conn_abort_endp_upgrade(struct stconn *sc)
+{
+       sc_ep_set(sc, SE_FL_T_MUX);
+       sc_ep_clr(sc, SE_FL_DETACHED);
+}
+
+/* Commit the endpoint upgrade. If stconn is attached, it means the new endpoint
+ * use it. So we do nothing. Otherwise, the stconn will be destroy with the
+ * overlying stream. So, it means we must commit the detach.
+*/
+void sc_conn_commit_endp_upgrade(struct stconn *sc)
+{
+       if (!sc_ep_test(sc, SE_FL_DETACHED))
+               return;
+       sc_detach_endp(&sc);
+       /* Because it was already set as detached, the sedesc must be preserved */
+       BUG_ON(!sc->sedesc);
+}

diff --git a/src/stream.c b/src/stream.c
index f3f776db624b50469d10e056f4f5522b617839cc..2f698482f3106d9b08511e8ee5a8674e66a9c589 100644 (file)
--- a/src/stream.c
+++ b/src/stream.c
@@ -1485,10 +1485,15 @@ int stream_set_http_mode(struct stream *s, const struct mux_proto_list *mux_prot
 
                if (conn->mux->flags & MX_FL_NO_UPG)
                        return 0;
+
+               sc_conn_prepare_endp_upgrade(sc);
                if (conn_upgrade_mux_fe(conn, sc, &s->req.buf,
                                        (mux_proto ? mux_proto->token : ist("")),
-                                       PROTO_MODE_HTTP)  == -1)
+                                       PROTO_MODE_HTTP)  == -1) {
+                       sc_conn_abort_endp_upgrade(sc);
                        return 0;
+               }
+               sc_conn_commit_endp_upgrade(sc);
 
                s->req.flags &= ~(CF_READ_PARTIAL|CF_AUTO_CONNECT);
                s->req.total = 0;