BUG/MINOR: mux-quic: handle closure of uni-stream

This commit is a direct follow-up to the previous one. As already
described, a previous fix was merged to prevent streamdesc attach
operation on already completed QCS instances scheduled for purging. This
was implemented by skipping app proto decoding.

However, this has a bad side-effect for remote uni-directional stream.
If receiving a FIN stream frame on such a stream, it will considered as
complete because streamdesc are never attached to a uni stream. Due to
the mentionned new fix, this prevent analysis of this last frame for
every uni stream.

To fix this, do not skip anymore app proto decoding for completed QCS.
Update instead qcs_attach_sc() to transform it as a noop function if QCS
is already fully closed before streamdesc instantiation. However,
success return value is still used to prevent an invalid decoding error
report.

The impact of this bug should be minor. Indeed, HTTP3 and QPACK uni
streams are never closed by the client as this is invalid due to the
spec. The only issue was that this prevented QUIC MUX to close the
connection with error H3_ERR_CLOSED_CRITICAL_STREAM.

This must be backported along the previous patch, at least to 3.1, and
eventually to 2.8 if mentionned patches are merged there.

(cherry picked from commit 801e39e1ccfc7404354d6870e779f83351593df0)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 85e3bf658163d97fd09ac63f6c997d230d32729b)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/h3.c b/src/h3.c
index b760d36..2e7c418 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -938,8 +938,8 @@ static ssize_t h3_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        if (htx)
                htx_to_buf(htx, &htx_buf);
 
-       /* buffer is transferred to the stream connector and set to NULL
-        * except on stream creation error.
+       /* Buffer is transferred to the stream connector and set to NULL except
+        * on stream creation error or QCS already fully closed.
         */
        if (b_size(&htx_buf)) {
                b_free(&htx_buf);

diff --git a/src/mux_quic.c b/src/mux_quic.c
index a68d5f2..3593b00 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -715,7 +715,12 @@ static struct qcs *qcc_init_stream_remote(struct qcc *qcc, uint64_t id)
  * the first received request data. <fin> must be set if the whole request is
  * already received.
  *
- * Returns 0 on success else a negative error code.
+ * Note that if <qcs> is already fully closed, no streamdesc is instantiated.
+ * This is useful if a RESET_STREAM was already emitted in response to a
+ * STOP_SENDING.
+ *
+ * Returns 0 on success else a negative error code. If stream is already fully
+ * closed and nothing is performed, it is considered as a success case.
  */
 int qcs_attach_sc(struct qcs *qcs, struct buffer *buf, char fin)
 {
@@ -723,6 +728,11 @@ int qcs_attach_sc(struct qcs *qcs, struct buffer *buf, char fin)
        struct session *sess = qcc->conn->owner;
 
 
+       if (qcs->st == QC_SS_CLO) {
+               TRACE_STATE("skip attach on already closed stream", QMUX_EV_STRM_RECV, qcc->conn, qcs);
+               goto out;
+       }
+
        /* TODO duplicated from mux_h2 */
        sess->t_idle = ns_to_ms(now_ns - sess->accept_ts) - sess->t_handshake;
 
@@ -771,6 +781,7 @@ int qcs_attach_sc(struct qcs *qcs, struct buffer *buf, char fin)
                se_fl_set_error(qcs->sd);
        }
 
+ out:
        return 0;
 }
 
@@ -964,7 +975,7 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs)
        if (qcs_is_close_remote(qcs))
                fin = 1;
 
-       if (!(qcs->flags & QC_SF_READ_ABORTED) && !qcs_is_completed(qcs)) {
+       if (!(qcs->flags & QC_SF_READ_ABORTED)) {
                ret = qcc->app_ops->rcv_buf(qcs, &b, fin);
 
                if (qcc->glitches != prev_glitches)