From: Amaury Denoyelle <adenoyelle@haproxy.com>
Date: Tue, 14 Feb 2023 14:36:36 +0000 (+0100)
Subject: BUG/MINOR: mux-quic: transfer FIN on empty STREAM frame
X-Git-Tag: v2.8-dev5~187
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fa241939c7ce77014eae2913252e5de21bdaa4d0;p=thirdparty%2Fhaproxy.git

BUG/MINOR: mux-quic: transfer FIN on empty STREAM frame

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
---

diff --git a/src/mux_quic.c b/src/mux_quic.c
index eb34c0b229..d68abbeee3 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -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);
 	}