From: Christopher Faulet Date: Wed, 1 Jul 2026 13:11:06 +0000 (+0200) Subject: BUG/MAJOR: htx: Don't swap buffers for empty HTX message with an error X-Git-Url: http://git.kaiwu.me/%22../static/gitweb.js?a=commitdiff_plain;h=80e0004c91fef597b4b9c139fb0085ece88d721e;p=haproxy.git BUG/MAJOR: htx: Don't swap buffers for empty HTX message with an error Recent fix of some HTX muxes to drain remaining data when the stream is in closed state revealed a bug, mainly due to a corner case of the HTX API. It is possible to have an empty HTX message with a parsing/internal error. In that case, the underlying buffer remains full. It is mandatory to prevent any buffer release and be sure the error will be handeled. On the other end, at several places, when data must be transfer from an HTX message to another one, we try to swap underlying buffers instead of performing a bloc-per-bloc copy. To do so, we rely on b_xfer() function. One condition is that the destination message must be empty. And here is the issue. The HTX message can be empty but the buffer can also be full because an HTX error was triggered earlier and not handled yet. In that case, attempting to call b_xfer() leads to a crash because the destination buffer is full. It is not expected to call b_xfer() if there is not enough space in the destination buffer. So, it appears the HTX API should be improved/fixed but first of all, the bug must be fixed. Especially because stable versions are also affected. The htx_is_empty_noerr() function was added to know if a HTX message is empty and no error was reported on it. And this function is now used, instead of htx_is_empty(), to know if we can safely swap the underlying buffers or not. the FCGI, H2 and QUIC multiplexers are concerned. The HTTP client and the applet API were also fixed while it seems harder to trigger the bug at these places. The fix must be backported to all supported versions. --- diff --git a/include/haproxy/htx.h b/include/haproxy/htx.h index 6497e8337..336db649f 100644 --- a/include/haproxy/htx.h +++ b/include/haproxy/htx.h @@ -776,6 +776,14 @@ static inline int htx_is_not_empty(const struct htx *htx) return (htx->head != -1); } +/* Return 1 if the message is empty and there is no parsing/internal error flag + * set. Otherwise it return 0. + */ +static inline int htx_is_empty_noerr(const struct htx *htx) +{ + return (htx_is_empty(htx) && !(htx->flags & (HTX_FL_PARSING_ERROR|HTX_FL_PROCESSING_ERROR))); +} + /* Returns 1 if no more data are expected for the message . Otherwise it * returns 0. Note that it is illegal to call this with htx == NULL. This * function relies on the HTX_FL_EOM flags. It means tunneled data are not diff --git a/src/applet.c b/src/applet.c index 6e5188404..db30df612 100644 --- a/src/applet.c +++ b/src/applet.c @@ -497,14 +497,14 @@ size_t appctx_htx_rcv_buf(struct appctx *appctx, struct buffer *buf, size_t coun struct htx *buf_htx = NULL; size_t ret = 0; - if (htx_is_empty(appctx_htx)) { + if (htx_is_empty_noerr(appctx_htx)) { htx_to_buf(appctx_htx, &appctx->outbuf); goto out; } ret = appctx_htx->data; buf_htx = htx_from_buf(buf); - if (b_size(&appctx->outbuf) == b_size(buf) && htx_is_empty(buf_htx) && htx_used_space(appctx_htx) <= count) { + if (b_size(&appctx->outbuf) == b_size(buf) && htx_is_empty_noerr(buf_htx) && htx_used_space(appctx_htx) <= count) { htx_to_buf(buf_htx, buf); htx_to_buf(appctx_htx, &appctx->outbuf); b_xfer(buf, &appctx->outbuf, b_data(&appctx->outbuf)); @@ -599,7 +599,7 @@ size_t appctx_htx_snd_buf(struct appctx *appctx, struct buffer *buf, size_t coun size_t ret = 0; ret = buf_htx->data; - if (htx_is_empty(appctx_htx) && buf_htx->data == count) { + if (htx_is_empty_noerr(appctx_htx) && buf_htx->data == count) { htx_to_buf(appctx_htx, &appctx->inbuf); htx_to_buf(buf_htx, buf); b_xfer(&appctx->inbuf, buf, b_data(buf)); diff --git a/src/http_client.c b/src/http_client.c index c1aa99404..157172242 100644 --- a/src/http_client.c +++ b/src/http_client.c @@ -605,11 +605,11 @@ void httpclient_applet_io_handler(struct appctx *appctx) hc->ops.req_payload(hc); hc_htx = htxbuf(&hc->req.buf); - if (htx_is_empty(hc_htx)) + if (htx_is_empty_noerr(hc_htx)) goto out; htx = htx_from_buf(outbuf); - if (htx_is_empty(htx)) { + if (htx_is_empty_noerr(htx)) { /* Here htx_to_buf() will set buffer data to 0 because * the HTX is empty, and allow us to do an xfer. */ diff --git a/src/mux_h2.c b/src/mux_h2.c index 50656eb44..8cceb3e0d 100644 --- a/src/mux_h2.c +++ b/src/mux_h2.c @@ -7960,7 +7960,7 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in goto end; // may be NULL if empty h2s_htx = htx_from_buf(rxbuf); - if (htx_is_empty(h2s_htx) && !(h2s_htx->flags & HTX_FL_PARSING_ERROR)) { + if (htx_is_empty_noerr(h2s_htx)) { /* Here htx_to_buf() will set buffer data to 0 because * the HTX is empty. */ @@ -7972,7 +7972,7 @@ static size_t h2_rcv_buf(struct stconn *sc, struct buffer *buf, size_t count, in /* is empty and the message is small enough, swap the * buffers. */ - if (htx_is_empty(buf_htx) && htx_used_space(h2s_htx) <= count) { + if (htx_is_empty_noerr(buf_htx) && htx_used_space(h2s_htx) <= count) { count -= h2s_htx->data; htx_to_buf(buf_htx, buf); htx_to_buf(h2s_htx, rxbuf); diff --git a/src/qcm_http.c b/src/qcm_http.c index c2f0ae1eb..f028cb992 100644 --- a/src/qcm_http.c +++ b/src/qcm_http.c @@ -22,7 +22,7 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count, *fin = 0; qcs_htx = htx_from_buf(&qcs->rx.app_buf); - if (htx_is_empty(qcs_htx)) { + if (htx_is_empty_noerr(qcs_htx)) { /* Set buffer data to 0 as HTX is empty. */ htx_to_buf(qcs_htx, &qcs->rx.app_buf); goto end; @@ -31,7 +31,7 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count, ret = qcs_htx->data; cs_htx = htx_from_buf(buf); - if (htx_is_empty(cs_htx) && htx_used_space(qcs_htx) <= count) { + if (htx_is_empty_noerr(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) && !(qcs->flags & QC_SF_EOI_SUSPENDED)) {