BUG/MINOR: h3: fix missing EOM after trailers without FIN

In HTTP/3, it is possible to receive a trailers frame without STREAM FIN
set. It is forbidden for the peer to emit any other HEADERS or DATA
frame, however it is possible to close the stream via an empty STREAM
FIN, or even using HTTP/3 extension frames which must be ignored by a
parser. In this case, EOT will be inserted without EOM flag. If it is
consumed by the stream layer prior to the final EOM reporting, a second
EOT will be inserted, resulting in an invalid message rejected by the
MUX from the other side.

To prevent this situation, disable EOT emission in case EOM cannot be
set. This is implemented via the new argument recently added to
http_trailers_to_htx().

Now that EOT trailers can be voluntarily delayed, it is necessary that
it will be inserted, which htx_set_eom() does not perform if the HTX
message is not empty. This could result with EOM set on the last HTX
headers block instead. Thus, HTTP/3 parser forces EOT emission when
trailers were already parsed, via a new argument on
qcs_http_handle_standalone_fin().

This must be backported up to 2.8. This relies on the previous
refactoring patch. Also note that http_trailers_to_htx() only exist in
master branch, as previously EOT block was added directly by h3 parser
in h3_trailers_to_htx().

diff --git a/include/haproxy/qcm_http.h b/include/haproxy/qcm_http.h
index 85de625eb8ed33aa8c7e98271edab6071a9a9174..c72428cb8b8084dff4ed58b1c72f7d6aab547bee 100644 (file)
--- a/include/haproxy/qcm_http.h
+++ b/include/haproxy/qcm_http.h
@@ -9,7 +9,7 @@
 size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count,
                         char *fin);
 
-int qcs_http_handle_standalone_fin(struct qcs *qcs);
+int qcs_http_handle_standalone_fin(struct qcs *qcs, int force_eot);
 
 size_t qcs_http_snd_buf(struct qcs *qcs, struct buffer *buf, size_t count,
                         char *fin);

diff --git a/src/h3.c b/src/h3.c
index 33fb55b077a4ab62166f705fcc3a5fb0ba95515a..0040b35799fcc70a254f111c23a83e5edcebf2e4 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -1634,7 +1634,7 @@ static ssize_t h3_trailers_to_htx(struct qcs *qcs, const struct buffer *buf,
                goto out;
        }
 
-       prs_status = http_trailers_to_htx(list, 1, htx, (qcs->qcc->flags & QC_CF_IS_BACK) ? HTTP_PF_DIR_RES : 0);
+       prs_status = http_trailers_to_htx(list, fin, htx, (qcs->qcc->flags & QC_CF_IS_BACK) ? HTTP_PF_DIR_RES : 0);
        switch (prs_status) {
        case HTTP_PRS_SUCCESS:
                break;
@@ -2020,7 +2020,7 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        goto done;
                }
 
-               if (quic_stream_is_bidi(qcs->id) && qcs_http_handle_standalone_fin(qcs)) {
+               if (quic_stream_is_bidi(qcs->id) && qcs_http_handle_standalone_fin(qcs, h3s->st_req == H3S_ST_REQ_TRAILERS)) {
                        TRACE_ERROR("cannot set EOM", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
                        qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1,
                                      muxc_tevt_type_internal_err);
@@ -2335,7 +2335,7 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                                }
 
                                /* Reuse wrapper function to easily set EOM on HTX message. */
-                               if (qcs_http_handle_standalone_fin(qcs)) {
+                               if (qcs_http_handle_standalone_fin(qcs, h3s->st_req == H3S_ST_REQ_TRAILERS)) {
                                        TRACE_ERROR("cannot set EOM", H3_EV_RX_FRAME, qcs->qcc->conn, qcs);
                                        qcc_set_error(qcs->qcc, H3_ERR_INTERNAL_ERROR, 1,
                                                      muxc_tevt_type_internal_err);

diff --git a/src/hq_interop.c b/src/hq_interop.c
index 9ffe2cb00624ea5ecee8506d24b1d1dd1f4bddef..c9faa8581ee988abbaa041892664bceb156c608e 100644 (file)
--- a/src/hq_interop.c
+++ b/src/hq_interop.c
@@ -39,7 +39,7 @@ static ssize_t hq_interop_rcv_buf_req(struct qcs *qcs, struct buffer *b, int fin
        BUG_ON(b_data(b) != b_contig_data(b, 0));
 
        if (!b_data(b) && fin && quic_stream_is_bidi(qcs->id)) {
-               if (qcs_http_handle_standalone_fin(qcs))
+               if (qcs_http_handle_standalone_fin(qcs, 0))
                        return -1;
                return 0;
        }
@@ -163,7 +163,7 @@ static ssize_t hq_interop_rcv_buf_res(struct qcs *qcs, struct buffer *b, int fin
 
        if (!to_copy) {
                if (fin && quic_stream_is_bidi(qcs->id)) {
-                       if (qcs_http_handle_standalone_fin(qcs)) {
+                       if (qcs_http_handle_standalone_fin(qcs, 0)) {
                                htx_to_buf(htx, htx_buf);
                                return -1;
                        }

diff --git a/src/qcm_http.c b/src/qcm_http.c
index e7f469d1f572f5d7c2f3e280e7fe48e3a4e4a278..8eea0dad363572d16d78e133d06fd576b9fe10f8 100644 (file)
--- a/src/qcm_http.c
+++ b/src/qcm_http.c
@@ -62,10 +62,13 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count,
        return ret;
 }
 
-/* Add EOM flag on input buffer for <qcs> instance.
+/* Add EOM flag on input buffer for <qcs> instance. If <force_eot> is set, EOT
+ * is manually inserted. This is necessary if HTX message contains not yet
+ * finalized trailers blocks.
+ *
  * Returns 0 on success else non zero on internal error.
  */
-int qcs_http_handle_standalone_fin(struct qcs *qcs)
+int qcs_http_handle_standalone_fin(struct qcs *qcs, int force_eot)
 {
        struct buffer *appbuf;
        struct htx *htx = NULL;
@@ -74,6 +77,9 @@ int qcs_http_handle_standalone_fin(struct qcs *qcs)
                goto err;
        htx = htx_from_buf(appbuf);
 
+       if (force_eot && !htx_add_endof(htx, HTX_BLK_EOT))
+               goto err;
+
        if (!htx_set_eom(htx))
                goto err;