]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
projects / thirdparty / haproxy.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 84c3c96)
BUG/MINOR: h2: don't use a block pointer to roll back a partial HTX conversion
authorWilly Tarreau <w@1wt.eu>
Mon, 27 Jul 2026 08:58:13 +0000 (10:58 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Mon, 27 Jul 2026 13:29:50 +0000 (15:29 +0200)
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 <tailblk> still points at the same address, hence at a
block located <head> 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.

src/h2.c patch | blob | blame | history

diff --git a/src/h2.c b/src/h2.c
index f0302211a6b9974a8c38318da6e13bf2421f8d1e..c66f79dca4aa0af85db883d2018b5762064938e9 100644 (file)
--- 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;
 }
Mirror of https://github.com/haproxy/haproxy.git