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 <rxbuf>. 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 <qcs> stream <rxbuf> 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
/* 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) {
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) {
}
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) {
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);
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.
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));
*/
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;
}
projects / haproxy.git / commitdiff