MINOR: h3/hq-interop: handle no data in decode_qcs() with FIN set

Properly handle a STREAM frame with no data but the FIN bit set at the
application layer. H3 and hq-interop decode_qcs() callback have been
adjusted to not return early in this case.

If the FIN bit is accepted, a HTX EOM must be inserted for the upper
stream layer. If the FIN is rejected because the stream cannot be
closed, a proper CONNECTION_CLOSE error will be triggered.

A new utility function qcs_http_handle_standalone_fin() has been
implemented in the qmux_http module. This allows to simply add the HTX
EOM on qcs HTX buffer. If the HTX buffer is empty, a EOT is first added
to ensure it will be transmitted above.

This commit will allow to properly handle FIN notify through an empty
STREAM frame. However, it is not sufficient as currently qcc_recv() skip
the decode_qcs() invocation when the offset is already received. This
will be fixed in the next commit.

This should be backported up to 2.6 along with the next patch.

diff --git a/include/haproxy/qmux_http.h b/include/haproxy/qmux_http.h
index a7dbe7c..98151db 100644 (file)
--- a/include/haproxy/qmux_http.h
+++ b/include/haproxy/qmux_http.h
@@ -12,6 +12,8 @@ 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);
 
+void qcs_http_handle_standalone_fin(struct qcs *qcs);
+
 #endif /* USE_QUIC */
 
 #endif /* _HAPROXY_MUX_QUIC_HTTP_H */

diff --git a/src/h3.c b/src/h3.c
index ba33311..263e0ce 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -33,6 +33,7 @@
 #include <haproxy/istbuf.h>
 #include <haproxy/mux_quic.h>
 #include <haproxy/pool.h>
+#include <haproxy/qmux_http.h>
 #include <haproxy/qpack-dec.h>
 #include <haproxy/qpack-enc.h>
 #include <haproxy/quic_conn-t.h>
@@ -982,8 +983,6 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
        ssize_t total = 0, ret;
 
        h3_debug_printf(stderr, "%s: STREAM ID: %lu\n", __func__, qcs->id);
-       if (!b_data(b))
-               return 0;
 
        if (quic_stream_is_uni(qcs->id) && !(h3s->flags & H3_SF_UNI_INIT)) {
                if ((ret = h3_init_uni_stream(h3c, qcs, b)) < 0)
@@ -1013,6 +1012,11 @@ static ssize_t h3_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
                return -1;
        }
 
+       if (!b_data(b) && fin && quic_stream_is_bidi(qcs->id)) {
+               qcs_http_handle_standalone_fin(qcs);
+               return 0;
+       }
+
        while (b_data(b) && !(qcs->flags & QC_SF_DEM_FULL) && !h3c->err && !h3s->err) {
                uint64_t ftype, flen;
                char last_stream_frame = 0;

diff --git a/src/hq_interop.c b/src/hq_interop.c
index 37bb2e2..175b92d 100644 (file)
--- a/src/hq_interop.c
+++ b/src/hq_interop.c
@@ -7,6 +7,7 @@
 #include <haproxy/htx.h>
 #include <haproxy/http.h>
 #include <haproxy/mux_quic.h>
+#include <haproxy/qmux_http.h>
 
 static ssize_t hq_interop_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
 {
@@ -19,6 +20,13 @@ static ssize_t hq_interop_decode_qcs(struct qcs *qcs, struct buffer *b, int fin)
        size_t size = b_size(b);
        size_t data = b_data(b);
 
+       if (!data && fin) {
+               /* FIN is notified with an empty STREAM frame. */
+               BUG_ON(!qcs->sd); /* sd must already be attached here */
+               qcs_http_handle_standalone_fin(qcs);
+               return 0;
+       }
+
        b_alloc(&htx_buf);
        htx = htx_from_buf(&htx_buf);
 

diff --git a/src/mux_quic.c b/src/mux_quic.c
index 26006fb..eb34c0b 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -768,10 +768,10 @@ static int qcc_decode_qcs(struct qcc *qcc, struct qcs *qcs)
                ret = b_data(&b);
        }
 
-       if (ret) {
+       if (ret)
                qcs_consume(qcs, ret);
+       if (ret || (!b_data(&b) && fin))
                qcs_notify_recv(qcs);
-       }
 
        TRACE_LEAVE(QMUX_EV_QCS_RECV, qcc->conn, qcs);
        return 0;

diff --git a/src/qmux_http.c b/src/qmux_http.c
index 3ce4a34..6eedf0c 100644 (file)
--- a/src/qmux_http.c
+++ b/src/qmux_http.c
@@ -107,3 +107,25 @@ size_t qcs_http_reset_buf(struct qcs *qcs, struct buffer *buf, size_t count)
 
        return count;
 }
+
+/* Utility function which can be used by app layer an empty STREAM frame is
+ * received with FIN bit set for <qcs> stream. It will ensure that HTX EOM is
+ * properly inserted in <qcs> app_buf.
+ */
+void qcs_http_handle_standalone_fin(struct qcs *qcs)
+{
+       struct buffer *appbuf;
+       struct htx *htx = NULL;
+
+       appbuf = qc_get_buf(qcs, &qcs->rx.app_buf);
+       BUG_ON(!appbuf);
+
+       htx = htx_from_buf(appbuf);
+       if (htx_is_empty(htx)) {
+               if (!htx_add_endof(htx, HTX_BLK_EOT)) {
+                       ABORT_NOW(); /* cannot happen for empty HTX message. */
+               }
+       }
+       htx->flags |= HTX_FL_EOM;
+       htx_to_buf(htx, appbuf);
+}