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 a7dbe7cc329ad79d06505b5e285ef513cc4b271a..98151db16711d578161de8d7278f911c0c03ee4d 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 ba3331120ffa411bd983363566d3b7d98403e487..263e0ceec5cb6945560cd4f573ea7f2de88a35bc 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 37bb2e219a7a5a50b49a8563a00a24ebc9575c6d..175b92decfd6d4d3b15115594f0ed89d76532178 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 26006fb66789a7e605de33fbeff53bb4426530aa..eb34c0b229e66de00ae744f72ac0e67abc6118a7 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 3ce4a343802e368937352120437d969c02a62cff..6eedf0c4a208ecab87b0a257ae6413427c994328 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);
+}