From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 15 Dec 2022 08:22:35 +0000 (+0100)
Subject: BUG/MEDIUM: mux-h1: Don't release H1 stream upgraded from TCP on error
X-Git-Tag: v2.8-dev1~124
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=da93802ffc5c38b470b95d250a9c0b496349ca64;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h1: Don't release H1 stream upgraded from TCP on error

When an error occurred during the request parsing, the H1 multiplexer is
responsible to sent a response to the client and to release the H1 stream
and the H1 connection. In HTTP mode, it is not an issue because at this
stage the H1 connection is in embryonic state. Thus it can be released
immediately.

However, it is a problem if the connection was first upgraded from a TCP
connection. In this case, a stream-connector is attached. The H1 stream is
not orphan. Thus it must not be released at this stage. It must be detached
first. Otherwise a BUG_ON() is triggered in h1s_destroy().

So now, the H1S is destroyed on early errors but only if the H1C is in
embryonic state.

This patch may be related to #1966. It must be backported to 2.7.
---

diff --git a/reg-tests/connection/tcp_to_http_upgrade.vtc b/reg-tests/connection/tcp_to_http_upgrade.vtc
index 093ba48e13..48ebebaef7 100644
--- a/reg-tests/connection/tcp_to_http_upgrade.vtc
+++ b/reg-tests/connection/tcp_to_http_upgrade.vtc
@@ -155,6 +155,13 @@ client c_err3 -connect ${h1_err3h1_sock} {
     expect_close
 } -run
 
+# TCP > HTTP upgrade with a parsing error
+client c_err4 -connect ${h1_fe2h1_sock} {
+    send "GET / BAD-VERSION\r\n\r\n"
+    rxresp
+    expect resp.status == 400
+} -run
+
 # To be sure no other request was received by the server
 client c_end -connect ${s1_sock} {
     txreq
diff --git a/src/mux_h1.c b/src/mux_h1.c
index a8421b76ca..c511cfede7 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -2629,7 +2629,8 @@ static int h1_send_error(struct h1c *h1c)
 		}
 	}
 
-	if (h1c->h1s) {
+	if (h1c->state == H1_CS_EMBRYONIC) {
+		BUG_ON(h1c->h1s == NULL || h1s_sc(h1c->h1s) == NULL);
 		TRACE_DEVEL("Abort embryonic H1S", H1_EV_H1C_ERR, h1c->conn, h1c->h1s);
 		h1s_destroy(h1c->h1s);
 	}