From: Amaury Denoyelle Date: Thu, 8 Oct 2026 07:34:34 +0000 (+0200) Subject: MEDIUM: h3: do not close conn on unaligned non DATA frame X-Git-Tag: v3.5-dev9~171 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=8fdd2099ff840916fcfd772081c97a649de62674;p=haproxy.git MEDIUM: h3: do not close conn on unaligned non DATA frame Until now, H3 parser rejected a non aligned frame, except for DATA. The whole connection was closed with error H3_EXCESSIVE_LOAD, which can be seen as a medium defect given that the client did not produce anything invalid or even suspect. In fact, most frame parsers are already able to support wrapping content, as they deal with a struct buffer representation. This is also the case for QPACK uni streams. The real limitation is for HEADERS frame which rely on qpack_decode_fs() which can only work on aligned content. This patch removes the connection closure on unaligned content. For H3 HEADERS frames, the buffer content is realigned by performing two memmove() calls into a temporarily allocated alternative area. The current parsing iteration will continue in this new aligned storage. A future optimisation would be to support wrapping content in qpack_decode_fs() to remove realign operation. This can be considered as a bug, as previous state prevents to handle valid H3 content. As no user complains about it, it may be better to refrain from backporting the patch, unless there is some explicit demand about it. Note that original Rx buffer is not realigned, as this would mangle the underlying MUX ncbuf storage. An alternative approach was implemented first, performing the realign at the MUX level, however it was probably less efficient as it required to copy the whole ncbuf. For the record, this patch is attached here with its original message. --- Subject: [PATCH] MEDIUM: quic/h3: support parsing of unaligned non DATA H3 frame Until now, H3 parser was not able to parse a non DATA frame which wraps in QCS Rx buffer. When this happened, the whole connection was closed with error H3_EXCESSIVE_LOAD, which can be seen as a medium defect given that the client did not produce anything invalid or even suspect. This patch finally implements a method to deal with unaligned non DATA H3 frames. When such frame is detected by H3 parser, rcv_buf callback is interrupted. MUX will react to a null returned value and detect that the buffer data is wrapping. In this case, the new function qcs_realign_rx_data() is used to perform the realign operation using b_slow_realign() helper. Callback rcv_buf is then reinvoked immediately. This can be considered as a bug, as previous state prevents to handle valid H3 content. As no user complains about it, it may be better to refrain from backporting the patch, unless there is some explicit demand about it. --- src/h3.c | 8 ++------ src/mux_quic.c | 35 ++++++++++++++++++++++++++++++++--- 2 files changed, 34 insertions(+), 9 deletions(-) diff --git a/src/h3.c b/src/h3.c index 4750ff69f..1ed4a4d8b 100644 --- a/src/h3.c +++ b/src/h3.c @@ -2068,13 +2068,9 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) goto err; } - /* TODO extend parser to support the realignment of a frame. */ if (b_head(b) + b_data(b) > b_wrap(b)) { - TRACE_ERROR("cannot parse unaligned data frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); - qcc_set_error(qcs->qcc, H3_ERR_EXCESSIVE_LOAD, 1, - muxc_tevt_type_other_err); - qcc_report_glitch(qcs->qcc, 1); - goto err; + TRACE_PROTO("pause parsing on unaligned payload", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); + break; } /* Only parse full HTTP/3 frames. */ diff --git a/src/mux_quic.c b/src/mux_quic.c index de443a568..70a6cad2d 100644 --- a/src/mux_quic.c +++ b/src/mux_quic.c @@ -1255,6 +1255,24 @@ static inline struct buffer qcs_b_dup(const struct qc_stream_rxbuf *b) } } +/* Realign buffer content in . This should be called when parsing is + * blocked due to wrapping content. + */ +static void qcs_realign_rx_data(struct qc_stream_rxbuf *rxbuf) +{ + struct ncbuf *ncb = &rxbuf->ncb; + struct buffer old; + + /* Convert ncbuf to a full buffer type. This forces b_slow_realign() + * will copy every non contiguous blocks plus first ncb block header. + */ + old = b_make(ncb_orig(ncb), ncb->size, ncb->head, ncb->size); + b_slow_realign(&old, trash.area, 0); + + /* ncb area now realigned - ensure head is resetted as well. */ + ncb->head = 0; +} + /* Transfer data into stream current Rx buffer from its directly * following buffer. This is useful when parsing was interrupted due to partial * data. If following buffer does not exists, nothing is done. @@ -1508,9 +1526,20 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs) * at a Rx buffer boundary. Try to realign data if possible and * restart decoding. */ - if (!ret && rxbuf && !(qcs->flags & QC_SF_DEM_FULL) && - qcs->rx.offset + ncb_data(&rxbuf->ncb, 0) == rxbuf->off_end) { - if (!qcs_transfer_rx_data(qcs, rxbuf)) { + if (!ret && rxbuf && !(qcs->flags & QC_SF_DEM_FULL)) { + int need_restart = 0; + + if (ncb_head(&rxbuf->ncb) + ncb_data(&rxbuf->ncb, 0) > ncb_wrap(&rxbuf->ncb)) { + qcs_realign_rx_data(rxbuf); + need_restart = 1; + } + + if (qcs->rx.offset + ncb_data(&rxbuf->ncb, 0) == rxbuf->off_end) { + if (!qcs_transfer_rx_data(qcs, rxbuf)) + need_restart = 1; + } + + if (need_restart) { TRACE_DEVEL("restart parsing after data realignment", QMUX_EV_QCS_RECV, qcc->conn, qcs); goto restart; } -- 2.56.0 --- diff --git a/src/h3.c b/src/h3.c index a52538cf8..57b2c00bd 100644 --- a/src/h3.c +++ b/src/h3.c @@ -755,8 +755,6 @@ static ssize_t h3_req_headers_to_htx(struct qcs *qcs, const struct buffer *buf, /* TODO support trailer parsing in this function */ - /* TODO support buffer wrapping */ - BUG_ON(b_head(buf) + len > b_wrap(buf)); ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp, list, sizeof(list) / sizeof(list[0])); if (ret < 0) { @@ -1260,8 +1258,6 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf, goto out; } - /* TODO support buffer wrapping */ - BUG_ON(b_head(buf) + len > b_wrap(buf)); ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp, list, sizeof(list) / sizeof(list[0])); if (ret < 0) { @@ -1532,9 +1528,6 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf, } if (len) { - /* TODO support buffer wrapping */ - BUG_ON(b_head(buf) + len > b_wrap(buf)); - ret = qpack_decode_fs((const unsigned char *)b_head(buf), len, tmp, list, sizeof(list) / sizeof(list[0])); if (ret < 0) { @@ -1908,6 +1901,7 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) struct h3s *h3s = qcs->ctx; struct h3c *h3c = h3s->h3c; ssize_t total = 0, ret = 0; + struct buffer aligned = BUF_NULL; TRACE_ENTER(H3_EV_RX_FRAME, qcs->qcc->conn, qcs); @@ -2050,8 +2044,8 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) ftype = h3s->demux_frame_type; /* Current HTTP/3 parser can currently only parse fully - * received and aligned frames. The only exception is for DATA - * frames as they can frequently be larger than bufsize. + * received frames. The only exception is for DATA frames as + * they can frequently be larger than bufsize. */ if (ftype != H3_FT_DATA) { /* Reject frames bigger than bufsize. @@ -2068,20 +2062,43 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) goto err; } - /* TODO extend parser to support the realignment of a frame. */ - if (b_head(b) + b_data(b) > b_wrap(b)) { - TRACE_ERROR("cannot parse unaligned data frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); - qcc_set_error(qcs->qcc, H3_ERR_EXCESSIVE_LOAD, 1, - muxc_tevt_type_other_err); - qcc_report_glitch(qcs->qcc, 1); - goto err; - } - /* Only parse full HTTP/3 frames. */ if (flen > b_data(b)) { TRACE_PROTO("pause parsing on incomplete payload", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); break; } + + /* HEADERS frame must be aligned before parsing. This + * is a requirement for qpack_decode_fs(). + * + * TODO implement wrapping content decoding in + * qpack_decode_fs() to remove alignment requirement + */ + if (ftype == H3_FT_HEADERS && b_head(b) + flen > b_wrap(b)) { + const size_t contig = b_wrap(b) - b_head(b); + + /* Cannot realign on the original area as this + * would mangle the base ncbuf MUX storage. + * Switch to a temporary area for the remainder + * of this parsing iteration. + */ + + /* TODO use a thread-local static storage buf, + * safe as not kept beyond current call. + * Do not rely on trash as already used by H3/QPACK parsers. + */ + if (!b_alloc(&aligned, DB_MUX_RX)) { + TRACE_ERROR("Rx aligned buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1, muxc_tevt_type_internal_err); + goto err; + } + + TRACE_PROTO("realign frame content", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); + memmove(aligned.area, b_head(b), contig); + memmove(aligned.area + contig, b_orig(b), b_data(b) - contig); + b->area = aligned.area; + b->head = 0; + } } last_stream_frame = (fin && flen == b_data(b)); @@ -2290,10 +2307,12 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) */ done: + b_free(&aligned); TRACE_LEAVE(H3_EV_RX_FRAME, qcs->qcc->conn, qcs); return total; err: + b_free(&aligned); TRACE_DEVEL("leaving on error", H3_EV_RX_FRAME, qcs->qcc->conn, qcs); return -1; }