BUG/MEDIUM: h3: do not overwrite interim with final response

An HTTP response may contain several interim response message prior (1xx
status) to a final response message (all other status codes). This may
cause issues with h3_resp_headers_send() called for response encoding
which assumes that it is only call one time per stream, most notably
during output buffer handling.

This commit fixes output buffer handling when h3_resp_headers_send() is
called multiple times due to an interim response. Prior to it, interim
response was overwritten with newer response message. Most of the time,
this resulted in error for the client due to QPACK decoding failure.
This is now fixed so that each response is encoded one after the other.

Note that if encoding of several responses is bigger than output buffer,
an error is reported. This can definitely occurs as small buffer are
used during header encoding. This situation will be improved by the next
patch.

This must be backported up to 2.6.

(cherry picked from commit 1290fb731d9bb84777d811b69cf71106b89f92cf)
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>
(cherry picked from commit 888fe7cc77106cc620c0c98777ac5345a2e6eeef)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 3baeb9f43035139be4fc50eac55b60d8f2b9fa1d)
[ad: replace label err_full used only since smallbuf alloc support]
Signed-off-by: Amaury Denoyelle <adenoyelle@haproxy.com>

diff --git a/src/h3.c b/src/h3.c
index cab7b2e..bd6f95d 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1733,13 +1733,20 @@ static int h3_resp_headers_send(struct qcs *qcs, struct htx *htx)
                goto end;
        }
 
-       /* Buffer allocated just now : must be enough for frame type + length as a max varint size */
-       BUG_ON(b_room(res) < 5);
+       /* Reserve space for frame type + length as a max varint size. */
+       if (unlikely(b_contig_space(res) < 5)) {
+               /* Most of the times, h3_resp_headers_send() is only called one
+                * time per stream, so buffer will be empty. However, this
+                * assumption is invalid when handling interim responses, so
+                * it's important to check out buffer remaining space.
+                */
+               goto err;
+       }
 
        b_reset(&outbuf);
        outbuf = b_make(b_tail(res), b_contig_space(res), 0, 0);
        /* Start the headers after frame type + length */
-       headers_buf = b_make(b_head(res) + 5, b_size(res) - 5, 0, 0);
+       headers_buf = b_make(b_tail(res) + 5, b_contig_space(res) - 5, 0, 0);
 
        TRACE_DATA("encoding HEADERS frame", H3_EV_TX_FRAME|H3_EV_TX_HDR,
                   qcs->qcc->conn, qcs);