MEDIUM: mux-quic: add BUG_ON if sending on locally closed QCS

Previously, if snd_buf operation was conducted despite QCS already
locally closed, the input buffer was silently dropped. This situation
could happen if a RESET_STREAM was emitted butemission not reported to
the stream layer. Resetting silently the buffer ensure QUIC MUX remain
compliant with RFC 9000 which forbid emission after RESET_STREAM.

Since previous commit, it is now ensured that RESET_STREAM sending will
always be reported to stream-layer. Thus, there is no need anymore to
silently reset the buffer. A BUG_ON() statement is added to ensure this
assumption will remain valid.

The new code is deemed cleaner as it does not hide a missing error
notification on the stconn-layer. Previously, if an error was missing,
sending would continue unnecessarily with a false success status
reported for the stream.

Note that the BUG_ON() statement was also added into nego_ff callback.
This is necessary to ensure both sending path remains consistent.

This patch is labelled as MEDIUM as issues were already encountered in
snd_buf/nego_ff implementation and it's not easy to cover all occurences
during test. If the BUG_ON() is triggered without any apparent
stream-layer issue, this commit should be reverted.

diff --git a/include/haproxy/qmux_http.h b/include/haproxy/qmux_http.h
index a7dbe7cc329ad79d06505b5e285ef513cc4b271a..4a7711401e73f32505ba56a9366a67912fbf9b30 100644 (file)
--- a/include/haproxy/qmux_http.h
+++ b/include/haproxy/qmux_http.h
@@ -10,7 +10,6 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count,
                         char *fin);
 size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
                         char *fin);
-size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count);
 
 #endif /* USE_QUIC */
 

diff --git a/src/mux_quic.c b/src/mux_quic.c
index bf3616e0c0ec967b8d9136203c1a5860fd93c76f..21b1cdec79a5eb39a45a94b76b1a5c8c4f9b2412 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -2811,6 +2811,9 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf,
 
        TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
 
+       /* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */
+       BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET));
+
        /* stream layer has been detached so no transfer must occur after. */
        BUG_ON_HOT(qcs->flags & QC_SF_DETACH);
 
@@ -2821,11 +2824,6 @@ static size_t qmux_strm_snd_buf(struct stconn *sc, struct buffer *buf,
                goto end;
        }
 
-       if (qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET)) {
-               ret = qcs_http_reset_buf(qcs, buf, count);
-               goto end;
-       }
-
        ret = qcs_http_snd_buf(qcs, buf, count, &fin);
        if (fin) {
                TRACE_STATE("reached stream fin", QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
@@ -2853,6 +2851,9 @@ static size_t qmux_strm_nego_ff(struct stconn *sc, struct buffer *input,
 
        TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
 
+       /* Sending forbidden if QCS is locally closed (FIN or RESET_STREAM sent). */
+       BUG_ON(qcs_is_close_local(qcs) || (qcs->flags & QC_SF_TO_RESET));
+
        /* stream layer has been detached so no transfer must occur after. */
        BUG_ON_HOT(qcs->flags & QC_SF_DETACH);
 

diff --git a/src/qmux_http.c b/src/qmux_http.c
index 3c5f3917cb9b2fcfc5a28a5efbe10f2d4e113d78..092eb15da9aea14a9a478fe1e7a5006397865dae 100644 (file)
--- a/src/qmux_http.c
+++ b/src/qmux_http.c
@@ -95,23 +95,3 @@ size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
 
        return ret;
 }
-
-/* QUIC MUX snd_buf reset. HTX data stored in <buf> of length <count> will be
- * cleared. This can be used when data should not be transmitted any longer.
- *
- * Return the size in bytes of cleared data.
- */
-size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count)
-{
-       struct htx *htx;
-
-       TRACE_ENTER(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
-
-       htx = htx_from_buf(buf);
-       htx_reset(htx);
-       htx_to_buf(htx, buf);
-
-       TRACE_LEAVE(QMUX_EV_STRM_SEND, qcs->qcc->conn, qcs);
-
-       return count;
-}