From: Willy Tarreau Date: Sun, 26 Jul 2026 20:41:32 +0000 (+0200) Subject: BUG/MINOR: slz: use the exact switch cost for the last literals of a block X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=fbfe17766e2dbd3fb6935461196e290715b3037f;p=thirdparty%2Fhaproxy.git BUG/MINOR: slz: use the exact switch cost for the last literals of a block The decision to send the pending literals as a stored block rather than in fixed huffman mode is taken when the 9-bit literals wasted more than the 52 bits it costs to leave the fixed huffman encoding and to come back to it. But for the last literals of a block, nothing comes after the stored block, so there is no need to pay for the block type of a next block nor for the EOB, while the huffman variant still has to send an EOB. The switch is thus 10 bits cheaper, and 10 more when the stream is still in EOB state, since then the block type is needed in both cases and no EOB has to be terminated. Using 52 there made the encoder prefer huffman for data that was cheaper to store, and the output could exceed the documented maximum. The smallest case found by fuzzing is a 47-byte input entirely made of bytes >= 144 which produced 55 bytes (3 bits of block type + 47*9 bits + 7 bits of EOB) where the stored block only needs 52, for a documented maximum of 54. With these correct costs, we no longer see outputs exceed the documented maximum, wether it's with small inputs (tested with ~3 million random small inputs as small as 47 bytes), or usual files found in tests/ and bash, gcc, libc, and silesia. No performance change was observed either. Note that a stream can still exceed the documented maximum by a few bytes (17 bytes were observed on a 390000-byte crafted input) because each reference emitted between two stored blocks forces them out and adds a 5-byte block header that the accounting attributes to the reference. This is for a future fix. This should be backported to 1.2. This is libslz upstream commit 97757536178f24aeb2cb41278706a88c1242f414. Note: the impact in haproxy is practically inexistent since we have a 1kB reserve and build by default with the tag at the end of pools, even if two extra bytes were to be emitted, it would have no effect. Better backport it to avoid triggering ASAN though. --- diff --git a/src/slz.c b/src/slz.c index e21a609b4..20ba23036 100644 --- a/src/slz.c +++ b/src/slz.c @@ -507,6 +507,22 @@ static void reset_refs(union ref *refs, long count) } while (refs < end); } +/* Number of bits wasted by the 9-bit literals above which it becomes + * preferable to send the pending literals as a stored block. It corresponds to + * the cost of leaving the fixed huffman encoding and coming back to it: EOB + * (7 bits) + block type (3 bits) + alignment (up to 7 bits) + LEN and NLEN + * (32 bits) + block type of the next block (3 bits), that is 52 bits. Two + * cheaper variants are needed for the last literals of a block: nothing + * follows the stored block while the huffman encoding still has to send an EOB + * (10 bits less), and if the stream is still in EOB state, no EOB has to be + * sent and the block type is needed in both cases (10 more bits less). Using a + * larger value than these would leave the output larger than the same data + * sent as stored blocks, in violation of the promise made above. + */ +#define SLZ_SWITCH_COST 52 +#define SLZ_LAST_COST 42 +#define SLZ_LAST_COST_EOB 32 + /* Compresses bytes from into according to RFC1951. The * output result may be up to 5 bytes larger than the input for each 65535 * nput bytes, to which 2 extra bytes may be added to send the last chunk due @@ -691,7 +707,8 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig * is what guarantees that once switched to FIXED we can stay * in it for as long as needed. */ - if (strm->state == SLZ_ST_EOB && (dist & 0x1f) + (code >> 16) + 52 > 8 * mlen) + if (strm->state == SLZ_ST_EOB && + (dist & 0x1f) + (code >> 16) + SLZ_SWITCH_COST > 8 * mlen) goto send_as_lit; /* first, copy pending literals */ @@ -703,7 +720,7 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig * block. Only use plain literals if there are more than 52 bits * to save then. */ - if (bit9 >= 52) + if (bit9 >= SLZ_SWITCH_COST) copy_lit(strm, in + pos - plit, plit, 1); else copy_lit_huff(strm, in + pos - plit, plit, 1); @@ -749,9 +766,19 @@ long slz_rfc1951_encode(struct slz_stream *strm, unsigned char *out, const unsig } final_lit_dump: - /* now copy remaining literals or mark the end */ + /* Now copy remaining literals or mark the end. The cost of switching to + * a stored block depends on the current state and on the presence of + * data after these literals, see the SLZ_*_COST definitions above. + */ if (plit) { - if (bit9 >= 52) + uint32_t cost; + + if (more) + cost = SLZ_SWITCH_COST; + else + cost = (strm->state == SLZ_ST_EOB) ? SLZ_LAST_COST_EOB : SLZ_LAST_COST; + + if (bit9 >= cost) copy_lit(strm, in + pos - plit, plit, more); else copy_lit_huff(strm, in + pos - plit, plit, more);