Repository navigation
legacy/v0.4: size the streaming output buffer for one extra block - #4815
Open
ManoharPaturi wants to merge 1 commit into
Open
ManoharPaturi wants to merge 1 commit into
ManoharPaturi wants to merge 1 commit into
Conversation
The v0.4 streaming decoder sizes its output buffer at exactly the frame window. When a flushed block leaves less than a full block of room, it wraps the buffer back to offset zero, discarding the history that later sequences still reference through offsets within the window. Match sources then read bytes the current block has already overwritten, so the streaming API silently returns different output than the one-shot API for the same frame. The v0.5 decoder had the same flaw (issue meta#4553), and the v0.6 and v0.7 decoders already solve it by sizing the buffer at windowSize plus one block, which makes the wrap harmless. Apply the same sizing here. Add a crafted frame to the legacy test whose first block is smaller than the declared window and whose next block references it: before this change the streaming decode differs from the one-shot decode, after it they match.
ManoharPaturi
force-pushed
the
fix-legacy-v04-stream-wrap
branch
from
October 4, 2026 04:37
51edc6f to
4023f9d
Compare
Author
|
@Cyan4973 these legacy decoder fixes (#4815 among the v0.2 through v0.7 batch) have been green on all checks for a few weeks now with no reviewer assigned yet. Each one is a narrow correctness fix in one legacy format with a round-trip regression test in tests/legacy.c. Could you or another maintainer take a look when the queue allows? |
This branch has not been deployed
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.
The v0.4 streaming decoder (ZBUFF) sizes its output buffer at exactly the frame window, so it must wrap the buffer back to offset zero whenever a flushed block leaves less than a full block of room. The wrap discards history that later sequences can still reference, because a block is allowed to be smaller than the window while following blocks use offsets that reach back into it. Match sources then read bytes the current block has already overwritten, and the streaming API silently returns different output than the one shot API for the same frame.
The v0.5 decoder had the same flaw (issue #4553). The v0.6 and v0.7 decoders already solve it by sizing the buffer at windowSize plus one block, which pushes the wrap point past a full window and makes it harmless. This change applies the same sizing to v0.4.
The defect only shows on frames whose block layout leaves the buffer partly filled at the wrap decision, which the v0.4 encoder never produces (it always fills blocks up to the window), so the frame in the test is hand crafted:
windowLog 11 (window of 2048 bytes)
block 1 : raw, 600 bytes
block 2 : compressed, one sequence with offset 1000 referencing block 1
Behavior on unfixed dev, via the public API:
With this change the two decoders agree, and the new test in tests/legacy.c locks that in at any ZSTD_LEGACY_SUPPORT level that can decode v0.4. The test fails on unfixed dev with "streaming decoder output differs from one-shot decoder" and passes after the change; the existing legacy tests pass at every level from 1 to 7.