Skip to content

Enforce a pledged zstd source size on one-shot compression and across reset() - #7524

Open
oddharsh wants to merge 4 commits into
cloudflare:mainfrom
oddharsh:zstd-pledged-src-size
Open

oddharsh wants to merge 4 commits into
cloudflare:mainfrom
oddharsh:zstd-pledged-src-size

Conversation

@oddharsh

Copy link
Copy Markdown

node:zlib's zstd accepts a pledgedSrcSize that is wrong, as long as the input arrives in one call. The stream path rejects the same mistake, so the two paths disagree:

const input = Buffer.alloc(1280);
zlib.zstdCompressSync(input, { pledgedSrcSize: 1281 });           // returns a frame
zlib.createZstdCompress({ pledgedSrcSize: 1281 }).end(input);     // emits 'Src size is incorrect'

Node rejects both.

Why the one-shot path misses it

zstd enforces ZSTD_CCtx_setPledgedSrcSize() only when a frame spans more than one ZSTD_compressStream2() call. When the first call is also the last (ZSTD_e_end), ZSTD_CCtx_init_compressStream2() replaces the pledge with the real input size (/* auto-determine pledgedSrcSize */). zstdCompressSync(), zstdCompress() and info: true always compress that way, so the frame comes out valid and nothing reports the mismatch.

Node hit the same override and counts the input itself: ZstdCompressContext tracks the bytes each frame consumes and raises srcSize_wrong when the frame ends on a different count (nodejs/node@c3de450f26).

What changed

Two commits, each building and passing on its own:

  1. Enforce the pledge on one-shot compression. ZstdEncoderContext keeps the pledge and a count of consumed input while one is set. work() compares them when a frame completes and sets ZSTD_error_srcSize_wrong, which getError() already reports as Zstd compression failed: Src size is incorrect. A frame spanning several calls is still rejected by zstd first, in the existing branch, so the new check only covers the case zstd skips.
  2. Keep the pledge across reset(). ZSTD_CCtx_reset(ZSTD_reset_session_only) also sets the pledged size back to unknown, so after reset() the next frame had no pledge at all, on the stream path too. resetStream() now pledges the size again and restarts the count, as Node's ResetStream() does.

Only node:zlib constructs a ZstdEncoderContext, so the web CompressionStream is untouched.

Tests

Two cases added to zlib-zstd-nodejs-test.js. Both pledge one byte too many and one byte too few, since zstd's override would hide either:

  • one-shot: zstdCompressSync, info: true and zstdCompress reject a wrong pledge, and a correct one still round-trips
  • reset(): a stream reset before writing still rejects a wrong pledge, and a correct one round-trips

As a control, both fail on main for the reasons above ("Missing expected exception: The fast path should reject a pledge of 1281", and "A wrong pledge should still be rejected after reset()"). Both pass with the change, and both also pass as plain Node scripts under v26.9.0 and v26.10.0.

Green locally in a Linux container (Ubuntu 24.04, LLVM 19): zlib-zstd-nodejs-test@, @all-compat-flags, @all-autogates and @eslint, and zlib-nodejs-test@ plus @all-autogates. clang-format and prettier report no changes.

Two things for a reviewer

This is a behaviour change on one path. A Worker passing a wrong pledgedSrcSize to the one-shot functions gets a frame today and will get an error after this. I didn't add a compatibility flag because the earlier node:zlib parity fixes I found landed without one, and the stream path already throws. Say the word if you'd rather gate it and I'll add one.

Node's reset now also refuses a half-written frame (ERR_ZLIB_INCOMPLETE_FRAME, from nodejs/node#65867). That's a separate behaviour, so I left it out here.

This overlaps with #7106, which also edits ZstdEncoderContext::initialize(). The two don't depend on each other, and whichever lands second needs a small rebase.

zstd enforces ZSTD_CCtx_setPledgedSrcSize() by itself only when a frame
spans more than one ZSTD_compressStream2() call. When the first call is
also the last (ZSTD_e_end), zstd replaces the pledge with the real input
size. zstdCompressSync(), zstdCompress() and the info: true path always
compress that way, so a wrong pledgedSrcSize produced a valid frame and
no error, while the stream path rejected the same mistake.

Count the input each frame consumes while a size is pledged, and fail
with ZSTD_error_srcSize_wrong when the frame ends on a different count.
Node does the same in ZstdCompressContext (nodejs/node@c3de450f26).
ZSTD_CCtx_reset(ZSTD_reset_session_only) also sets the pledged size back
to unknown, so after reset() a zstd stream wrote its next frame with no
pledge at all and accepted any amount of input. Pledge the size again
after the reset and restart the count that enforces it, as Node's
ZstdCompressContext::ResetStream() does.
@oddharsh
oddharsh requested review from a team as code owners September 25, 2026 18:53
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