From: Willy Tarreau <w@1wt.eu>
Date: Thu, 3 Jan 2019 13:48:18 +0000 (+0100)
Subject: MEDIUM: mux-h2: make h2c_decode_headers() support recoverable errors
X-Git-Tag: v2.0-dev1~287
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=259192370f182f2b4597fe1aecb8857e988b7c0e;p=thirdparty%2Fhaproxy.git

MEDIUM: mux-h2: make h2c_decode_headers() support recoverable errors

When a decoding error is recoverable, we should emit a stream error and
not a connection error. This patch does this by carefully checking the
connection state before deciding to send a connection error. If only the
stream is in error, an RST_STREAM is sent.
---

diff --git a/src/mux_h2.c b/src/mux_h2.c
index d7cf2379d9..a06ea5f1e9 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1878,12 +1878,23 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
 		goto conn_err;
 	}
 
-	if (h2c_decode_headers(h2c, &rxbuf, &flags) <= 0)
-		goto out;
+	error = h2c_decode_headers(h2c, &rxbuf, &flags);
 
+	/* unrecoverable error ? */
 	if (h2c->st0 >= H2_CS_ERROR)
 		goto out;
 
+	if (error <= 0) {
+		if (error == 0)
+			goto out; // missing data
+
+		/* Failed to decode this stream (e.g. too large request)
+		 * but the HPACK decompressor is still synchronized.
+		 */
+		h2s = (struct h2s*)h2_error_stream;
+		goto send_rst;
+	}
+
 	/* Note: we don't emit any other logs below because ff we return
 	 * positively from h2c_frt_stream_new(), the stream will report the error,
 	 * and if we return in error, h2c_frt_stream_new() will emit the error.
@@ -1965,15 +1976,20 @@ static struct h2s *h2c_bck_handle_headers(struct h2c *h2c, struct h2s *h2s)
 	if (b_data(&h2c->dbuf) < h2c->dfl && !b_full(&h2c->dbuf))
 		return NULL; // incomplete frame
 
-	if (h2c_decode_headers(h2c, &h2s->rxbuf, &h2s->flags) <= 0)
-		return NULL;
+	error = h2c_decode_headers(h2c, &h2s->rxbuf, &h2s->flags);
 
+	/* unrecoverable error ? */
 	if (h2c->st0 >= H2_CS_ERROR)
 		return NULL;
 
-	if (h2s->st >= H2_SS_ERROR) {
+	if (error <= 0) {
+		if (error == 0)
+			return NULL; // missing data
+
 		/* stream error : send RST_STREAM */
+		h2s_error(h2s, H2_ERR_PROTOCOL_ERROR);
 		h2c->st0 = H2_CS_FRAME_E;
+		return NULL;
 	}
 
 	if (h2c->dff & H2_F_HEADERS_END_STREAM) {
@@ -3338,6 +3354,7 @@ next_frame:
 		try = b_size(rxbuf);
 	}
 
+	/* past this point we cannot roll back in case of error */
 	outlen = hpack_decode_frame(h2c->ddht, hdrs, flen, list,
 	                            sizeof(list)/sizeof(list[0]), tmp);
 	if (outlen < 0) {
@@ -3345,6 +3362,14 @@ next_frame:
 		goto fail;
 	}
 
+	/* The PACK decompressor was updated, let's update the input buffer and
+	 * the parser's state to commit these changes and allow us to later
+	 * fail solely on the stream if needed.
+	 */
+	b_del(&h2c->dbuf, h2c->dfl + hole);
+	h2c->dfl = hole = 0;
+	h2c->st0 = H2_CS_FRAME_H;
+
 	/* OK now we have our header list in <list> */
 	msgf = (h2c->dff & H2_F_HEADERS_END_STREAM) ? 0 : H2_MSGF_BODY;
 
@@ -3362,7 +3387,7 @@ next_frame:
 	}
 
 	if (outlen < 0) {
-		h2c_error(h2c, H2_ERR_COMPRESSION_ERROR);
+		/* too large headers? this is a stream error only */
 		goto fail;
 	}
 
@@ -3374,11 +3399,6 @@ next_frame:
 			*flags |= H2_SF_DATA_CHNK;
 	}
 
-	/* now consume the input data (length of possibly aggregated frames) */
-	b_del(&h2c->dbuf, h2c->dfl + hole);
-	hole = 0;
-	h2c->st0 = H2_CS_FRAME_H;
-
 	if (htx && h2c->dff & H2_F_HEADERS_END_STREAM)
 		htx_add_endof(htx, HTX_BLK_EOM);