Skip to content

Decompress: grow from a small buffer for unknown-size frames with no dst#169

Open
KrisKennawayDD wants to merge 2 commits into
1.xfrom
kris.kennaway/decompress-nil-buffer-legacy-sizing
Open

Decompress: grow from a small buffer for unknown-size frames with no dst#169
KrisKennawayDD wants to merge 2 commits into
1.xfrom
kris.kennaway/decompress-nil-buffer-legacy-sizing

Conversation

@KrisKennawayDD

Copy link
Copy Markdown
Contributor

Stacked on #168 (base branch kris.kennaway/decompress-reuse-caller-buffer). Review/merge #168 first; this PR's diff is just the final commit. It's split out so it can be accepted or rejected independently.

Problem

For frames that do not advertise their decompressed size — legacy zstd v0.5 frames, and streaming frames compressed without a pledged source size — decompressSizeHint returns only a pessimistic upper bound (max(50×len(src), decompressSizeBufferLimit) = at least 1 MB). #168 fixed the case where the caller passes a reusable buffer. This PR handles the remaining case: Decompress(nil, src) / no caller buffer, where the code still allocates that ≥1 MB bound up front on every such decode regardless of the real payload size.

Fix

When the size is unknown and no caller buffer is supplied, grow from 3×len(src) (3× → 6× → 12×) before falling back to the streaming reader, instead of allocating the bound. The size-hint fast path (frames that advertise their size) is unchanged.

History note

This restores the sizing used before 489b911 "Remove Decompress multiple retries." That commit dropped the grow loop on the premise that "zstd 1.3.0+ frames always carry a size hint." That premise does not hold for legacy v0.5 frames or unpledged streaming frames — exactly the case handled here — so the retries weren't evaluated against it. This reintroduces the grow schedule only for the unknown-size + no-buffer path, leaving the 1.3.0+ hint optimization intact.

No DoS regression vs 30c4b29 "Add a sanity limit": growDecompress never allocates based on an attacker-claimed size (only from the input length, bounded to ≤12×len(src)), so it does not reintroduce the unbounded-allocation vector that decompressSizeBufferLimit guards. (The streaming fallback's ReadAll is unbounded, but that already exists on the current code path.)

Applies to both Decompress and ctx.Decompress; regression tests (nil-buffer unknown-size avoids the bound; multi-block stream fallback; known-size still exact) run as subtests against both.

🤖 Generated with Claude Code

@KrisKennawayDD
KrisKennawayDD force-pushed the kris.kennaway/decompress-reuse-caller-buffer branch 2 times, most recently from 539664d to ddc2ccc Compare July 24, 2026 17:44
When the frame does not advertise its decompressed size and the caller passes
no buffer, Decompress/ctx.Decompress allocated decompressSizeBufferLimit (the
pessimistic >=1MB upper bound) up front. For legacy zstd v0.5 frames and for
streaming frames compressed without a pledged size, that is every such decode,
regardless of the real payload size.

Grow from 3x the compressed size (3x -> 6x -> 12x) before falling back to the
streaming reader, restoring the sizing used before commit 489b911 ("Remove
Decompress multiple retries"). That commit dropped the grow loop on the premise
that zstd 1.3.0+ frames always carry a size hint -- which is false for v0.5 and
unpledged streaming frames, exactly the case handled here. The size-hint fast
path for frames that do advertise their size is unchanged.

growDecompress never allocates based on an attacker-claimed size (only from the
input length), so it does not reintroduce the DoS that decompressSizeBufferLimit
(commit 30c4b29) guards against.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@KrisKennawayDD
KrisKennawayDD force-pushed the kris.kennaway/decompress-nil-buffer-legacy-sizing branch from e75a32d to 56f2b04 Compare July 24, 2026 17:46
@KrisKennawayDD
KrisKennawayDD marked this pull request as ready for review July 24, 2026 17:47
Base automatically changed from kris.kennaway/decompress-reuse-caller-buffer to 1.x July 24, 2026 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant