]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
BUG/MINOR: mux-quic: transfer FIN on empty STREAM frame
authorAmaury Denoyelle <adenoyelle@haproxy.com>
Tue, 14 Feb 2023 14:36:36 +0000 (15:36 +0100)
committerAmaury Denoyelle <adenoyelle@haproxy.com>
Fri, 17 Feb 2023 15:28:12 +0000 (16:28 +0100)
Implement support for clients that emit the stream FIN with an empty
STREAM frame. For that, qcc_recv() offset comparison has been adjusted.
If offset has already been received but the FIN bit is now transmitted,
do not skip the rest of the function and call application layer
decode_qcs() callback.

Without this, streams will be kept open forever as HTX EOM is never
transfered to the upper stream layer.

This behavior was observed with mvfst client prior to its patch
  38c955a024aba753be8bf50fdeb45fba3ac23cfd
  Fix hq-interop (HTTP 0.9 over QUIC)

This notably caused the interop multiplexing test to fail as unclosed
streams on haproxy side prevented the emission of new MAX_STREAMS frame
to the client.

This shoud be backported up to 2.6. It also relies on previous commit :
  381d8137e31d941c9143a1dc8b5760d29f388fef
  MINOR: h3/hq-interop: handle no data in decode_qcs() with FIN set

src/mux_quic.c

index eb34c0b229e66de00ae744f72ac0e67abc6118a7..d68abbeee3971c775ad4d80534b58c53d05e94ac 100644 (file)
@@ -962,11 +962,8 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
                goto out;
        }
 
-       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.
-                */
+       if (offset + len < qcs->rx.offset ||
+           (offset + len == qcs->rx.offset && (!fin || (qcs->flags & QC_SF_SIZE_KNOWN)))) {
                TRACE_DATA("already received offset", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
                goto out;
        }
@@ -1009,9 +1006,13 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
                offset = qcs->rx.offset;
        }
 
-       ret = ncb_add(&qcs->rx.ncbuf, offset - qcs->rx.offset, data, len, NCB_ADD_COMPARE);
-       if (ret != NCB_RET_OK) {
-               if (ret == NCB_RET_DATA_REJ) {
+       if (len) {
+               ret = ncb_add(&qcs->rx.ncbuf, offset - qcs->rx.offset, data, len, NCB_ADD_COMPARE);
+               switch (ret) {
+               case NCB_RET_OK:
+                       break;
+
+               case NCB_RET_DATA_REJ:
                        /* RFC 9000 2.2. Sending and Receiving Data
                         *
                         * An endpoint could receive data for a stream at the
@@ -1025,12 +1026,13 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
                        TRACE_ERROR("overlapping data rejected", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV|QMUX_EV_PROTO_ERR,
                                    qcc->conn, qcs);
                        qcc_emit_cc(qcc, QC_ERR_PROTOCOL_VIOLATION);
-               }
-               else if (ret == NCB_RET_GAP_SIZE) {
+                       return 1;
+
+               case NCB_RET_GAP_SIZE:
                        TRACE_DATA("cannot bufferize frame due to gap size limit", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV,
                                   qcc->conn, qcs);
+                       return 1;
                }
-               return 1;
        }
 
        if (fin)
@@ -1041,7 +1043,7 @@ int qcc_recv(struct qcc *qcc, uint64_t id, uint64_t len, uint64_t offset,
                qcs_close_remote(qcs);
        }
 
-       if (ncb_data(&qcs->rx.ncbuf, 0) && !(qcs->flags & QC_SF_DEM_FULL)) {
+       if ((ncb_data(&qcs->rx.ncbuf, 0) && !(qcs->flags & QC_SF_DEM_FULL)) || fin) {
                qcc_decode_qcs(qcc, qcs);
                qcc_refresh_timeout(qcc);
        }