From: Willy Tarreau Date: Mon, 27 Jul 2026 08:58:13 +0000 (+0200) Subject: BUG/MINOR: h2: don't use a block pointer to roll back a partial HTX conversion X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=865351d4ed7cb883349fe8b61ee1aacade8f467e;p=thirdparty%2Fhaproxy.git BUG/MINOR: h2: don't use a block pointer to roll back a partial HTX conversion h2_make_htx_request(), h2_make_htx_response() and h2_make_htx_trailers() save a pointer on the tail HTX block on entry and use it on their error path to remove everything they added: struct htx_blk *tailblk = htx_get_tail_blk(htx); ... fail: htx_truncate_blk(htx, tailblk); An HTX block pointer is not a stable reference: it is computed from a block *position* ("htx->blocks + htx->size - (pos + 1) * sizeof(struct htx_blk)"), so it only designates the same block as long as the block table is not compacted. htx_add_header()/htx_add_trailer() end up in htx_reserve_nxblk(), which calls htx_defrag_blks() when the table has grown down to the payload while htx->head > 0. After such a compaction all the blocks move down by the old value of htx->head, but still points at the same address, hence at a block located positions further in the message. htx_truncate_blk() would then truncate at the wrong place, either leaving partially converted headers or trailers in the message, or dropping valid blocks that were there before. This only concerns the trailers, and possibly the response on the backend side, because the destination message may already hold payload with a partially consumed head there, while a request is always converted into an empty buffer which cannot defragment. Let's save the amount of data present on entry and use htx_truncate() instead, which relies on a byte offset and is therefore immune to any block move. This is the same pattern as the one already used by htx_append_msg(). This has been there since the H2 to HTX conversion was introduced, so it should be backported to all supported versions. --- diff --git a/src/h2.c b/src/h2.c index f0302211a..c66f79dca 100644 --- a/src/h2.c +++ b/src/h2.c @@ -324,7 +324,7 @@ static struct htx_sl *h2_prepare_htx_reqline(uint32_t fields, struct ist *phdr, */ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, int relaxed) { - struct htx_blk *tailblk = htx_get_tail_blk(htx); + uint32_t data_ofs = htx->data; /* used to rollback on error */ struct ist phdr_val[H2_PHDR_NUM_ENTRIES]; uint32_t fields; /* bit mask of H2_PHDR_FND_* */ uint32_t idx; @@ -539,7 +539,12 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms return ret; fail: - htx_truncate_blk(htx, tailblk); + /* Rollback the partial conversion. Note that a pointer on the tail + * block cannot be used for this because it encodes a block position + * and adding blocks may compact the block table, so rely on the + * amount of data that was present on entry instead. + */ + htx_truncate(htx, data_ofs); return -1; } @@ -644,7 +649,7 @@ static struct htx_sl *h2_prepare_htx_stsline(uint32_t fields, struct ist *phdr, */ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *msgf, unsigned long long *body_len, char *upgrade_protocol) { - struct htx_blk *tailblk = htx_get_tail_blk(htx); + uint32_t data_ofs = htx->data; /* used to rollback on error */ struct ist phdr_val[H2_PHDR_NUM_ENTRIES]; uint32_t fields; /* bit mask of H2_PHDR_FND_* */ uint32_t idx; @@ -801,7 +806,12 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m return ret; fail: - htx_truncate_blk(htx, tailblk); + /* Rollback the partial conversion. Note that a pointer on the tail + * block cannot be used for this because it encodes a block position + * and adding blocks may compact the block table, so rely on the + * amount of data that was present on entry instead. + */ + htx_truncate(htx, data_ofs); return -1; } @@ -821,7 +831,7 @@ int h2_make_htx_response(struct http_hdr *list, struct htx *htx, unsigned int *m */ int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx) { - struct htx_blk *tailblk = htx_get_tail_blk(htx); + uint32_t data_ofs = htx->data; /* used to rollback on error */ const char *ctl; struct ist v; uint32_t idx; @@ -881,6 +891,11 @@ int h2_make_htx_trailers(struct http_hdr *list, struct htx *htx) return 1; fail: - htx_truncate_blk(htx, tailblk); + /* Rollback the partial conversion. Note that a pointer on the tail + * block cannot be used for this because it encodes a block position + * and adding blocks may compact the block table, so rely on the + * amount of data that was present on entry instead. + */ + htx_truncate(htx, data_ofs); return -1; }