From: Christopher Faulet <cfaulet@haproxy.com>
Date: Mon, 10 Oct 2022 16:05:25 +0000 (+0200)
Subject: BUG/MEDIUM: mux-h1: Handle abort with an incomplete message during parsing
X-Git-Tag: v2.7-dev8~30
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=104985610dc4c673f66e4c2eddf9633bc1c7149c;p=thirdparty%2Fhaproxy.git

BUG/MEDIUM: mux-h1: Handle abort with an incomplete message during parsing

In h1_process_demux(), aborts for incomplete messages were not properly
handled. It was not an issue because the abort was detected later in
h1_process(). But it will be an issue to perform the aborts refoctoring.

First, when a read0 was detected, the SE_FL_EOI flag was set for messages in
DONE or TUNNEL state or for messages without known length (so responses in
close mode). The last statement is not accurate. The message must also be in
DATA state. Otherwise, SE_FL_EOI flag may be set on incomplete message.

Then, an error was reported, via SE_FL_ERROR flag, only when an incomplete
message was detected on the payload parsing. It must also be reported if
headers are incomplete. Here again, the error is detected later for now. But
it could be an issue later.

There is no reason to backport this patch.
---

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 45173b7dfd..2dcbf14a7a 100644
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1881,12 +1881,12 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count
 		se_fl_clr(h1s->sd, SE_FL_RCV_MORE | SE_FL_WANT_ROOM);
 		if (h1s->flags & H1S_F_REOS) {
 			se_fl_set(h1s->sd, SE_FL_EOS);
-			if (h1m->state >= H1_MSG_DONE || !(h1m->flags & H1_MF_XFER_LEN)) {
+			if (h1m->state >= H1_MSG_DONE || (h1m->state > H1_MSG_LAST_LF && !(h1m->flags & H1_MF_XFER_LEN))) {
 				/* DONE or TUNNEL or SHUTR without XFER_LEN, set
 				 * EOI on the stream connector */
 				se_fl_set(h1s->sd, SE_FL_EOI);
 			}
-			else if (h1m->state > H1_MSG_LAST_LF && h1m->state < H1_MSG_DONE) {
+			else if (h1m->state < H1_MSG_DONE) {
 				se_fl_set(h1s->sd, SE_FL_ERROR);
 				TRACE_ERROR("message aborted, set error on SC", H1_EV_RX_DATA|H1_EV_H1S_ERR, h1c->conn, h1s);
 			}
@@ -2942,16 +2942,16 @@ static int h1_process(struct h1c * h1c)
 			h1c->flags &= ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ);
 			TRACE_ERROR("internal error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
 		}
-		else if (h1s->flags & H1S_F_PARSING_ERROR) {
-			h1_handle_parsing_error(h1c);
-			h1c->flags = (h1c->flags & ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ)) | H1C_F_ST_ERROR;
-			TRACE_ERROR("parsing error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
-		}
 		else if (h1s->flags & H1S_F_NOT_IMPL_ERROR) {
 			h1_handle_not_impl_err(h1c);
 			h1c->flags = (h1c->flags & ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ)) | H1C_F_ST_ERROR;
 			TRACE_ERROR("not-implemented error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
 		}
+		else if (h1s->flags & H1S_F_PARSING_ERROR || se_fl_test(h1s->sd, SE_FL_ERROR)) {
+			h1_handle_parsing_error(h1c);
+			h1c->flags = (h1c->flags & ~(H1C_F_ST_IDLE|H1C_F_WAIT_NEXT_REQ)) | H1C_F_ST_ERROR;
+			TRACE_ERROR("parsing error detected", H1_EV_H1C_WAKE|H1_EV_H1C_ERR);
+		}
 	}
 	h1_send(h1c);