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 9c7be50..ff0de43 100644
--- a/include/haproxy/stconn.h
+++ b/include/haproxy/stconn.h
@@ -51,6 +51,10 @@
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 5d4ed3e..02a8dc4 100644
--- a/src/stconn.c
+++ b/src/stconn.c
@@ -1943,3 +1943,35 @@
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 f3f776d..2f69848 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1485,10 +1485,15 @@
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;