MEDIUM: mux-h2: remove padlen during headers phase
Three types of frames may be padded : DATA, HEADERS and PUSH_PROMISE.
Currently, each of these independently deals with padding and needs to
wait for and skip the initial padlen byte. Not only this complicates
frame processing, but it makes it very hard to process CONTINUATION
frames after a padded HEADERS frame, and makes it complicated to perform
atomic calls to h2s_decode_headers(), which are needed if we want to be
able to maintain the HPACK decompressor's context even when dropping
streams.
This patch takes a different approach : the padding is checked when
parsing the frame header, the padlen byte is waited for and parsed,
and the dpl value is updated with this padlen value. This will allow
the frame parsers to decide to overwrite the padding if needed when
merging adjacent frames.
diff --git a/src/mux_h2.c b/src/mux_h2.c
index 6ae04f4..1608279 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2105,7 +2105,10 @@
goto fail;
}
- /* that's OK, switch to FRAME_P to process it */
+ /* that's OK, switch to FRAME_P to process it. This is
+ * a SETTINGS frame whose header has already been
+ * deleted above.
+ */
h2c->dfl = hdr.len;
h2c->dsi = hdr.sid;
h2c->dft = hdr.ft;
@@ -2124,6 +2127,7 @@
if (h2c->st0 == H2_CS_FRAME_H) {
struct h2_fh hdr;
+ unsigned int padlen = 0;
if (!h2_peek_frame_hdr(&h2c->dbuf, &hdr))
break;
@@ -2138,13 +2142,48 @@
break;
}
+ if (h2_ft_bit(hdr.ft) & H2_FT_PADDED_MASK && hdr.ff & H2_F_PADDED) {
+ /* If the frame is padded (HEADERS, PUSH_PROMISE or DATA),
+ * we read the pad length and drop it from the remaining
+ * payload (one byte + the 9 remaining ones = 10 total
+ * removed), so we have a frame payload starting after the
+ * pad len. Flow controlled frames (DATA) also count the
+ * padlen in the flow control, so it must be adjusted.
+ */
+ if (hdr.len < 1) {
+ h2c_error(h2c, H2_ERR_FRAME_SIZE_ERROR);
+ sess_log(h2c->conn->owner);
+ goto fail;
+ }
+ hdr.len--;
+
+ if (b_data(&h2c->dbuf) < 10)
+ break; // missing padlen
+
+ padlen = *(uint8_t *)b_peek(&h2c->dbuf, 9);
+
+ if (padlen > hdr.len) {
+ /* RFC7540#6.1 : pad length = length of
+ * frame payload or greater => error.
+ */
+ h2c_error(h2c, H2_ERR_PROTOCOL_ERROR);
+ sess_log(h2c->conn->owner);
+ goto fail;
+ }
+
+ if (h2_ft_bit(hdr.ft) & H2_FT_FC_MASK) {
+ h2c->rcvd_c++;
+ h2c->rcvd_s++;
+ }
+ b_del(&h2c->dbuf, 1);
+ }
+ h2_skip_frame_hdr(&h2c->dbuf);
h2c->dfl = hdr.len;
h2c->dsi = hdr.sid;
h2c->dft = hdr.ft;
h2c->dff = hdr.ff;
- h2c->dpl = 0;
+ h2c->dpl = padlen;
h2c->st0 = H2_CS_FRAME_P;
- h2_skip_frame_hdr(&h2c->dbuf);
}
/* Only H2_CS_FRAME_P and H2_CS_FRAME_A here */
@@ -3099,7 +3138,7 @@
unsigned int msgf;
struct buffer *csbuf;
struct htx *htx = NULL;
- int flen = h2c->dfl;
+ int flen = h2c->dfl - h2c->dpl;
int outlen = 0;
int wrap;
int try = 0;
@@ -3126,20 +3165,6 @@
hdrs = (uint8_t *) copy->area;
}
- /* The padlen is the first byte before data, and the padding appears
- * after data. padlen+data+padding are included in flen.
- */
- if (h2c->dff & H2_F_HEADERS_PADDED) {
- h2c->dpl = *hdrs;
- if (h2c->dpl >= flen) {
- /* RFC7540#6.2 : pad length = length of frame payload or greater */
- h2c_error(h2c, H2_ERR_PROTOCOL_ERROR);
- goto fail;
- }
- flen -= h2c->dpl + 1;
- hdrs += 1; // skip Pad Length
- }
-
/* Skip StreamDep and weight for now (we don't support PRIORITY) */
if (h2c->dff & H2_F_HEADERS_PRIORITY) {
if (read_n32(hdrs) == h2s->id) {
@@ -3266,27 +3291,6 @@
h2c->flags &= ~H2_CF_DEM_SFULL;
- /* The padlen is the first byte before data, and the padding appears
- * after data. padlen+data+padding are included in flen.
- */
- if (h2c->dff & H2_F_DATA_PADDED) {
- if (b_data(&h2c->dbuf) < 1)
- return 0;
-
- h2c->dpl = *(uint8_t *)b_head(&h2c->dbuf);
- if (h2c->dpl >= h2c->dfl) {
- /* RFC7540#6.1 : pad length = length of frame payload or greater */
- h2c_error(h2c, H2_ERR_PROTOCOL_ERROR);
- return 0;
- }
-
- /* skip the padlen byte */
- b_del(&h2c->dbuf, 1);
- h2c->dfl--;
- h2c->rcvd_c++; h2c->rcvd_s++;
- h2c->dff &= ~H2_F_DATA_PADDED;
- }
-
csbuf = h2_get_buf(h2c, &h2s->rxbuf);
if (!csbuf) {
h2c->flags |= H2_CF_DEM_SALLOC;