Conversation
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.
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.
node:zlib's zstd accepts apledgedSrcSizethat is wrong, as long as the input arrives in one call. The stream path rejects the same mistake, so the two paths disagree:Node rejects both.
Why the one-shot path misses it
zstd enforces
ZSTD_CCtx_setPledgedSrcSize()only when a frame spans more than oneZSTD_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()andinfo: truealways compress that way, so the frame comes out valid and nothing reports the mismatch.Node hit the same override and counts the input itself:
ZstdCompressContexttracks the bytes each frame consumes and raisessrcSize_wrongwhen the frame ends on a different count (nodejs/node@c3de450f26).What changed
Two commits, each building and passing on its own:
ZstdEncoderContextkeeps the pledge and a count of consumed input while one is set.work()compares them when a frame completes and setsZSTD_error_srcSize_wrong, whichgetError()already reports asZstd 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.reset().ZSTD_CCtx_reset(ZSTD_reset_session_only)also sets the pledged size back to unknown, so afterreset()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'sResetStream()does.Only
node:zlibconstructs aZstdEncoderContext, so the webCompressionStreamis 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:zstdCompressSync,info: trueandzstdCompressreject a wrong pledge, and a correct one still round-tripsreset(): a stream reset before writing still rejects a wrong pledge, and a correct one round-tripsAs a control, both fail on
mainfor 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-autogatesand@eslint, andzlib-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
pledgedSrcSizeto the one-shot functions gets a frame today and will get an error after this. I didn't add a compatibility flag because the earliernode:zlibparity 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.