From: Willy Tarreau <w@1wt.eu>
Date: Wed, 9 Oct 2024 18:00:07 +0000 (+0200)
Subject: MINOR: mux-h2: clear up H2_CF_DEM_DFULL and H2_CF_DEM_SHORT_READ ambiguity
X-Git-Tag: v3.1-dev10~82
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6279cbc9e9a8d9e65eb222d6c37b78766298cad6;p=thirdparty%2Fhaproxy.git

MINOR: mux-h2: clear up H2_CF_DEM_DFULL and H2_CF_DEM_SHORT_READ ambiguity

Since commit 485da0b05 ("BUG/MEDIUM: mux_h2: Handle others remaining
read0 cases on partial frames"), H2_CF_DEM_SHORT_READ is set when there
is no blocking flags. However, it checks H2_CF_DEM_BLOCK_ANY which does
not include H2_CF_DEM_DFULL. This results in many cases where both
H2_CF_DEM_DFULL and H2_CF_DEM_SHORT_READ are set together, which makes
no sense, since one says the demux buffer is full while the other one
says an incomplete read was done. This doesn't permit to properly
decide whether to restart reading or processing.

Let's make sure to clear DFULL in h2_process_demux() whenever we
consume incoming data from the dbuf, and check for DFULL before
setting SHORT_READ.

This could probably be considered as a bug fix but it's hard to say if
it has any impact on the current code, probably at worst it might cause
a few useless wakeups, so until there's any proof that it needs to be
backported, better not do it.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 0167dd9e9b..972750b431 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -3114,8 +3114,10 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
 			}
 
 			if (error == 0) {
-				/* Demux not blocked because of the stream, it is an incomplete frame */
-				if (!(h2c->flags &H2_CF_DEM_BLOCK_ANY))
+				/* Demux not blocked because of the stream, it is an incomplete frame,
+				 * or the rxbuf is not empty (e.g. for trailers).
+				 */
+				if (!(h2c->flags & (H2_CF_DEM_BLOCK_ANY | H2_CF_DEM_DFULL)))
 					h2c->flags |= H2_CF_DEM_SHORT_READ;
 				goto out; // missing data
 			}
@@ -3175,7 +3177,7 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
 
 	if (error == 0) {
 		/* No error but missing data for demuxing, it is an incomplete frame */
-		if (!(h2c->flags &H2_CF_DEM_BLOCK_ANY))
+		if (!(h2c->flags & (H2_CF_DEM_BLOCK_ANY | H2_CF_DEM_DFULL)))
 			h2c->flags |= H2_CF_DEM_SHORT_READ;
 		goto out;
 	}
@@ -3345,8 +3347,10 @@ static struct h2s *h2c_bck_handle_headers(struct h2c *h2c, struct h2s *h2s)
 
 	if (error <= 0) {
 		if (error == 0) {
-			/* Demux not blocked because of the stream, it is an incomplete frame */
-			if (!(h2c->flags &H2_CF_DEM_BLOCK_ANY))
+			/* Demux not blocked because of the stream, it is an incomplete frame,
+			 * or the rxbuf is not empty (e.g. for trailers).
+			 */
+			if (!(h2c->flags & (H2_CF_DEM_BLOCK_ANY | H2_CF_DEM_DFULL)))
 				h2c->flags |= H2_CF_DEM_SHORT_READ;
 			goto fail; // missing data
 		}
@@ -3886,6 +3890,10 @@ static void h2_process_demux(struct h2c *h2c)
 	while (1) {
 		int ret = 0;
 
+		/* Make sure to clear DFULL if contents were deleted */
+		if (!b_full(&h2c->dbuf))
+			h2c->flags &= ~H2_CF_DEM_DFULL;
+
 		if (!b_data(&h2c->dbuf)) {
 			TRACE_DEVEL("no more Rx data", H2_EV_RX_FRAME, h2c->conn);
 			h2c->flags |= H2_CF_DEM_SHORT_READ;
@@ -4004,6 +4012,10 @@ static void h2_process_demux(struct h2c *h2c)
 				h2c->idle_start = now_ms;
 		}
 
+		/* Make sure to clear DFULL if contents were deleted */
+		if (!b_full(&h2c->dbuf))
+			h2c->flags &= ~H2_CF_DEM_DFULL;
+
 		/* Only H2_CS_FRAME_P, H2_CS_FRAME_A and H2_CS_FRAME_E here.
 		 * H2_CS_FRAME_P indicates an incomplete previous operation
 		 * (most often the first attempt) and requires some validity
@@ -4196,6 +4208,10 @@ static void h2_process_demux(struct h2c *h2c)
 			h2c->flags |= H2_CF_END_REACHED;
 	}
 
+	/* Make sure to clear DFULL if contents were deleted */
+	if (!b_full(&h2c->dbuf))
+		h2c->flags &= ~H2_CF_DEM_DFULL;
+
 	if (h2s && h2s_sc(h2s) &&
 	    (b_data(&h2s->rxbuf) ||
 	     h2c_read0_pending(h2c) ||
@@ -4659,9 +4675,6 @@ static int h2_process(struct h2c *h2c)
 
 		if (h2c->st0 >= H2_CS_ERROR || (h2c->flags & H2_CF_ERROR))
 			b_reset(&h2c->dbuf);
-
-		if (!b_full(&h2c->dbuf))
-			h2c->flags &= ~H2_CF_DEM_DFULL;
 	}
 	h2_send(h2c);