From: Christopher Faulet Date: Tue, 8 Sep 2026 05:33:18 +0000 (+0200) Subject: MAJOR: tree-wide: No longer use HTX_FL_HAS_EOM to detect end of message X-Git-Tag: v3.5-dev8~158 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=40e5dff61a1737be042b4a10b6dba7b517660f0d;p=haproxy.git MAJOR: tree-wide: No longer use HTX_FL_HAS_EOM to detect end of message HTX_FL_HAS_EOM flag on the HTX message is no longer used to mark or detect the end of the message. htx_has_eom() function is used to detect it and htx_set_eom() is used to set it (except inside the HTX API). Thus we now rely on flags on the tail block to deal with the end of the message. Most of time, the change is quite light and easy to review. However there are 3 parts no so obvious: * The decompression filter: DECOMP_STATE_EOM_SEEN flag is now set only when a block is removed. It is the only way to loose the EOM. This part was placed in a dedicated function to not duplicate the code. At the end, before exiting and when the decompression is finished, the EOM flag is added on the tail block if necessary. * The compression filter: As for the decompression filter, the end of message is only evaluated when an HTX block is removed. It is the only way to loose it. And we try to restore it only on the last DATA block. * The lua filter API: The internal function used to remove part of the payload was updated to be sure to now loos the EOM on the tail block if it is removed. In the HTX API, HTX_FL_HAS_EOM flag is now longet set. --- diff --git a/src/fcgi-app.c b/src/fcgi-app.c index 79c1f8e57..9b785b964 100644 --- a/src/fcgi-app.c +++ b/src/fcgi-app.c @@ -343,7 +343,7 @@ static int fcgi_flt_http_headers(struct stream *s, struct filter *filter, struct sl = http_get_stline(htx); if (s->txn.http->meth != HTTP_METH_HEAD && sl && (msg->flags & (HTTP_MSGF_XFER_LEN|HTTP_MSGF_CNT_LEN|HTTP_MSGF_TE_CHNK)) == HTTP_MSGF_XFER_LEN && - (htx->flags & HTX_FL_HAS_EOM)) { + htx_has_eom(htx)) { struct htx_blk * blk; char *end; size_t len = 0; diff --git a/src/flt_decomp.c b/src/flt_decomp.c index d09b650a4..d4294456c 100644 --- a/src/flt_decomp.c +++ b/src/flt_decomp.c @@ -181,6 +181,35 @@ static int decomp_stream_blk_drain(struct stream *s, struct filter *f, struct ch return drainable; } +/* Skips bytes, consumed by the decompression at the beginning of the HTX + * block <*blk>, and updates the filter offsets accordingly. Because it is not + * supported to leave an empty HTX data block, the block is removed if all its + * data are skipped. In this case, <*blk> is set to NULL and <*next> is set to + * the following block, if any, to tell the caller not to look for it by + * itself. Otherwise, <*blk> is updated to point on the possibly reallocated + * block and <*next> is left untouched. + * + * The EOM flag, if any, is saved in the filter context because it is lost when + * the last block of an empty message is removed. It is restored once the + * decompression is finished. + */ +static void decomp_stream_blk_skip(struct filter *f, struct channel *chn, struct htx *htx, + struct htx_blk **blk, struct htx_blk **next, struct ist data) +{ + struct decomp_state *st = f->ctx; + + flt_update_offsets(f, chn, -data.len); + + if (data.len == htx_get_blksz(*blk)) { + if ((*blk)->flags & HTX_BLK_FL_EOM) + st->flags |= DECOMP_STATE_EOM_SEEN; + *next = htx_remove_blk(htx, *blk); + *blk = NULL; // make sure we don't use the old block + } + else + *blk = htx_replace_blk_value(htx, *blk, data, ist2("", 0)); +} + /* Reads input stream and decompress as many bytes as possible which will be removed * from the stream, then try to write as many decoded bytes as possible in the stream. */ @@ -206,15 +235,6 @@ static int decomp_stream_blk(struct stream *s, struct filter *f, struct channel offset = htxret.ret; blk = htxret.blk; - /* Remove EOM flag from HTX message to be sure to not expose it too - * early to next filters. But save the information to be able to restore - * the flag at the end of the decompression. - */ - if (htx->flags & HTX_FL_HAS_EOM) { - st->flags |= DECOMP_STATE_EOM_SEEN; - htx->flags &= ~HTX_FL_HAS_EOM; - } - while (blk && len) { blk_remain = 0; /* next is special variable that must be set to NULL, it will be used at the @@ -260,17 +280,7 @@ static int decomp_stream_blk(struct stream *s, struct filter *f, struct channel * happen if the input is not garbage filled and * decompression library consumed all data */ - flt_update_offsets(f, chn, -val.len); - - /* skip consumed bytes: remove the block if empty due to skipping - * bytes because it is not supported to leave empty htx data block - */ - if (val.len == htx_get_blksz(blk)) { - next = htx_remove_blk(htx, blk); - blk = NULL; // make sure we don't use the old block - } - else - blk = htx_replace_blk_value(htx, blk, ist2(val.ptr, val.len), ist2("", 0)); + decomp_stream_blk_skip(f, chn, htx, &blk, &next, val); blk_remain -= val.len; len -= val.len; @@ -301,17 +311,7 @@ static int decomp_stream_blk(struct stream *s, struct filter *f, struct channel } } else { - flt_update_offsets(f, chn, -consumed); - - /* skip consumed bytes: remove the block if empty due to skipping - * bytes because it is not supported to leave empty htx data block - */ - if (consumed == htx_get_blksz(blk)) { - next = htx_remove_blk(htx, blk); - blk = NULL; // make sure we don't use the old block - } - else - blk = htx_replace_blk_value(htx, blk, ist2(val.ptr, consumed), ist2("", 0)); + decomp_stream_blk_skip(f, chn, htx, &blk, &next, ist2(val.ptr, consumed)); } break; @@ -398,11 +398,10 @@ static int decomp_stream_blk(struct stream *s, struct filter *f, struct channel if ((st->decomp_ctx->flags & DECOMP_CTX_FL_DONE) && !st->decomp_ctx->drain_len) { st->flags &= ~DECOMP_STATE_PROCESSING; - /* The decompression is finished and we must now restore the EOM - * flag on the HTX message. We must also take care to forward - * any EOT block added to support the EOM flag. + /* The decompression is finished and we must now be sure to + * restore EOM on last block (possibly by adding an EOT). */ - if (st->flags & DECOMP_STATE_EOM_SEEN) { + if ((st->flags & DECOMP_STATE_EOM_SEEN) && !htx_has_eom(htx)) { unsigned int data = htx->data; BUG_ON(!(st->decomp_ctx->flags & DECOMP_CTX_FL_DONE) || st->decomp_ctx->drain_len); @@ -410,14 +409,12 @@ static int decomp_stream_blk(struct stream *s, struct filter *f, struct channel total += htx->data - data; } } + end: htx_to_buf(htx, &chn->buf); return total; error: - /* On error, restore HTX_FL_HAS_EOM flag */ - if (st->flags & DECOMP_STATE_EOM_SEEN) - htx_set_eom(htx); htx_to_buf(htx, &chn->buf); return -1; } diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c index e02e85eb8..7400be7b7 100644 --- a/src/flt_http_comp.c +++ b/src/flt_http_comp.c @@ -26,7 +26,6 @@ #include #define COMP_STATE_PROCESSING 0x01 -#define COMP_STATE_EOM_SEEN 0x02 const char *http_comp_req_flt_id = "comp-req filter"; const char *http_comp_res_flt_id = "comp-res filter"; @@ -129,7 +128,7 @@ comp_prepare_compress_request(struct comp_state *st, struct stream *s, struct ht comp_type = NULL; /* compress only if body size is >= than the min size */ - if (((msg->flags & HTTP_MSGF_CNT_LEN) || (htx->flags & HTX_FL_HAS_EOM)) && + if (((msg->flags & HTTP_MSGF_CNT_LEN) || htx_has_eom(htx)) && ((s->be->comp && (comp_minsize = s->be->comp->minsize_req)) || (strm_fe(s)->comp && (comp_minsize = strm_fe(s)->comp->minsize_req)))) { /* small requests should not be compressed */ @@ -287,15 +286,6 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, blk = htxret.blk; offset = htxret.ret; - /* Remove EOM flag from HTX message to be sure to not expose it too - * early to next filters. But save the information to be able to restore - * the flag at the end of the compression. - */ - if (htx->flags & HTX_FL_HAS_EOM) { - st->flags |= COMP_STATE_EOM_SEEN; - htx->flags &= ~HTX_FL_HAS_EOM; - } - for (next = NULL; blk && len; blk = next) { enum htx_blk_type type = htx_get_blk_type(blk); uint32_t sz = htx_get_blksz(blk); @@ -340,8 +330,22 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, len -= ret; consumed += ret; to_forward += b_data(&trash); - if (last) + if (last) { st->flags &= ~COMP_STATE_PROCESSING; + + /* The compression is finished. The EOM flag was + * carried by this block. It was lost if the block + * was removed and was the last one of the + * message. In this case, it must be restored, + * adding an EOT block to support it. + */ + if (!htx_has_eom(htx)) { + unsigned int data = htx->data; + + htx_set_eom(htx); + to_forward += htx->data - data; + } + } break; case HTX_BLK_TLR: @@ -376,16 +380,6 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, } end: - /* The compression is finished and we must now restore the EOM flog on - * the HTX message. - */ - if ((st->flags & (COMP_STATE_PROCESSING|COMP_STATE_EOM_SEEN)) == COMP_STATE_EOM_SEEN) { - unsigned int data = htx->data; - - htx_set_eom(htx); - to_forward += htx->data - data; - } - if (to_forward != consumed) flt_update_offsets(filter, msg->chn, to_forward - consumed); @@ -409,9 +403,6 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, return to_forward; error: - /* On error, restore HTX_FL_HAS_EOM flag */ - if (st->flags & COMP_STATE_EOM_SEEN) - htx->flags |= HTX_FL_HAS_EOM; return -1; } @@ -695,7 +686,7 @@ select_compression_response_header(struct comp_state *st, struct stream *s, stru goto fail; /* compress only if body size is >= than the min size */ - if (((msg->flags & HTTP_MSGF_CNT_LEN) || (htx->flags & HTX_FL_HAS_EOM)) && + if (((msg->flags & HTTP_MSGF_CNT_LEN) || htx_has_eom(htx)) && ((s->be->comp && (comp_minsize = s->be->comp->minsize_res)) || (strm_fe(s)->comp && (comp_minsize = strm_fe(s)->comp->minsize_res)))) { /* small responses should not be compressed */ diff --git a/src/haload.c b/src/haload.c index 9a1a83b64..024afc56c 100644 --- a/src/haload.c +++ b/src/haload.c @@ -1228,7 +1228,7 @@ static int hldstream_build_http_req(struct hldstream *hs, struct ist path) } /* Continue sending the POST body once the previous output buffer was fully - * drained but hs->to_send is still > 0. Sets HTX_FL_HAS_EOM once it reaches 0. + * drained but hs->to_send is still > 0. Sets EOM once it reaches 0. * Return 1 if succeeded, 0 if not. */ static int hldstream_build_http_body(struct hldstream *hs) diff --git a/src/hlua.c b/src/hlua.c index b9c7beaaa..0d38e262c 100644 --- a/src/hlua.c +++ b/src/hlua.c @@ -7416,6 +7416,12 @@ static void _hlua_http_msg_delete(struct http_msg *msg, struct filter *filter, s struct htx_blk *blk; struct htx_ret htxret; size_t ret = 0; + int eom; + + /* Removed blocks may carry the EOM flag. In this case, the flag must be + * set again on the last remaining block to keep the message complete. + */ + eom = htx_has_eom(htx); /* Be sure is always the amount of DATA to remove */ if (htx->data == offset+len && htx_get_tail_type(htx) == HTX_BLK_DATA) { @@ -7487,10 +7493,20 @@ static void _hlua_http_msg_delete(struct http_msg *msg, struct filter *filter, s } end: + if (eom && !htx_has_eom(htx)) { + unsigned int data = htx->data; + + /* Restore the EOM flag. An EOT block may be added to carry it if + * the message is now empty. In this case, it must be accounted + * for as inserted data. + */ + htx_set_eom(htx); + ret -= (htx->data - data); + } flt_update_offsets(filter, msg->chn, -ret); flt_ctx->cur_len[CHN_IDX(msg->chn)] -= ret; - /* WARNING: we don't call htx_to_buf() on purpose, because we don't want - * to loose the EOM flag if the message is empty. + /* WARNING: we don't call htx_to_buf() on purpose, because the caller may + * still add data in the message. */ } @@ -7914,7 +7930,8 @@ __LJMP static int hlua_http_msg_unset_eom(lua_State *L) MAY_LJMP(check_args(L, 1, "set_eom")); msg = MAY_LJMP(hlua_checkhttpmsg(L, 1)); htx = htxbuf(&msg->chn->buf); - htx->flags &= ~HTX_FL_HAS_EOM; + if (!htx_has_eom(htx)) + return 0; for (blk = htx_get_tail_blk(htx); blk; blk = htx_get_prev_blk(htx, blk)) { if (blk->flags & HTX_BLK_FL_EOM) { diff --git a/src/hq_interop.c b/src/hq_interop.c index 81eaf5dd9..7b45e22fe 100644 --- a/src/hq_interop.c +++ b/src/hq_interop.c @@ -240,7 +240,7 @@ static size_t hq_interop_snd_buf(struct qcs *qcs, struct buffer *buf, *fin = 0; htx = htx_from_buf(buf); /* EOM is saved here, useful if 0-copy is performed with HTX buf. */ - eom = htx->flags & HTX_FL_HAS_EOM; + eom = htx_has_eom(htx); while (count && !htx_is_empty(htx) && qcc_stream_can_send(qcs)) { /* Not implemented : QUIC on backend side */ diff --git a/src/http_ana.c b/src/http_ana.c index 344926e8d..42c6bd505 100644 --- a/src/http_ana.c +++ b/src/http_ana.c @@ -743,7 +743,7 @@ int http_process_request(struct stream *s, struct channel *req, int an_bit) * in case we previously disabled it, otherwise we might cause * the client to delay further data. */ - if ((sess->listener && (sess->listener->bind_conf->options & BC_O_NOQUICKACK)) && !(htx->flags & HTX_FL_HAS_EOM)) + if ((sess->listener && (sess->listener->bind_conf->options & BC_O_NOQUICKACK)) && !htx_has_eom(htx)) conn_set_quickack(cli_conn, 1); /************************************************************* @@ -992,7 +992,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) else { c_adv(req, htx->data - co_data(req)); if ((global.tune.options & GTUNE_USE_FAST_FWD) && (msg->flags & HTTP_MSGF_XFER_LEN) && - (!(msg->flags & HTTP_MSGF_CONN_UPG) || (htx->flags & HTX_FL_HAS_EOM))) + (!(msg->flags & HTTP_MSGF_CONN_UPG) || htx_has_eom(htx))) channel_htx_forward_forever(req, htx); } @@ -1003,7 +1003,7 @@ int http_request_forward_body(struct stream *s, struct channel *req, int an_bit) * in HTTP_MSG_ENDING state. Then if all data was marked to be * forwarded, set the state to HTTP_MSG_DONE. */ - if (!(htx->flags & HTX_FL_HAS_EOM)) + if (!htx_has_eom(htx)) goto missing_data_or_waiting; msg->msg_state = HTTP_MSG_ENDING; @@ -2229,7 +2229,7 @@ int http_response_forward_body(struct stream *s, struct channel *res, int an_bit * in HTTP_MSG_ENDING state. Then if all data was marked to be * forwarded, set the state to HTTP_MSG_DONE. */ - if (!(htx->flags & HTX_FL_HAS_EOM)) + if (!htx_has_eom(htx)) goto missing_data_or_waiting; msg->msg_state = HTTP_MSG_ENDING; @@ -4372,7 +4372,7 @@ enum rule_result http_wait_for_msg_body(struct stream *s, struct channel *chn, /* Now we're are waiting for the payload. We just need to know if all * data have been received or if the buffer is full. */ - if ((htx->flags & HTX_FL_HAS_EOM) || htx_get_tail_type(htx) > HTX_BLK_DATA) + if (htx_has_eom(htx) || htx_get_tail_type(htx) > HTX_BLK_DATA) goto end; /* all data received */ if (bytes) { diff --git a/src/http_client.c b/src/http_client.c index 4fec3f069..4bb9e1bac 100644 --- a/src/http_client.c +++ b/src/http_client.c @@ -533,6 +533,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) struct htx *htx; struct htx_sl *sl = NULL; uint32_t hdr_num; + int res_has_eom = 0; int ret; if (unlikely(applet_fl_test(appctx, APPCTX_FL_EOS|APPCTX_FL_ERROR))) { @@ -576,7 +577,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) goto out; } - if (htx->flags & HTX_FL_HAS_EOM) { /* check if a body need to be added */ + if (htx_has_eom(htx)) { /* check if a body need to be added */ appctx->st0 = HTTPCLIENT_S_RES_STLINE; applet_set_eoi(appctx); goto out; /* we need to leave the IO handler once we wrote the request */ @@ -638,8 +639,8 @@ void httpclient_applet_io_handler(struct appctx *appctx) htx = htxbuf(outbuf); - /* if the request contains the HTX_FL_HAS_EOM, we finished the request part. */ - if (htx->flags & HTX_FL_HAS_EOM) { + /* if the request contains the EOM, we finished the request part. */ + if (htx_has_eom(htx)) { appctx->st0 = HTTPCLIENT_S_RES_STLINE; applet_set_eoi(appctx); goto out; /* we need to leave the IO handler once we wrote the request */ @@ -667,6 +668,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) htx = htxbuf(inbuf); if (htx_get_head_type(htx) != HTX_BLK_RES_SL) goto error; + res_has_eom = htx_has_eom(htx); blk = DISGUISE(htx_get_head_blk(htx)); sl = htx_get_blk_ptr(htx, blk); @@ -704,7 +706,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) /* if there is no HTX data anymore and the EOM flag is * set, leave (no body) */ - if (htx_is_empty(htx) && htx->flags & HTX_FL_HAS_EOM) + if (htx_is_empty(htx) && res_has_eom) appctx->st0 = HTTPCLIENT_S_RES_END; else appctx->st0 = HTTPCLIENT_S_RES_HDR; @@ -723,7 +725,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) htx = htxbuf(inbuf); BUG_ON(htx_is_empty(htx)); - + res_has_eom = htx_has_eom(htx); if (hc->options & HTTPCLIENT_O_RES_HTX) { /* HTX mode transfers the header to the hc buffer */ struct htx *hc_htx; @@ -787,7 +789,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) /* if there is no HTX data anymore and the EOM flag is * set, leave (no body) */ - if (htx_is_empty(htx) && htx->flags & HTX_FL_HAS_EOM) { + if (htx_is_empty(htx) && res_has_eom) { appctx->st0 = HTTPCLIENT_S_RES_END; } else { appctx->st0 = HTTPCLIENT_S_RES_BODY; @@ -812,6 +814,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) applet_need_more_data(appctx); goto out; } + res_has_eom = htx_has_eom(htx); if (!b_alloc(&hc->res.buf, DB_MUX_TX)) { applet_wont_consume(appctx); @@ -869,7 +872,7 @@ void httpclient_applet_io_handler(struct appctx *appctx) hc->ops.res_payload(hc); /* if not finished, should be called again */ - if ((htx_is_empty(htx) && (htx->flags & HTX_FL_HAS_EOM))) { + if (htx_is_empty(htx) && res_has_eom) { appctx->st0 = HTTPCLIENT_S_RES_END; htx_to_buf(htx, inbuf); break; diff --git a/src/http_htx.c b/src/http_htx.c index fa15875b5..62814772c 100644 --- a/src/http_htx.c +++ b/src/http_htx.c @@ -2831,7 +2831,7 @@ smp_fetch_htx_has_eom(const struct arg *arg_p, struct sample *smp, const char *k if (!htx) return 0; - smp->data.u.sint = !!(htx->flags & HTX_FL_HAS_EOM); + smp->data.u.sint = htx_has_eom(htx); smp->data.type = SMP_T_BOOL; smp->flags = SMP_F_VOLATILE | SMP_F_MAY_CHANGE; return 1; diff --git a/src/htx.c b/src/htx.c index 4480ade42..aa473d555 100644 --- a/src/htx.c +++ b/src/htx.c @@ -827,8 +827,6 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla goto stop; } last_dstblk->flags = blk->flags; - if (last_dstblk->flags & HTX_BLK_FL_EOM) - dst->flags |= HTX_FL_HAS_EOM; break; default: @@ -845,8 +843,6 @@ size_t htx_xfer(struct htx *dst, struct htx *src, size_t count, unsigned int fla last_dstblk->flags = blk->flags; last_dstblk->info = blk->info; htx_memcpy(htx_get_blk_ptr(dst, last_dstblk), htx_get_blk_ptr(src, blk), sz); - if (last_dstblk->flags & HTX_BLK_FL_EOM) - dst->flags |= HTX_FL_HAS_EOM; last_dstblk_sz = sz; count -= meta_sz + sz; ret += meta_sz + sz; @@ -1296,7 +1292,6 @@ int htx_append_msg(struct htx *dst, const struct htx *src) newblk->info = blk->info; htx_memcpy(htx_get_blk_ptr(dst, newblk), htx_get_blk_ptr(src, blk), blksz); } - dst->flags |= (src->flags & HTX_FL_HAS_EOM); return 1; error: diff --git a/src/qcm_http.c b/src/qcm_http.c index 7f7cc57aa..776f8cdf3 100644 --- a/src/qcm_http.c +++ b/src/qcm_http.c @@ -33,8 +33,7 @@ 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_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_HAS_EOM) && - !(qcs->flags & QC_SF_EOI_SUSPENDED)) { + if (htx_has_eom(qcs_htx) && !(qcs->flags & QC_SF_EOI_SUSPENDED)) { *fin = 1; } @@ -48,7 +47,7 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count, BUG_ON(qcs_htx->flags & HTX_FL_PARSING_ERROR); /* EOM was copied to cs_htx if all data were copied. */ - if (cs_htx->flags & HTX_FL_HAS_EOM) { + if (htx_has_eom(cs_htx)) { if (!(qcs->flags & QC_SF_EOI_SUSPENDED)) *fin = 1; } diff --git a/src/tcpcheck.c b/src/tcpcheck.c index fb4fe92b9..416086cdf 100644 --- a/src/tcpcheck.c +++ b/src/tcpcheck.c @@ -2000,7 +2000,7 @@ enum tcpcheck_eval_ret tcpcheck_eval_expect_http(struct check *check, struct tcp TRACE_ENTER(CHK_EV_TCPCHK_EXP, check); - last_read |= (!htx_free_data_space(htx) || (htx->flags & HTX_FL_HAS_EOM)); + last_read |= (!htx_free_data_space(htx) || htx_has_eom(htx)); if (htx->flags & HTX_FL_PARSING_ERROR) { TRACE_ERROR("invalid response", CHK_EV_TCPCHK_EXP|CHK_EV_TCPCHK_ERR, check);