Repository navigation
wsflate: accept Go 1.27's BFINAL fixed-Huffman stream tail - #230
Closed
fxfactorial wants to merge 1 commit into
Closed
fxfactorial wants to merge 1 commit into
fxfactorial wants to merge 1 commit into
Conversation
Go 1.27 rewrote compress/flate (golang/go@388145c, golang/go#75532). The new encoder ends a stream with an empty final fixed-Huffman block (BFINAL=1, BTYPE=01, i.e. 0x03 0x00 0x00) instead of the 4-byte empty non-final stored block (0x00 0x00 0xff 0xff) used before. cbuf holds back the last 4 bytes, so the stream tail is now {0xff,0xff,0x03,0x00} and the old checkTail() rejected it with 'wsflate: bad compressor: unexpected stream tail: 0x7d0a0300 vs 0x0000ffff' (gobwas#221). Both tails are valid per RFC 7692 7.2.3.4. checkTail() now also accepts the BFINAL-fixed tail, passing its 4 held bytes through to the destination so the on-wire payload is a complete, self-terminating DEFLATE stream (the receiver appends 0x00 0x00 0xff 0xff per 7.2.2, which lands after the BFINAL block and is ignored). The classic tail path is unchanged, and truly unexpected tails are still rejected. A tailChecked flag keeps checkTail idempotent across Flush()+Close(). Adds toolchain-independent tests (blockCompressor drives the exact tail bytes) covering both tails, the rejected-tail case, plus round-trip and context-takeover/fragmented edge cases. Verified on go1.26.8 and go1.27.0.
Owner
|
Hey @fxfactorial, thanks for your PR! I think for now we're considering #227, which feels to be more in line with the spec and less bound to a concrete compressor implementation behaviour (the stdlib's one): accepting a second known tail keeps us bound to how one particular compressor happens to encode its final block, which is exactly what broke here. #227 treats the message as finished at Flush() and keeps the compressor's Close output off, so no tail list is needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #221.
Problem
Building against Go 1.27+ (or
gotipafter golang/go@388145c) breakswsflatewith:Root cause
Go 1.27 rewrote
compress/flate(golang/go@388145c, golang/go#75532). The new encoderends a stream with an empty final fixed-Huffman block (BFINAL=1, BTYPE=01,
i.e.
03 00 00— 3 bytes) instead of the empty non-final stored block(
00 00 00 FF FF— 4 bytes) that previous versions appended as a flush trailer.cbufholds back the last 4 bytes, so the held tail is now{0xff, 0xff, 0x03, 0x00}and the old
checkTail()rejected it. Verified byte-for-byte across all levels 0-9 andvarious payloads: Go 1.26 -> tail
00 00 FF FF; Go 1.27 -> tailFF FF 03 00.Both tails are valid per RFC 7692 section 7.2.3.4
("Using a DEFLATE Block with BFINAL Set to 1").
Fix
checkTail()inwsflate/writer.gonow accepts both tails:00 00 FF FF): unchanged — the 4 held bytes are dropped (thereceiver appends them back per RFC 7692 7.2.2).
FF FF 03 00): the 4 held bytes are passed through to thedestination so the on-wire payload is a complete, self-terminating DEFLATE stream
(
... [00 00 00 FF FF] [03 00]). The receiver then appends00 00 FF FFper 7.2.2,which lands after the BFINAL block and is ignored by the decompressor.
tailCheckedflag keepscheckTailidempotent acrossFlush()+Close()(as
Helper.CompressTodoes), andReset()clears it.The on-wire format for classic-tail compressors is byte-identical to before; only
Go 1.27's tail is newly accepted. The extra 1 byte of payload for the BFINAL tail is
the spec-sanctioned way to keep the stream byte-aligned and minimal.
Tests
writer_tail_test.go: toolchain-independent unit tests — ablockCompressordrives the exact tail bytes, so the new BFINAL branch is covered even on Go 1.26.
Covers both accepted tails, the rejected-tail case, and a default round-trip.
zz_edge_test.go: round-trips for empty/short/exact-4/big payloads,fragmented writes with mid-message
Flush()calls, and context-takeover(single long-lived writer, carried LZ77 window).
Verified on go1.26.8 and go1.27.0: full module test suite passes on both,
plus
-raceandgo vetclean.cc @gobwas @cristaloleg — thanks @prattmic for the analysis in #221 (this follows
the RFC 7.2.3.4 BFINAL approach discussed there).