From: Amaury Denoyelle Date: Fri, 11 Jul 2025 14:06:51 +0000 (+0200) Subject: BUG/MINOR: h3: properly handle interim response on BE side X-Git-Tag: v3.3-dev4~76 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=63586a8ab4eccc9b63af406d56ae12d639d94b45;p=thirdparty%2Fhaproxy.git BUG/MINOR: h3: properly handle interim response on BE side On backend side, H3 layer is responsible to decode a HTTP/3 response into an HTX message. Multiple responses may be received on a single stream with interim status codes prior to the final one. h3_resp_headers_to_htx() is the function used solely on backend side responsible for H3 response to HTX transcoding. This patch extends it to be able to properly support interim responses. When such a response is received, the new flag H3_SF_RECV_INTERIM is set. This is converted to QMUX qcs flag QC_SF_EOI_SUSPENDED. The objective of this latter flag is to prevent stream EOI to be reported during stream rcv_buf callback, even if HTX message contains EOM and is empty. QC_SF_EOI_SUSPENDED will be cleared when the final response is finally converted, which unblock stream EOI notification for next rcv_buf invocations. Note however that HTX EOM is untouched : it is always set for both interim and final response reception. As a minor adjustment, HTX_SL_F_BODYLESS is always set for interim responses. Contrary to frontend interim response handling, a flag is necessary on QMUX layer. This is because H3 to HTX transcoding and rcv_buf callback are two distinct operations, called under different context (MUX vs stream tasklet). Also note that H3 layer has two distinct flags for interim response handling, one only used as a server (FE side) and the other as a client (BE side). It was preferred to used two distinct flags which is considered less error-prone, contrary to a single unified flag which would require to always set the proxy side to ensure it is relevant or not. No need to backport. --- diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h index 6b7f1218a..906b383f2 100644 --- a/include/haproxy/mux_quic-t.h +++ b/include/haproxy/mux_quic-t.h @@ -275,6 +275,7 @@ static forceinline char *qcc_show_flags(char *buf, size_t len, const char *delim #define QC_SF_TO_STOP_SENDING 0x00000200 /* a STOP_SENDING must be sent */ #define QC_SF_UNKNOWN_PL_LENGTH 0x00000400 /* HTX EOM may be missing from the stream layer */ #define QC_SF_RECV_RESET 0x00000800 /* a RESET_STREAM was received */ +#define QC_SF_EOI_SUSPENDED 0x00001000 /* EOI must not be reported even if HTX EOM is present - useful when transferring HTTP interim responses */ /* This function is used to report flags in debugging tools. Please reflect * below any single-bit flag addition above in the same order via the diff --git a/src/h3.c b/src/h3.c index f3267db01..c93752334 100644 --- a/src/h3.c +++ b/src/h3.c @@ -154,6 +154,7 @@ DECLARE_STATIC_POOL(pool_head_h3c, "h3c", sizeof(struct h3c)); #define H3_SF_UNI_NO_H3 0x00000002 /* unidirectional stream does not carry H3 frames */ #define H3_SF_HAVE_CLEN 0x00000004 /* content-length header is present; relevant either for request or response depending on the side of the connection */ #define H3_SF_SENT_INTERIM 0x00000008 /* last response sent is 1xx interim. Used on FE side only. */ +#define H3_SF_RECV_INTERIM 0x00000010 /* last response sent is 1xx interim. Used on BE side only. */ struct h3s { struct h3c *h3c; @@ -1092,6 +1093,9 @@ static ssize_t h3_req_headers_to_htx(struct qcs *qcs, const struct buffer *buf, * transcoded into HTX buffer of stream. must be set if this is the * last data to transfer for this stream. * + * If the response handled was an interim one (1xx code), H3_SF_RECV_INTERIM + * flag will be set on h3 stream instance. + * * Returns the number of consumed bytes or a negative error code. */ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf, @@ -1241,10 +1245,9 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf, goto out; } - if (fin) - sl->flags |= HTX_SL_F_BODYLESS; - sl->info.res.status = h * 100 + t * 10 + u; + if (fin || (sl->info.res.status >= 100 && sl->info.res.status < 200)) + sl->flags |= HTX_SL_F_BODYLESS; /* now treat standard headers */ while (1) { @@ -1319,6 +1322,17 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf, if (fin) htx->flags |= HTX_FL_EOM; + /* Detect if an interim or final response was handled. */ + if (sl->info.res.status >= 100 && sl->info.res.status < 200) { + TRACE_USER("receiving interim response", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + BUG_ON(!conn_is_back(qcs->qcc->conn)); /* H3_SF_RECV_INTERIM is BE side only. */ + h3s->flags |= H3_SF_RECV_INTERIM; + } + else { + TRACE_USER("receiving final response", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs); + h3s->flags &= ~H3_SF_RECV_INTERIM; + } + out: if (appbuf) htx_to_buf(htx, appbuf); @@ -1794,10 +1808,24 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin) break; case H3_FT_HEADERS: if (h3s->st_req == H3S_ST_REQ_BEFORE) { - ret = !conn_is_back(qcs->qcc->conn) ? - h3_req_headers_to_htx(qcs, b, flen, last_stream_frame) : - h3_resp_headers_to_htx(qcs, b, flen, last_stream_frame); - h3s->st_req = H3S_ST_REQ_HEADERS; + if (!conn_is_back(qcs->qcc->conn)) { + ret = h3_req_headers_to_htx(qcs, b, flen, last_stream_frame); + h3s->st_req = H3S_ST_REQ_HEADERS; + } + else { + ret = h3_resp_headers_to_htx(qcs, b, flen, last_stream_frame); + + /* Check if an interim or final response was parsed. */ + if (h3s->flags & H3_SF_RECV_INTERIM) { + /* Interim resp: do not advance . */ + qcs->flags |= QC_SF_EOI_SUSPENDED; + } + else { + /* Final resp: mark as headers received. */ + h3s->st_req = H3S_ST_REQ_HEADERS; + qcs->flags &= ~QC_SF_EOI_SUSPENDED; + } + } } else { ret = h3_trailers_to_htx(qcs, b, flen, last_stream_frame); diff --git a/src/qmux_http.c b/src/qmux_http.c index 57b1eef35..9a310a397 100644 --- a/src/qmux_http.c +++ b/src/qmux_http.c @@ -33,8 +33,10 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count, cs_htx = htx_from_buf(buf); if (htx_is_empty(cs_htx) && htx_used_space(qcs_htx) <= count) { /* EOM will be copied to cs_htx via b_xfer(). */ - if (qcs_htx->flags & HTX_FL_EOM) + if ((qcs_htx->flags & HTX_FL_EOM) && + !(qcs->flags & QC_SF_EOI_SUSPENDED)) { *fin = 1; + } htx_to_buf(cs_htx, buf); htx_to_buf(qcs_htx, &qcs->rx.app_buf); @@ -48,7 +50,8 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count, /* Copy EOM from src to dst buffer if all data copied. */ if (htx_is_empty(qcs_htx) && (qcs_htx->flags & HTX_FL_EOM)) { cs_htx->flags |= HTX_FL_EOM; - *fin = 1; + if (!(qcs->flags & QC_SF_EOI_SUSPENDED)) + *fin = 1; } cs_htx->extra = qcs_htx->extra ? (qcs_htx->data + qcs_htx->extra) : 0;