BUG/MINOR: h2/h3: Never insert partial headers/trailers in an HTX message

In HTX, headers and trailers parts must always be complete. It is unexpected
to found header blocks without the EOH block or trailer blocks without the
EOT block. So, during H2/H3 message parsing, we must take care to remove any
HEADER/TRAILER block inserted when an error is encountered. It is mandatory
to be sure to properly report parsing error to upper layer.x

It is now performed by calling htx_truncat_blk() function on the error
path. The tail block is saved before converting any HEADERS/TRAILERS frame
to HTX. It is used to remove all inserted block on error.

This patch rely on the following one:

  "MINOR: htx: Add function to truncate all blocks after a specific block"

It should be backported with the commit above to all stable versions for
the H2 part and as far as 2.8 for h3 one.

diff --git a/src/h2.c b/src/h2.c
index 64c23d4745bbf15915102642802c460294124a00..ef30384ceaefb8051131fd2495597d17a1a2992b 100644 (file)
--- a/src/h2.c
+++ b/src/h2.c
@@ -319,6 +319,7 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr,
  */
 int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, int relaxed)
 {
+       struct htx_blk *tailblk = htx_get_tail_blk(htx);
        struct ist phdr_val[H2_PHDR_NUM_ENTRIES];
        uint32_t fields; /* bit mask of H2_PHDR_FND_* */
        uint32_t idx;
@@ -533,6 +534,7 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
        return ret;
 
  fail:
+       htx_truncate_blk(htx, tailblk);
        return -1;
 }
 
@@ -637,6 +639,7 @@ static struct htx_sl *h2_prepare_htx_stsline(uint32_t fields, struct ist *phdr,
  */
 int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, char *upgrade_protocol)
 {
+       struct htx_blk *tailblk = htx_get_tail_blk(htx);
        struct ist phdr_val[H2_PHDR_NUM_ENTRIES];
        uint32_t fields; /* bit mask of H2_PHDR_FND_* */
        uint32_t idx;
@@ -793,6 +796,7 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
        return ret;
 
  fail:
+       htx_truncate_blk(htx, tailblk);
        return -1;
 }
 
@@ -812,6 +816,7 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m
  */
 int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx)
 {
+       struct htx_blk *tailblk = htx_get_tail_blk(htx);
        const char *ctl;
        struct ist v;
        uint32_t idx;
@@ -871,5 +876,6 @@ int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx)
        return 1;
 
  fail:
+       htx_truncate_blk(htx, tailblk);
        return -1;
 }

diff --git a/src/h3.c b/src/h3.c
index 09634f14c05bb9a7b252c243cc3a32f4a245d555..ef6502d77f090308334faf1794eee60a98dd1fd9 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1111,6 +1111,7 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        struct buffer *tmp = get_trash_chunk();
        struct htx *htx = NULL;
        struct htx_sl *sl;
+       struct htx_blk *tailblk = NULL;
        struct http_hdr list[global.tune.max_http_hdr * 2];
        unsigned int flags = HTX_SL_F_NONE;
        struct ist status = IST_NULL;
@@ -1161,7 +1162,7 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        }
        BUG_ON(!b_size(appbuf)); /* TODO */
        htx = htx_from_buf(appbuf);
-
+       tailblk = htx_get_tail_blk(htx);
        /* Only handle one HEADERS frame at a time. Thus if HTX buffer is too
         * small, it happens solely from a single frame and the only option is
         * to close the stream.
@@ -1351,8 +1352,11 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        }
 
  out:
-       if (appbuf)
+       if (appbuf) {
+               if ((ssize_t)len < 0)
+                       htx_truncate_blk(htx, tailblk);
                htx_to_buf(htx, appbuf);
+       }
 
        TRACE_LEAVE(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
        return len;
@@ -1376,6 +1380,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
        struct buffer *appbuf = NULL;
        struct htx *htx = NULL;
        struct htx_sl *sl;
+       struct htx_blk *tailblk = NULL;
        struct http_hdr list[global.tune.max_http_hdr * 2];
        int hdr_idx, ret;
        const char *ctl;
@@ -1406,6 +1411,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
        }
        BUG_ON(!b_size(appbuf)); /* TODO */
        htx = htx_from_buf(appbuf);
+       tailblk = htx_get_tail_blk(htx);
 
        if (!h3s->data_len) {
                /* Notify that no body is present. This can only happens if
@@ -1521,8 +1527,11 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
 
  out:
        /* HTX may be non NULL if error before previous htx_to_buf(). */
-       if (appbuf)
+       if (appbuf) {
+               if ((ssize_t)len < 0)
+                       htx_truncate_blk(htx, tailblk);
                htx_to_buf(htx, appbuf);
+       }
 
        TRACE_LEAVE(H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
        return len;