BUG/MINOR: mux-quic: do not remotely close stream too early

A stream is considered as remotely closed once we have received all the
data with the FIN bit set.

The condition to close the stream was wrong. In particular, if we
receive an empty STREAM frame with FIN bit set, this would have close
the stream even if we do not have yet received all the data. The
condition is now adjusted to ensure that Rx buffer contains all the data
up to the stream final size.

In most cases, this bug is harmless. However, if compiled with
DEBUG_STRICT=2, a BUG_ON_HOT crash would have been triggered if close is
done too early. This was most notably the case sometimes on interop test
suite with quinn or kwik clients. This can also be artificially
reproduced by simulating reception of an empty STREAM frame with FIN bit
set in qc_handle_strm_frm() :

+       if (strm_frm->fin) {
+               qcc_recv(qc->qcc, strm_frm->id, 0,
+                        strm_frm->len, strm_frm->fin,
+                        (char *)strm_frm->data);
+       }
        ret = qcc_recv(qc->qcc, strm_frm->id, strm_frm->len,
                       strm_frm->offset.key, strm_frm->fin,
                       (char *)strm_frm->data);

This must be backported up to 2.6.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 5b4a35469e61adf6d9dad724132ff26630b8836a..f4b7e2f2e574886d24333ab5b20fd0bb46d055db 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -818,10 +818,8 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs)
 
        b = qcs_b_dup(&qcs->rx.ncbuf);
 
-       /* Signal FIN to application if STREAM FIN received and there is no gap
-        * in the Rx buffer.
-        */
-       if (qcs->flags & QC_SF_SIZE_KNOWN && !ncb_is_fragmented(&qcs->rx.ncbuf))
+       /* Signal FIN to application if STREAM FIN received with all data. */
+       if (qcs_is_close_remote(qcs))
                fin = 1;
 
        ret = qcc->app_ops->decode_qcs(qcs, &b, fin);
@@ -958,6 +956,10 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
        }
 
        if (offset + len <= qcs->rx.offset) {
+               /* TODO offset may have been received without FIN first and now
+                * with it. In this case, it must be notified to be able to
+                * close the stream.
+                */
                TRACE_DATA("already received offset", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
                goto out;
        }
@@ -1027,8 +1029,10 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
        if (fin)
                qcs->flags |= QC_SF_SIZE_KNOWN;
 
-       if (qcs->flags & QC_SF_SIZE_KNOWN && !ncb_is_fragmented(&qcs->rx.ncbuf))
+       if (qcs->flags & QC_SF_SIZE_KNOWN &&
+           qcs->rx.offset_max == qcs->rx.offset + ncb_data(&qcs->rx.ncbuf, 0)) {
                qcs_close_remote(qcs);
+       }
 
        if (ncb_data(&qcs->rx.ncbuf, 0) && !(qcs->flags & QC_SF_DEM_FULL)) {
                qcc_decode_qcs(qcc, qcs);