Fix sync-flush stored-block misalignment in Deflate.flush#286
Open
kixelated wants to merge 1 commit into
Open
Conversation
When a deflated block ended at a bit position with `(s.r & 7) == 6`, `flush(true)` wrote the empty stored block's LEN/NLEN fields one byte too early. `wfblk` byte-aligns using `shft(pos + 2)`, reserving only 2 header bits, but the flush path's `pos` (s.r) still points at the BFINAL bit, so the stored-block header is 3 bits (BFINAL + 2-bit BTYPE). At that alignment the third header bit lands on a byte boundary and the 00 00 ff ff sync marker becomes bit-misaligned. fflate's own inflate masked the corruption, but spec-compliant decoders (zlib, pako) reject the following block with "invalid stored block lengths". Pass `(s.r & 7) + 1` to account for the implicit BFINAL bit, matching the convention at wfblk's other call site in wblk. This also fixes a latent case where the carried partial-byte value packed into s.r's high bits would skew the shft() arithmetic. Adds a regression test covering fflate self round-trip and reference decode (Node zlib) for the minimal trigger, a real JSON snapshot+deltas stream, and a sweep of repeated-byte lengths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Author
|
And just to elaborate, this is It makes a HUGE difference for my framed JSON payloads. 40% bitrate savings -> 90% bitrate savings, all because flushing means we can reuse the sliding window between these relatively small snapshots (emitted once every 30ms). I have to use |
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
1 participant
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.
NOTE: AI generated. I did verify that fflate produces incorrect output periodically.
When a deflated block ended at a bit position with
(s.r & 7) == 6,flush(true)wrote the empty stored block's LEN/NLEN fields one byte too early.wfblkbyte-aligns usingshft(pos + 2), reserving only 2 header bits, but the flush path'spos(s.r) still points at the BFINAL bit, so the stored-block header is 3 bits (BFINAL + 2-bit BTYPE). At that alignment the third header bit lands on a byte boundary and the 00 00 ff ff sync marker becomes bit-misaligned. fflate's own inflate masked the corruption, but spec-compliant decoders (zlib, pako) reject the following block with "invalid stored block lengths".Pass
(s.r & 7) + 1to account for the implicit BFINAL bit, matching the convention at wfblk's other call site in wblk. This also fixes a latent case where the carried partial-byte value packed into s.r's high bits would skew the shft() arithmetic.Adds a regression test covering fflate self round-trip and reference decode (Node zlib) for the minimal trigger, a real JSON snapshot+deltas stream, and a sweep of repeated-byte lengths.