From 381d8137e31d941c9143a1dc8b5760d29f388fef Mon Sep 17 00:00:00 2001 From: Amaury Denoyelle Date: Fri, 17 Feb 2023 09:51:20 +0100 Subject: [PATCH] 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. --- include/haproxy/qmux_http.h | 2 ++ src/h3.c | 8 ++++++-- src/hq_interop.c | 8 ++++++++ src/mux_quic.c | 4 ++-- src/qmux_http.c | 22 ++++++++++++++++++++++ 5 files changed, 40 insertions(+), 4 deletions(-) diff --git a/include/haproxy/qmux_http.h b/include/haproxy/qmux_http.h index a7dbe7cc32..98151db167 100644 --- 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 ba3331120f..263e0ceec5 100644 --- a/src/h3.c +++ b/src/h3.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -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 37bb2e219a..175b92decf 100644 --- a/src/hq_interop.c +++ b/src/hq_interop.c @@ -7,6 +7,7 @@ #include #include #include +#include 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 26006fb667..eb34c0b229 100644 --- 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 3ce4a34380..6eedf0c4a2 100644 --- 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 stream. It will ensure that HTX EOM is + * properly inserted in 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); +}