From: Christopher Faulet <cfaulet@haproxy.com>
Date: Thu, 4 May 2023 13:49:12 +0000 (+0200)
Subject: BUG/MEDIUM: mux-h2: Properly handle end of request to expect data from server
X-Git-Tag: v2.8-dev10~59
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4403cdf65380756903c412472ea4186d78d7a91f;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h2: Properly handle end of request to expect data from server

The commit 2722c04b ("MEDIUM: mux-h2: Don't expect data from server as long
as request is unfinished") introduced a regression in the H2 multiplexer.
The end of the request is not systematically handled to state a H2 stream on
client side now expexts data from the server.

Indeed, while the client is uploading its request, the H2 stream warns it
does not expect data from the server. This way, no server timeout is applied
at this stage. When end of the request is detected, the H2 stream must state
it now expects the server response. This enables the server timeout.

However, it was only performed at one place while the end of the request can
be handled at different places. First, during a zero-copy in
h2_rcv_buf(). Then, when the SC is created with the full request. Because of
this bug, it is possible to totally disable the server timeout for H2
streams.

In h2_rcv_buf(), we now rely on h2s flags to detect the end of the request,
but only when the rxbuf was emptied.

It is a 2.8-specific bug. No backport needed.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 4b8592a16e..44bb51fb3c 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1560,7 +1560,11 @@ static struct h2s *h2c_frt_stream_new(struct h2c *h2c, int id, struct buffer *in
 	h2s->sd->se   = h2s;
 	h2s->sd->conn = h2c->conn;
 	se_fl_set(h2s->sd, SE_FL_T_MUX | SE_FL_ORPHAN | SE_FL_NOT_FIRST);
-	se_expect_no_data(h2s->sd);
+	/* The request is not finished, don't expect data from the opposite side
+	 * yet
+	 */
+	if (!(h2c->dff & (H2_F_HEADERS_END_STREAM| H2_F_DATA_END_STREAM)))
+		se_expect_no_data(h2s->sd);
 
 	/* FIXME wrong analogy between ext-connect and websocket, this need to
 	 * be refine.
@@ -6430,7 +6434,6 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in
 		htx_to_buf(h2s_htx, &h2s->rxbuf);
 		goto end;
 	}
-
 	ret = h2s_htx->data;
 	buf_htx = htx_from_buf(buf);
 
@@ -6452,13 +6455,6 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in
 	}
 	else if (htx_is_empty(h2s_htx)) {
 		buf_htx->flags |= (h2s_htx->flags & HTX_FL_EOM);
-
-		if (!(h2c->flags & H2_CF_IS_BACK) && (buf_htx->flags & HTX_FL_EOM)) {
-			/* If request EOM is reported to the upper layer, it means the
-			 * H2S now expects data from the opposite side.
-			 */
-			se_expect_data(h2s->sd);
-		}
 	}
 
 	buf_htx->extra = (h2s_htx->extra ? (h2s_htx->data + h2s_htx->extra) : 0);
@@ -6470,6 +6466,13 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in
 	if (b_data(&h2s->rxbuf))
 		se_fl_set(h2s->sd, SE_FL_RCV_MORE | SE_FL_WANT_ROOM);
 	else {
+		if (!(h2c->flags & H2_CF_IS_BACK) && (h2s->flags & H2_SF_ES_RCVD)) {
+			/* If request ES is reported to the upper layer, it means the
+			 * H2S now expects data from the opposite side.
+			 */
+			se_expect_data(h2s->sd);
+		}
+
 		se_fl_clr(h2s->sd, SE_FL_RCV_MORE | SE_FL_WANT_ROOM);
 		if (h2s->flags & H2_SF_ES_RCVD) {
 			se_fl_set(h2s->sd, SE_FL_EOI);