BUG/MINOR: h3: ensure that invalid status code are not encoded (FE side)

On frontend side, H3 layer transcodes HTX status code into HTTP/3
HEADERS frame. This is done by calling qpack_encode_int_status().

Prior to this patch, the latter function was also responsible to reject
an invalid value, which guarantee that only valid codes are encoded
(between 100 and 999 values). However, this is not practical as it is
impossible to differentiate between an invalid code error and a buffer
room exhaustation.

Changes this so that now HTTP/3 layer first ensures that HTX code is
valid. The stream is closed with H3_INTERNAL_ERROR if invalid value is
present. Thus, qpack_encode_int_status() will only report an error due
to buffer room exhaustion. If a small buffer is used, a standard buffer
will be reallocated which should be sufficient to encode the response.

The impact of this bug is minimal. Its main benefit is code clarity,
while also removing an unnecessary realloc when confronting with an
invalid HTTP code.

This should be backported at least up to 3.1. Prior to it, smallbuf
mechanism isn't present, hence the impact of this patch is less
important. However, it may still be backported to older versions, which
should facilitate picking patches for HTTP 1xx interim response support.

diff --git a/src/h3.c b/src/h3.c
index 53679b91de05b4f8a6679651377ca08e332d9865..ac39feef4191a5eb22b95deea53bb3ad0473e360 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -2177,8 +2177,11 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                        /* start-line -> HEADERS h3 frame */
                        BUG_ON(sl);
                        sl = htx_get_blk_ptr(htx, blk);
-                       /* TODO should be on h3 layer */
                        status = sl->info.res.status;
+                       if (status < 100 || status > 999) {
+                               TRACE_ERROR("invalid response status code", H3_EV_STRM_SEND, qcs->qcc->conn, qcs);
+                               goto err;
+                       }
                }
                else if (type == HTX_BLK_HDR) {
                        if (unlikely(hdr >= sizeof(list) / sizeof(list[0]) - 1)) {
@@ -2196,6 +2199,9 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                }
        }
 
+       /* Current function expects HTX start-line to be present. This also
+        * ensures <status> conformance has been checked prior to encoding it.
+        */
        BUG_ON(!sl);
 
        list[hdr].n = ist("");
@@ -2233,7 +2239,6 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
        if (qpack_encode_field_section_line(&headers_buf))
                goto err_full;
        if (qpack_encode_int_status(&headers_buf, status)) {
-               /* TODO handle invalid status code VS no buf space left */
                TRACE_ERROR("error during status code encoding", H3_EV_TX_FRAME|H3_EV_TX_HDR, qcs->qcc->conn, qcs);
                goto err_full;
        }

diff --git a/src/qpack-enc.c b/src/qpack-enc.c
index 311d2ae2053d049edfbe8f8803fae8fae9d6d41e..19de6a9ec74c15cbf7049013d83c0b4ea40ddac0 100644 (file)
--- a/src/qpack-enc.c
+++ b/src/qpack-enc.c
@@ -69,8 +69,8 @@ int qpack_encode_int_status(struct buffer *out, unsigned int status)
 {
        int status_size, idx = 0;
 
-       if (status < 100 || status > 999)
-               return 1;
+       /* HTTP layer must not encode invalid status codes. */
+       BUG_ON(status < 100 || status > 999);
 
        switch (status) {
        case 103: idx = 24; break;