BUG/MEDIUM: h3: do not block FIN on empty DATA frame

A HTTP/3 DATA frame may be empty as its payload is entirely optional.
However, prior to this patch, H3 stream parsing was interrupted in this
case, unless other frames follow. The main problem is that it would lead
to a missing EOM if QUIC FIN is reported with an empty DATA frame,
causing the stream to freeze until timeout.

The first step of this patch is to first remove the early return for
empty HTTP/3 frame payload. This has the effect to allow calling the
different frame handlers with an empty payload. The current patch is
dedicated to HTTP/3 DATA support; other patches will follow to ensure
the safety of the other frame types handlers.

The second part of this patch is to adjust h3_data_to_htx() which is the
HTTP/3 data frame handler. It is now safe to call it with an empty
frame, with or without FIN. It will default to a noop operation, unless
FIN is set, in which case htx_set_eom() is used to report the closure of
the stream read channel.

This must be backported up to 2.6.

diff --git a/src/h3.c b/src/h3.c
index dbf9dbc76e360150f362469941f68a4dace28de0..e149288ab4202774a355cfeac6b2fcb51ede6e8c 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1624,7 +1624,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
 static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
                               uint64_t len, char fin)
 {
-       struct buffer *appbuf;
+       struct buffer *appbuf = NULL;
        struct htx *htx = NULL;
        size_t htx_sent = 0;
        int htx_space;
@@ -1632,6 +1632,14 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
 
        TRACE_ENTER(H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
 
+       if (!len && !fin) {
+               /* Ignore empty frame to prevent unnecessary appbuf alloc. Not
+                * performed though if FIN is set as HTX EOM must be reported.
+                */
+               TRACE_STATE("skip parsing of empty DATA frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+               goto out;
+       }
+
        if (!(appbuf = qcc_get_stream_rxbuf(qcs))) {
                TRACE_ERROR("data buffer alloc failure", H3_EV_RX_FRAME|H3_EV_RX_DATA, qcs->qcc->conn, qcs);
                goto err;
@@ -1639,6 +1647,19 @@ static ssize_t h3_data_to_htx(struct qcs *qcs, const struct buffer *buf,
 
        htx = htx_from_buf(appbuf);
 
+       if (!len) {
+               /* Ensure EOM is reported when an empty frame is handled. */
+               if (!htx_set_eom(htx)) {
+                       TRACE_ERROR("cannot set EOM", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+                       qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1,
+                                     muxc_tevt_type_internal_err);
+                       goto err;
+               }
+
+               TRACE_STATE("EOM reported after empty frame", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
+               goto out;
+       }
+
        if (len > b_data(buf)) {
                len = b_data(buf);
                fin = 0;
@@ -1951,9 +1972,6 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                                        qcs->sd->kip += flen;
                                }
                        }
-
-                       if (!b_data(b))
-                               break;
                }
 
                /* RFC 9114 7.1. Frame Layout