From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 8 Feb 2021 14:56:36 +0000 (+0100)
Subject: BUG/MINOR: mux-h1: Fix data skipping for bodyless responses
X-Git-Tag: v2.4-dev8~88
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=0d7e634631dd96378256d5783f6876169558b7f2;p=thirdparty%2Fhaproxy.git

BUG/MINOR: mux-h1: Fix data skipping for bodyless responses

When payload is received for a bodyless response, for instance a response to
a HEAD request, it is silently skipped. Unfortunately, when this happens,
the end of the message is not properly handled. The response remains in the
MSG_DATA state (or MSG_TRAILERS if the message is chunked). In addition,
when a zero-copy is possible, the data are not removed from the channel
buffer and the H1 connection is killed because an error is then triggered.

To fix the bug, the zero-copy is disabled for bodyless responses. It is not
a problem because there is no copy at all. And the last block (DATA or EOT)
is now properly handled.

This bug was introduced by the commit e5596bf53 ("MEDIUM: mux-h1: Don't emit
any payload for bodyless responses").

This fix is specific for 2.4. No backport needed.
---

diff --git a/src/mux_h1.c b/src/mux_h1.c
index b5772519bd..ddafda6ac7 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1786,16 +1786,12 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 	 */
 	if (!b_data(&h1c->obuf)) {
 		if ((h1m->state == H1_MSG_DATA || h1m->state == H1_MSG_TUNNEL) &&
+		    (!(h1m->flags & H1_MF_RESP) || !(h1s->flags & H1S_F_BODYLESS_RESP)) &&
 		    htx_nbblks(chn_htx) == 1 &&
 		    htx_get_blk_type(blk) == HTX_BLK_DATA &&
 		    htx_get_blk_value(chn_htx, blk).len == count) {
 			void *old_area;
 
-			if ((h1m->flags & H1_MF_RESP) && (h1s->flags & H1S_F_BODYLESS_RESP)) {
-				TRACE_PROTO("Skip data for bodyless response", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, chn_htx);
-				goto skip_zero_copy;
-			}
-
 			TRACE_PROTO("sending message data (zero-copy)", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, chn_htx, (size_t[]){count});
 			if (h1m->state == H1_MSG_DATA && chn_htx->flags & HTX_FL_EOM) {
 				TRACE_DEVEL("last message block", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s);
@@ -1834,7 +1830,6 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 				TRACE_PROTO((!(h1m->flags & H1_MF_RESP) ? "H1 request tunneled data xferred" : "H1 response tunneled data xferred"),
 					    H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, 0, (size_t[]){count});
 
-		  skip_zero_copy:
 			total += count;
 			if (last_data) {
 				h1m->state = H1_MSG_DONE;
@@ -2103,6 +2098,9 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 			case H1_MSG_DATA:
 			case H1_MSG_TUNNEL:
 				if (type == HTX_BLK_EOT || type == HTX_BLK_TLR) {
+					if ((h1m->flags & H1_MF_RESP) && (h1s->flags & H1S_F_BODYLESS_RESP))
+						goto trailers;
+
 					/* If the message is not chunked, never
 					 * add the last chunk. */
 					if ((h1m->flags & H1_MF_CHNK) && !chunk_memcat(&tmp, "0\r\n", 3))
@@ -2113,11 +2111,6 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 				else if (type != HTX_BLK_DATA)
 					goto error;
 
-				if (h1m->state == H1_MSG_DATA && (h1m->flags & H1_MF_RESP) && (h1s->flags & H1S_F_BODYLESS_RESP)) {
-					TRACE_PROTO("Skip data for bodyless response", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, chn_htx);
-					break;
-				}
-
 				TRACE_PROTO("sending message data", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, chn_htx, (size_t[]){sz});
 
 				/* It is the last block of this message. After this one,
@@ -2133,6 +2126,11 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 					last_data = 0;
 				}
 
+				if (h1m->state == H1_MSG_DATA && (h1m->flags & H1_MF_RESP) && (h1s->flags & H1S_F_BODYLESS_RESP)) {
+					TRACE_PROTO("Skip data for bodyless response", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, chn_htx);
+					goto skip_data;
+				}
+
 				chklen = 0;
 				if (h1m->flags & H1_MF_CHNK) {
 					chklen = b_room(&tmp);
@@ -2169,6 +2167,8 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 				else
 					TRACE_PROTO((!(h1m->flags & H1_MF_RESP) ? "H1 request tunneled data xferred" : "H1 response tunneled data xferred"),
 						    H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, 0, (size_t[]){v.len});
+
+			  skip_data:
 				if (last_data)
 					goto done;
 				break;
@@ -2186,6 +2186,8 @@ static size_t h1_process_output(struct h1c *h1c, struct buffer *buf, size_t coun
 
 				if ((h1m->flags & H1_MF_RESP) && (h1s->flags & H1S_F_BODYLESS_RESP)) {
 					TRACE_PROTO("Skip trailers for bodyless response", H1_EV_TX_DATA|H1_EV_TX_BODY, h1c->conn, h1s, chn_htx);
+					if (type == HTX_BLK_EOT)
+						goto done;
 					break;
 				}