MEDIUM: mux-h2: decode HEADERS frames before allocating the stream
It's hard to recover from a HEADERS frame decoding error after having
already created the stream, and it's not possible to recover from a
stream allocation error without dropping the connection since we can't
maintain the HPACK context, so let's decode it before allocating the
stream, into a temporary buffer that will then be offered to the newly
created stream.
diff --git a/src/mux_h2.c b/src/mux_h2.c
index dae53c6..54e823d 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -244,7 +244,7 @@
static int h2_process(struct h2c *h2c);
static struct task *h2_io_cb(struct task *t, void *ctx, unsigned short state);
static inline struct h2s *h2c_st_by_id(struct h2c *h2c, int id);
-static int h2s_decode_headers(struct h2s *h2s);
+static int h2c_decode_headers(struct h2c *h2c, struct buffer *rxbuf, uint32_t *flags);
static int h2_frt_transfer_data(struct h2s *h2s);
static struct task *h2_deferred_shut(struct task *t, void *ctx, unsigned short state);
static struct h2s *h2c_bck_stream_new(struct h2c *h2c, struct conn_stream *cs, struct session *sess);
@@ -1821,6 +1821,8 @@
*/
static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
{
+ struct buffer rxbuf = BUF_NULL;
+ uint32_t flags = 0;
int error;
if (!h2c->dfl) {
@@ -1855,6 +1857,12 @@
goto conn_err;
}
+ if (!h2c_decode_headers(h2c, &rxbuf, &flags))
+ goto out;
+
+ if (h2c->st0 >= H2_CS_ERROR)
+ goto out;
+
/* 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.
@@ -1866,19 +1874,17 @@
}
h2s->st = H2_SS_OPEN;
- if (h2c->dff & H2_F_HEADERS_END_STREAM) {
- h2s->st = H2_SS_HREM;
+ h2s->rxbuf = rxbuf;
+ h2s->flags |= flags;
+
+ if (h2c->dff & H2_F_HEADERS_END_STREAM)
h2s->flags |= H2_SF_ES_RCVD;
- /* note: cs cannot be null for now (just created above) */
+
+ if (h2s->flags & H2_SF_ES_RCVD) {
+ h2s->st = H2_SS_HREM;
h2s->cs->flags |= CS_FL_REOS;
}
- if (!h2s_decode_headers(h2s))
- return NULL;
-
- if (h2c->st0 >= H2_CS_ERROR)
- return NULL;
-
if (h2s->st >= H2_SS_ERROR) {
/* stream error : send RST_STREAM */
h2c->st0 = H2_CS_FRAME_E;
@@ -1893,7 +1899,7 @@
conn_err:
h2c_error(h2c, error);
- return NULL;
+ goto out;
strm_err:
if (h2s) {
@@ -1902,6 +1908,8 @@
}
else
h2c_error(h2c, error);
+ out:
+ h2_release_buf(h2c, &rxbuf);
return NULL;
}
@@ -1935,7 +1943,7 @@
h2s->cs->flags |= CS_FL_REOS;
}
- if (!h2s_decode_headers(h2s))
+ if (!h2c_decode_headers(h2c, &h2s->rxbuf, &h2s->flags))
return NULL;
if (h2c->st0 >= H2_CS_ERROR)
@@ -3125,18 +3133,16 @@
/* Decode the payload of a HEADERS frame and produce the equivalent HTTP/1 or
* HTX request or response depending on the connection's side. Returns the
- * number of bytes emitted if > 0, or 0 if it couldn't proceed. Stream errors
- * are reported in h2s->errcode and connection errors in h2c->errcode.
+ * number of bytes emitted if > 0, or 0 if it couldn't proceed. Connection
+ * errors in h2c->errcode.
*/
-static int h2s_decode_headers(struct h2s *h2s)
+static int h2c_decode_headers(struct h2c *h2c, struct buffer *rxbuf, uint32_t *flags)
{
- struct h2c *h2c = h2s->h2c;
const uint8_t *hdrs = (uint8_t *)b_head(&h2c->dbuf);
struct buffer *tmp = get_trash_chunk();
struct http_hdr list[MAX_HTTP_HDR * 2];
struct buffer *copy = NULL;
unsigned int msgf;
- struct buffer *csbuf;
struct htx *htx = NULL;
int flen = h2c->dfl - h2c->dpl;
int outlen = 0;
@@ -3161,7 +3167,7 @@
/* Skip StreamDep and weight for now (we don't support PRIORITY) */
if (h2c->dff & H2_F_HEADERS_PRIORITY) {
- if (read_n32(hdrs) == h2s->id) {
+ if (read_n32(hdrs) == h2c->dsi) {
/* RFC7540#5.3.1 : stream dep may not depend on itself */
h2c_error(h2c, H2_ERR_PROTOCOL_ERROR);
goto fail;
@@ -3180,8 +3186,7 @@
goto fail;
}
- csbuf = h2_get_buf(h2c, &h2s->rxbuf);
- if (!csbuf) {
+ if (!h2_get_buf(h2c, rxbuf)) {
h2c->flags |= H2_CF_DEM_SALLOC;
goto fail;
}
@@ -3193,15 +3198,15 @@
*/
if (h2c->proxy->options2 & PR_O2_USE_HTX) {
- htx = htx_from_buf(&h2s->rxbuf);
+ htx = htx_from_buf(rxbuf);
if (!htx_is_empty(htx))
goto fail;
} else {
- if (b_data(csbuf))
+ if (b_data(rxbuf))
goto fail;
- csbuf->head = 0;
- try = b_size(csbuf);
+ rxbuf->head = 0;
+ try = b_size(rxbuf);
}
outlen = hpack_decode_frame(h2c->ddht, hdrs, flen, list,
@@ -3222,7 +3227,7 @@
outlen = h2_make_htx_request(list, htx, &msgf);
} else {
/* HTTP/1 mode */
- outlen = h2_make_h1_request(list, b_tail(csbuf), try, &msgf);
+ outlen = h2_make_h1_request(list, b_tail(rxbuf), try, &msgf);
}
if (outlen < 0) {
@@ -3233,26 +3238,22 @@
if (msgf & H2_MSGF_BODY) {
/* a payload is present */
if (msgf & H2_MSGF_BODY_CL)
- h2s->flags |= H2_SF_DATA_CLEN;
+ *flags |= H2_SF_DATA_CLEN;
else if (!(msgf & H2_MSGF_BODY_TUNNEL) && !htx)
- h2s->flags |= H2_SF_DATA_CHNK;
+ *flags |= H2_SF_DATA_CHNK;
}
/* now consume the input data */
b_del(&h2c->dbuf, h2c->dfl);
h2c->st0 = H2_CS_FRAME_H;
- b_add(csbuf, outlen);
+ b_add(rxbuf, outlen);
- if (h2c->dff & H2_F_HEADERS_END_STREAM) {
- h2s->flags |= H2_SF_ES_RCVD;
- h2s->cs->flags |= CS_FL_REOS;
- if (htx)
- htx_add_endof(htx, HTX_BLK_EOM);
- }
+ if (htx && h2c->dff & H2_F_HEADERS_END_STREAM)
+ htx_add_endof(htx, HTX_BLK_EOM);
leave:
if (htx)
- htx_to_buf(htx, &h2s->rxbuf);
+ htx_to_buf(htx, rxbuf);
free_trash_chunk(copy);
return outlen;
fail: