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 d7cf237..a06ea5f 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1878,12 +1878,23 @@
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 @@
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 @@
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 @@
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 @@
}
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 @@
*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);