From: Willy Tarreau Date: Sun, 8 Nov 2026 11:03:45 +0000 (+0100) Subject: BUG/MEDIUM: compression: use a trash chunk as large as the buffer for the output X-Git-Url: https://git.kaiwu.me/$%7BISSUES%7D/static/gitweb.js?a=commitdiff_plain;h=7dcddd3504dc95b45136da12d695eef38be0b63b;p=haproxy.git BUG/MEDIUM: compression: use a trash chunk as large as the buffer for the output Since commit a0b37fb47 ("BUG/MEDIUM: compression: fix incompatibility between compression and large-buffers"), slz_encode() writes directly into the output chunk passed by the compression filter, which is the global trash, sized like regular buffers. But with large buffers, a DATA block may be larger than this trash. The input is then cut to the trash size by the test added in commit f82ace414 ("MEDIUM: compression: Be sure to never compress more than a chunk at once"), leaving no room for the deflater's overhead. On incompressible data, the first call emits the input plus 5 bytes for the stored block plus the 10 bytes of the gzip header, which overflows the trash: FATAL: bug condition "b->data + count > b->size" matched at include/haproxy/buf.h:395 #0 b_add (b=..., count=32783) at include/haproxy/buf.h:395 #1 comp_rfc195x_add_data (..., in_len=32768, out=...) at src/compression.c:404 This was seen in production on a PDF served with 32 kB buffers and "http-response wait-for-body use-large-buffer", where the body formed a 37 kB DATA block. Without DEBUG_STRICT=2, the overflow goes unnoticed. The output replaces the input in the HTX, so the right size for the output chunk is the one of the buffer it comes from. It is now obtained with get_best_trash_chunk(), which returns a large trash chunk for large buffers and the regular one otherwise. htx_compression_buffer_init() still ensures that the HTX has the room for the deflater's overhead, so a DATA block plus this overhead always fits in such a chunk. A side effect is that the data of a large buffer are now compressed in a single call, as intended by the use of large buffers for compression. The reg-test compression/large_buffers.vtc reproduces the crash with unpatched versions built with DEBUG_STRICT=2. This must be backported wherever a0b37fb47 is backported. --- diff --git a/reg-tests/compression/large_buffers.vtc b/reg-tests/compression/large_buffers.vtc new file mode 100644 index 000000000..8e24a5e4f --- /dev/null +++ b/reg-tests/compression/large_buffers.vtc @@ -0,0 +1,64 @@ +varnishtest "Compression filter: incompressible contents in large buffers" + +# The compressed output replaces the input in the HTX buffer, and is built in +# a trash chunk. With large buffers, a DATA block may be larger than the +# regular trash, so the chunk must be as large as the buffer, otherwise the +# deflater's output exceeds it on incompressible data (input + 5 bytes per +# stored block + the gzip header). Here the response is waited for into a +# large buffer, so that its body forms a single DATA block of up to 200 kB, +# well beyond the 16 kB of the regular buffers. The client decompresses it, +# which checks the gzip trailer's CRC and length, and compares the length to +# the one announced by the service. +feature cmd "$HAPROXY_PROGRAM -cc '(feature(ZLIB) || feature(SLZ)) && feature(LUA)'" +feature ignore_unknown_macro + +haproxy h1 -conf { + global + tune.bufsize 16384 + tune.bufsize.large 262144 + tune.lua.bool-sample-conversion normal + lua-load ${testdir}/output_margin.lua + + defaults + mode http + timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" + timeout client "${HAPROXY_TEST_TIMEOUT-5s}" + timeout server "${HAPROXY_TEST_TIMEOUT-5s}" + + frontend fe-gzip + bind "fd@${fe_gzip}" + compression algo gzip + compression type text/plain + default_backend be + + backend be + http-response wait-for-body time 5s use-large-buffer + server gen ${h1_gen_addr}:${h1_gen_port} + + listen gen + bind "fd@${gen}" + http-request use-service lua.gen +} -start + +client c1 -connect ${h1_fe_gzip_sock} { + txreq -url "/?p=random&c=whole&n=20000" -hdr "Accept-Encoding: gzip" + rxresp + expect resp.status == 200 + expect resp.http.content-encoding == "gzip" + gunzip + expect resp.bodylen == 20000 + + txreq -url "/?p=random&c=whole&n=200000" -hdr "Accept-Encoding: gzip" + rxresp + expect resp.status == 200 + expect resp.http.content-encoding == "gzip" + gunzip + expect resp.bodylen == 200000 + + txreq -url "/?p=mixed&c=whole&n=200000" -hdr "Accept-Encoding: gzip" + rxresp + expect resp.status == 200 + expect resp.http.content-encoding == "gzip" + gunzip + expect resp.bodylen == 200000 +} -run diff --git a/src/flt_http_comp.c b/src/flt_http_comp.c index 71dffb859..9649dc5af 100644 --- a/src/flt_http_comp.c +++ b/src/flt_http_comp.c @@ -296,6 +296,7 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, struct htx *htx = htxbuf(&msg->chn->buf); struct htx_ret htxret = htx_find_offset(htx, offset); struct htx_blk *blk, *next; + struct buffer *out; int ret, consumed = 0, to_forward = 0, last = 0, flush; blk = htxret.blk; @@ -311,7 +312,14 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, if (!(st->flags & COMP_STATE_PROCESSING)) goto consume; - if (htx_compression_buffer_init(htx, &trash) < 0) { + /* The output replaces the input in the HTX, so the chunk must + * be as large as the buffer, which may be a large one. + */ + out = get_best_trash_chunk(&msg->chn->buf, b_size(&msg->chn->buf)); + if (!out) + goto error; + + if (htx_compression_buffer_init(htx, out) < 0) { msg->chn->flags |= CF_WAKE_WRITE; goto end; } @@ -325,9 +333,9 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, last = 0; v.len = len; } - if (v.len > b_size(&trash)) { + if (v.len > b_size(out)) { last = 0; - v.len = b_size(&trash); + v.len = b_size(out); } /* The output only needs to be flushed when no other @@ -342,15 +350,15 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, */ flush = last || v.len == len || !next || htx_get_blk_type(next) != HTX_BLK_DATA; - ret = htx_compression_buffer_add_data(st, v.ptr, v.len, &trash, dir); - if (ret < 0 || (flush && htx_compression_buffer_end(st, &trash, last, dir) < 0)) + ret = htx_compression_buffer_add_data(st, v.ptr, v.len, out, dir); + if (ret < 0 || (flush && htx_compression_buffer_end(st, out, last, dir) < 0)) goto error; BUG_ON(v.len != ret); - if (ret == sz && !b_data(&trash)) + if (ret == sz && !b_data(out)) next = htx_remove_blk(htx, blk); else { - blk = htx_replace_blk_value(htx, blk, v, ist2(b_head(&trash), b_data(&trash))); + blk = htx_replace_blk_value(htx, blk, v, ist2(b_head(out), b_data(out))); if (!blk) goto error; next = htx_get_next_blk(htx, blk); @@ -358,7 +366,7 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, len -= ret; consumed += ret; - to_forward += b_data(&trash); + to_forward += b_data(out); if (last) { st->flags &= ~COMP_STATE_PROCESSING; @@ -379,17 +387,17 @@ comp_http_payload(struct stream *s, struct filter *filter, struct http_msg *msg, case HTX_BLK_TLR: case HTX_BLK_EOT: - if (htx_compression_buffer_end(st, &trash, 1, dir) < 0) + if (htx_compression_buffer_end(st, out, 1, dir) < 0) goto error; - if (b_data(&trash)) { - struct htx_blk *last = htx_add_last_data(htx, ist2(b_head(&trash), b_data(&trash))); + if (b_data(out)) { + struct htx_blk *last = htx_add_last_data(htx, ist2(b_head(out), b_data(out))); if (!last) goto error; blk = htx_get_next_blk(htx, last); if (!blk) goto error; next = htx_get_next_blk(htx, blk); - to_forward += b_data(&trash); + to_forward += b_data(out); } st->flags &= ~COMP_STATE_PROCESSING; __fallthrough;