From 5e39bf61ae80e8db6d0f7aa032c540eef383ae5a Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 16 Jun 2026 21:50:02 +0300 Subject: [PATCH 01/12] perf(decode): decode StreamingDecoder read_to_end in place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `StreamingDecoder::read_to_end` ran the generic `read` loop: decode a block into the internal `RingBuffer`, then COPY the collectable bytes into the caller's buffer — a double copy (decode -> ring -> user) plus `read_to_end`'s power-of-two regrow churn. The single-copy direct path (`run_direct_decode`, decode straight into a user slice) already existed but only `decode_all` (in-memory input + pre-sized output) used it. Specialize `read_to_end`: when the decoder sits at the start of its (already header-read) frame, buffer the compressed source and decode the frame STRAIGHT into the output `Vec`'s grown capacity via the direct path, pre-sized from the frame's declared content size. Eliminates the ring drain copy and the regrow churn. Falls back to the generic grow-and-drain loop when the caller already consumed part of the frame (mixed read + read_to_end), and to a window-bounded ring drain for frames without a declared content size. - new `FrameDecoder::decode_current_frame_to_vec` + `is_at_frame_start` - read_to_end override is cfg-split (std returns the byte count, no_std returns `()`), no_std build verified - regression tests: fast path fires (direct_frames == 1) and matches byte-for-byte; mixed read+read_to_end stays correct via the fallback Decode output is unchanged (852 tests incl. cross-validation FFI round-trip, verify-mode checksum, dict, streaming; clippy, fmt clean). --- zstd/src/decoding/frame_decoder.rs | 102 ++++++++++++++++++++ zstd/src/decoding/streaming_decoder.rs | 128 +++++++++++++++++++++++++ 2 files changed, 230 insertions(+) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 864beff07..74165835e 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -1456,6 +1456,14 @@ impl FrameDecoder { /// Whether the current frames last block has been decoded yet /// If this returns true you can call the drain* functions to get all content /// (the read() function will drain automatically if this returns true) + /// Test-only: number of frames decoded through the single-copy direct + /// path (`run_direct_decode`). Lets cross-module tests assert that a + /// given decode took the decode-in-place path rather than the ring drain. + #[cfg(test)] + pub(crate) fn direct_frames(&self) -> u64 { + self.direct_frames + } + pub fn is_finished(&self) -> bool { let state = match &self.state { None => return true, @@ -2279,6 +2287,100 @@ impl FrameDecoder { } } + /// Decode every frame in `input`, APPENDING the decompressed bytes to + /// `output` (growing it as needed), and return the number of bytes + /// appended. + /// + /// This is the `Vec`-output sibling of [`Self::decode_all`]. A frame that + /// declares its content size decodes DIRECTLY into freshly-reserved + /// `output` capacity through the single-copy direct path + /// ([`Self::run_direct_decode`]) — bypassing the `Ring`/`FlatBuf` → + /// `read()` drain copy that the streaming loop pays. A frame WITHOUT a + /// declared size falls back to the window-bounded ring drain (still one + /// copy, into `output`). Skippable frames are skipped. + /// + /// Each frame is (re)initialised via [`Self::init`] (resolving any + /// attached dictionary by id), so the decoder must be at a frame boundary + /// (no partially-decoded frame in flight) when this is called. It backs + /// [`StreamingDecoder`](crate::decoding::StreamingDecoder)'s `read_to_end` + /// fast path. + /// + /// Whether the decoder sits at the very start of an initialised frame: + /// the header has been read (state populated) but no block has been + /// decoded and the frame is not finished. In this state the wrapped + /// source is positioned exactly after the frame header, so + /// [`Self::decode_current_frame_to_vec`] can decode the rest of the frame + /// straight from the remaining source bytes. + pub(crate) fn is_at_frame_start(&self) -> bool { + self.state + .as_ref() + .is_some_and(|s| s.block_counter == 0 && !s.frame_finished) + } + + /// Decode the CURRENT (already-initialised) frame, APPENDING the + /// decompressed bytes to `output`, and return the number appended. + /// + /// `input` must be the frame's post-header bytes (the wrapped source after + /// `init` consumed the header). Unlike [`Self::decode_all_to_vec`] this + /// neither re-reads a header nor requires the caller to pre-reserve + /// capacity: a frame that declares its content size decodes DIRECTLY into + /// freshly-grown `output` capacity via the single-copy direct path + /// ([`Self::run_direct_decode`]) — bypassing the `Ring`/`FlatBuf` → + /// `read()` drain copy the streaming loop pays — while an unsized frame + /// falls back to the window-bounded ring drain (still one copy, into + /// `output`). Backs [`StreamingDecoder`](crate::decoding::StreamingDecoder)'s + /// `read_to_end` fast path; the caller must ensure + /// [`Self::is_at_frame_start`]. + /// + /// # Errors + /// + /// Propagates any [`FrameDecoderError`] from block decode, content-size + /// mismatch, or (in `Verify` mode) checksum validation. + pub(crate) fn decode_current_frame_to_vec( + &mut self, + mut input: &[u8], + output: &mut Vec, + ) -> Result { + let start_len = output.len(); + let content_size = self + .state + .as_ref() + .expect("caller ensures is_at_frame_start") + .frame_header + .frame_content_size(); + if content_size > 0 { + // FCS declared and non-empty: reserve exactly the frame's content + // and decode straight into it (single copy, no ring). The direct + // path writes precisely `content_size` bytes and never reads past + // the current write position, so the grown region is fully written. + let cs = content_size as usize; + output.resize(start_len + cs, 0); + let written = + self.run_direct_decode(&mut input, &mut output[start_len..], content_size)?; + output.truncate(start_len + written); + #[cfg(feature = "hash")] + self.verify_content_checksum()?; + return Ok(written); + } + // No declared size (or explicit FCS=0): window-bounded ring drain, + // appended directly to `output` via `collect_to_writer` (no staging + // buffer). Empty FCS=0 frames append nothing. + loop { + self.decode_blocks(&mut input, BlockDecodingStrategy::UptoBytes(1024 * 1024))?; + self.collect_to_writer(&mut *output) + .map_err(FrameDecoderError::FailedToDrainDecodebuffer)?; + if self.is_finished() { + // Final flush of the retained window tail. + self.collect_to_writer(&mut *output) + .map_err(FrameDecoderError::FailedToDrainDecodebuffer)?; + break; + } + } + #[cfg(feature = "hash")] + self.verify_content_checksum()?; + Ok(output.len() - start_len) + } + /// Default-feature decode_all_impl: no visitor parameter so the /// no-lsm build's call surface and codegen are byte-identical to /// the pre-#172 implementation. Compiles only when `lsm` is OFF. diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index d608838cc..eb2bd50b8 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -211,6 +211,76 @@ impl> Read for StreamingDecoder `RingBuffer` -> copy into the + /// caller buffer), buffer the (compressed, hence small) source and decode + /// STRAIGHT into `output`'s spare capacity via the single-copy direct path, + /// pre-sized from the frame's declared content size. Only taken when the + /// decoder is at a frame boundary (nothing partially decoded / undrained); + /// otherwise it falls back to the generic grow-and-`read` loop so a caller + /// that mixed `read` with `read_to_end` still gets correct output. + #[cfg(feature = "std")] + fn read_to_end(&mut self, output: &mut alloc::vec::Vec) -> Result { + // `new()` already read the frame header, so the fast path applies when + // the decoder sits at the start of that frame with nothing decoded yet. + let at_start = { + let d = self.decoder.borrow_mut(); + d.is_at_frame_start() && d.can_collect() == 0 + }; + if at_start { + let mut compressed = alloc::vec::Vec::new(); + self.source.read_to_end(&mut compressed)?; + let written = self + .decoder + .borrow_mut() + .decode_current_frame_to_vec(&compressed, output) + .map_err(Error::other)?; + return Ok(written); + } + // Mid-frame fallback: grow `output` and drain through the generic path. + let mut total = 0; + loop { + let start = output.len(); + output.resize(start + MAX_BLOCK_SIZE as usize, 0); + let n = self.read(&mut output[start..])?; + output.truncate(start + n); + if n == 0 { + break; + } + total += n; + } + Ok(total) + } + + /// no_std counterpart of the decode-in-place `read_to_end` fast path above + /// (the no_std `Read::read_to_end` returns `()` instead of the byte count). + #[cfg(not(feature = "std"))] + fn read_to_end(&mut self, output: &mut alloc::vec::Vec) -> Result<(), Error> { + let at_start = { + let d = self.decoder.borrow_mut(); + d.is_at_frame_start() && d.can_collect() == 0 + }; + if at_start { + let mut compressed = alloc::vec::Vec::new(); + self.source.read_to_end(&mut compressed)?; + self.decoder + .borrow_mut() + .decode_current_frame_to_vec(&compressed, output) + .map_err(|e| Error::new(ErrorKind::Other, alloc::boxed::Box::new(e)))?; + return Ok(()); + } + loop { + let start = output.len(); + output.resize(start + MAX_BLOCK_SIZE as usize, 0); + let n = self.read(&mut output[start..])?; + output.truncate(start + n); + if n == 0 { + break; + } + } + Ok(()) + } } #[cfg(test)] @@ -269,4 +339,62 @@ mod tests { .expect_err("deferred checksum mismatch must surface on the terminating read"); assert_eq!(err.kind(), ErrorKind::Other); } + + /// A fresh `read_to_end` must take the single-copy decode-in-place path + /// (FCS-declared frame decoded straight into the output `Vec`, no ring + /// drain) AND reproduce the payload byte-for-byte. + #[cfg(feature = "std")] + #[test] + fn read_to_end_decode_in_place_matches_and_takes_direct_path() { + use crate::encoding::{CompressionLevel, FrameCompressor}; + use alloc::vec::Vec; + + let payload: Vec = (0..20_000u32) + .map(|i| (i.wrapping_mul(2654435761) >> 24) as u8) + .collect(); + let mut compressor = FrameCompressor::new(CompressionLevel::Default); + compressor.set_source(payload.as_slice()); + let mut compressed = Vec::new(); + compressor.set_drain(&mut compressed); + compressor.compress(); + + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + let mut out = Vec::new(); + let n = decoder.read_to_end(&mut out).unwrap(); + assert_eq!(n, payload.len()); + assert_eq!(out, payload); + // FrameCompressor declares FCS, so the fresh fast path used the direct + // (decode-in-place) route, not the ring drain. + assert_eq!(decoder.decoder.direct_frames(), 1); + } + + /// `read_to_end` after a partial `read` must still produce the full + /// payload. The decoder is mid-frame, so the fast path is skipped and the + /// generic grow-and-drain fallback runs (no direct frame). + #[cfg(feature = "std")] + #[test] + fn read_to_end_after_partial_read_is_complete() { + use crate::encoding::{CompressionLevel, FrameCompressor}; + use alloc::vec; + use alloc::vec::Vec; + + let payload: Vec = (0..20_000u32).map(|i| (i & 0xFF) as u8).collect(); + let mut compressor = FrameCompressor::new(CompressionLevel::Default); + compressor.set_source(payload.as_slice()); + let mut compressed = Vec::new(); + compressor.set_drain(&mut compressed); + compressor.compress(); + + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + let mut head = vec![0u8; 4096]; + let got = decoder.read(&mut head).unwrap(); + assert!(got > 0 && got <= head.len()); + + let mut out = Vec::new(); + out.extend_from_slice(&head[..got]); + decoder.read_to_end(&mut out).unwrap(); + assert_eq!(out, payload); + // Mid-frame entry → fallback path, never the direct route. + assert_eq!(decoder.decoder.direct_frames(), 0); + } } From 1a6e4c9683e3bcf6e97347e9a74178f37ac2eba0 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Tue, 16 Jun 2026 22:48:01 +0300 Subject: [PATCH 02/12] fix(decode): multi-frame read_to_end, FCS-0 validation, checked sizing Review fixes for the decode-in-place read_to_end fast path: - read_to_end now decodes ALL frames in the source (concatenated + skippable), not just the current one. The fast path buffers the whole source per the Read::read_to_end "read to EOF" contract, so decoding only the first frame silently dropped trailing frames. decode_current_ frame_to_vec loops: decode the already-init'd frame, then init+decode the rest. Regression test feeds two concatenated frames (fails pre-fix: only the first decoded). - declared content size is validated against produced bytes in the ring branch (fcs_declared && produced != content_size -> FrameContentSizeMismatch), matching decode_all_impl. A frame with an explicit Frame_Content_Size=0 whose body emits bytes is now rejected instead of accepted. - direct-path sizing goes through usize::try_from + checked_add instead of `as usize`, so a 32-bit / oversized declared FCS falls back to the window-bounded ring drain rather than allocating a truncated buffer that would violate run_direct_decode's precondition. (32-bit-only path; not reachable by a 64-bit test.) 854 tests pass (incl. new multi-frame + empty-FCS-0 streaming tests), clippy + fmt clean, no_std builds. --- zstd/src/decoding/frame_decoder.rs | 90 ++++++++++++++++++++------ zstd/src/decoding/streaming_decoder.rs | 51 +++++++++++++++ 2 files changed, 121 insertions(+), 20 deletions(-) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 74165835e..94f9e0b4d 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -2342,31 +2342,70 @@ impl FrameDecoder { output: &mut Vec, ) -> Result { let start_len = output.len(); - let content_size = self - .state - .as_ref() - .expect("caller ensures is_at_frame_start") - .frame_header - .frame_content_size(); - if content_size > 0 { - // FCS declared and non-empty: reserve exactly the frame's content - // and decode straight into it (single copy, no ring). The direct - // path writes precisely `content_size` bytes and never reads past - // the current write position, so the grown region is fully written. - let cs = content_size as usize; - output.resize(start_len + cs, 0); + // The current frame is already initialised (its header consumed by the + // caller). Decode it, then decode any FOLLOWING concatenated / skippable + // frames in `input` so the whole source is consumed to EOF and nothing + // is dropped (matching `read_to_end` semantics). + self.decode_one_frame_to_vec(&mut input, output)?; + while !input.is_empty() { + match self.init(&mut input) { + Ok(_) => {} + Err(FrameDecoderError::ReadFrameHeaderError( + crate::decoding::errors::ReadFrameHeaderError::SkipFrame { length, .. }, + )) => { + input = input + .get(length as usize..) + .ok_or(FrameDecoderError::FailedToSkipFrame)?; + continue; + } + Err(e) => return Err(e), + } + self.decode_one_frame_to_vec(&mut input, output)?; + } + Ok(output.len() - start_len) + } + + /// Decode the single CURRENT (already-initialised) frame, APPENDING to + /// `output`. Helper for [`Self::decode_current_frame_to_vec`]. + fn decode_one_frame_to_vec( + &mut self, + input: &mut &[u8], + output: &mut Vec, + ) -> Result { + let frame_start = output.len(); + let (content_size, fcs_declared) = { + let s = self.state.as_ref().expect("frame is initialised"); + ( + s.frame_header.frame_content_size(), + s.frame_header.fcs_declared(), + ) + }; + // Direct path: a declared, non-empty content size that FITS in `usize` + // (and whose end offset does not overflow). `usize::try_from` guards the + // 32-bit / oversized-FCS truncation; an unrepresentable size falls + // through to the window-bounded ring drain rather than allocating a + // truncated buffer that would violate `run_direct_decode`'s precondition. + if content_size > 0 + && let Ok(cs) = usize::try_from(content_size) + && let Some(frame_end) = frame_start.checked_add(cs) + { + // Reserve exactly the frame's content and decode straight into it + // (single copy, no ring). The direct path writes precisely + // `content_size` bytes (erroring otherwise), so the grown region is + // fully written. + output.resize(frame_end, 0); let written = - self.run_direct_decode(&mut input, &mut output[start_len..], content_size)?; - output.truncate(start_len + written); + self.run_direct_decode(&mut *input, &mut output[frame_start..], content_size)?; + output.truncate(frame_start + written); #[cfg(feature = "hash")] self.verify_content_checksum()?; return Ok(written); } - // No declared size (or explicit FCS=0): window-bounded ring drain, - // appended directly to `output` via `collect_to_writer` (no staging - // buffer). Empty FCS=0 frames append nothing. + // No declared size, explicit FCS=0, or an unrepresentable FCS: window- + // bounded ring drain, appended directly to `output` via + // `collect_to_writer` (no staging buffer). loop { - self.decode_blocks(&mut input, BlockDecodingStrategy::UptoBytes(1024 * 1024))?; + self.decode_blocks(&mut *input, BlockDecodingStrategy::UptoBytes(1024 * 1024))?; self.collect_to_writer(&mut *output) .map_err(FrameDecoderError::FailedToDrainDecodebuffer)?; if self.is_finished() { @@ -2376,9 +2415,20 @@ impl FrameDecoder { break; } } + let produced = (output.len() - frame_start) as u64; + // A declared content size MUST match what the body produced — otherwise + // accept the same corrupt frames `decode_all_impl` rejects (e.g. an + // explicit FCS=0 whose body emits bytes). Use `fcs_declared()` so an + // on-wire FCS=0 is validated, while an unknown size is not. + if fcs_declared && produced != content_size { + return Err(FrameDecoderError::FrameContentSizeMismatch { + declared: content_size, + produced, + }); + } #[cfg(feature = "hash")] self.verify_content_checksum()?; - Ok(output.len() - start_len) + Ok(produced as usize) } /// Default-feature decode_all_impl: no visitor parameter so the diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index eb2bd50b8..f8937a532 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -220,6 +220,13 @@ impl> Read for StreamingDecoder) -> Result { // `new()` already read the frame header, so the fast path applies when @@ -397,4 +404,48 @@ mod tests { // Mid-frame entry → fallback path, never the direct route. assert_eq!(decoder.decoder.direct_frames(), 0); } + + /// `read_to_end` reads the WHOLE source to EOF: a stream of concatenated + /// frames must decode every frame, not just the first. (The fast path + /// buffers the whole source, so dropping the trailing frame would lose + /// data.) + #[cfg(feature = "std")] + #[test] + fn read_to_end_decodes_all_concatenated_frames() { + use crate::encoding::{CompressionLevel, compress_slice_to_vec}; + use alloc::vec::Vec; + + let a: Vec = (0..5000u32).map(|i| (i & 0xFF) as u8).collect(); + let b: Vec = (0..3000u32) + .map(|i| ((i.wrapping_mul(7)) & 0xFF) as u8) + .collect(); + let mut stream = compress_slice_to_vec(&a, CompressionLevel::Level(3)); + stream.extend_from_slice(&compress_slice_to_vec(&b, CompressionLevel::Level(3))); + + let mut decoder = StreamingDecoder::new(stream.as_slice()).unwrap(); + let mut out = Vec::new(); + decoder.read_to_end(&mut out).unwrap(); + + let mut expected = a.clone(); + expected.extend_from_slice(&b); + assert_eq!(out, expected); + // Both FCS-declared frames took the direct path. + assert_eq!(decoder.decoder.direct_frames(), 2); + } + + /// An empty (`Frame_Content_Size = 0`) frame decodes to nothing through the + /// `read_to_end` fast path — the declared-size validation accepts the valid + /// case (produced == 0) instead of erroring. + #[cfg(feature = "std")] + #[test] + fn read_to_end_empty_frame_decodes_to_empty() { + use crate::encoding::{CompressionLevel, compress_slice_to_vec}; + use alloc::vec::Vec; + + let compressed = compress_slice_to_vec(&[], CompressionLevel::Level(3)); + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + let mut out = Vec::new(); + decoder.read_to_end(&mut out).unwrap(); + assert!(out.is_empty()); + } } From 7bda2a24beb68de1079689cff3ad9749aeb3ec01 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 00:56:40 +0300 Subject: [PATCH 03/12] test(decode): regression tests for read_to_end concatenated/dict/error paths Three failing tests pinning bugs in the read_to_end fast path: - concatenated dictionary frames lose the forced dictionary on re-init (frame 2 fails with DictNotProvided) - read_to_end after a partial read stops at the current frame's EOF instead of consuming concatenated frames to true EOF - a direct-path decode error leaves the resized (zeroed) output tail in place, exposing non-decoded bytes to callers --- zstd/src/decoding/streaming_decoder.rs | 108 +++++++++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index f8937a532..0a13e82df 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -433,6 +433,114 @@ mod tests { assert_eq!(decoder.decoder.direct_frames(), 2); } + /// `read_to_end` after a partial `read` must STILL consume the source to + /// EOF across concatenated frames, not stop at the current frame's end. The + /// partial read forces the mid-frame fallback path; with two concatenated + /// frames the fallback must finish frame 1, then advance through frame 2. + #[cfg(feature = "std")] + #[test] + fn read_to_end_after_partial_read_decodes_all_concatenated_frames() { + use crate::encoding::{CompressionLevel, compress_slice_to_vec}; + use alloc::vec; + use alloc::vec::Vec; + + let a: Vec = (0..6000u32).map(|i| (i & 0xFF) as u8).collect(); + let b: Vec = (0..4000u32) + .map(|i| ((i.wrapping_mul(11)) & 0xFF) as u8) + .collect(); + let mut stream = compress_slice_to_vec(&a, CompressionLevel::Level(3)); + stream.extend_from_slice(&compress_slice_to_vec(&b, CompressionLevel::Level(3))); + + let mut decoder = StreamingDecoder::new(stream.as_slice()).unwrap(); + // Partial read of frame 1 → mid-frame, so read_to_end takes the fallback. + let mut head = vec![0u8; 2048]; + let got = decoder.read(&mut head).unwrap(); + assert!(got > 0 && got <= head.len()); + + let mut out = Vec::new(); + out.extend_from_slice(&head[..got]); + decoder.read_to_end(&mut out).unwrap(); + + let mut expected = a.clone(); + expected.extend_from_slice(&b); + assert_eq!( + out, expected, + "fallback path must decode frame 2 too, not stop at frame 1 EOF" + ); + } + + /// `read_to_end` on a stream of concatenated DICTIONARY frames must decode + /// every frame WITH the dictionary the decoder was constructed with. The + /// fast-path concatenated loop re-initialises following frames, and a plain + /// re-init resolves dictionaries by frame id only — losing the forced + /// dictionary for frames that omit (or can't resolve) the id. + #[cfg(feature = "std")] + #[test] + fn read_to_end_concatenated_dict_frames_decode_with_dictionary() { + use crate::encoding::{CompressionLevel, FrameCompressor}; + use alloc::vec::Vec; + + let dict_raw = include_bytes!("../../dict_tests/dictionary"); + let compress_with_dict = |payload: &[u8]| -> Vec { + let mut compressor = FrameCompressor::new(CompressionLevel::Default); + compressor + .set_dictionary_from_bytes(dict_raw) + .expect("dict load"); + compressor.set_source(payload); + let mut compressed = Vec::new(); + compressor.set_drain(&mut compressed); + compressor.compress(); + compressed + }; + + let a = b"first dictionary-compressed frame payload".to_vec(); + let b = b"second dictionary-compressed frame payload".to_vec(); + let mut stream = compress_with_dict(&a); + stream.extend_from_slice(&compress_with_dict(&b)); + + let mut decoder = + StreamingDecoder::new_with_dictionary_bytes(stream.as_slice(), dict_raw).unwrap(); + let mut out = Vec::new(); + decoder + .read_to_end(&mut out) + .expect("both dict frames must decode with the forced dictionary"); + + let mut expected = a.clone(); + expected.extend_from_slice(&b); + assert_eq!(out, expected); + } + + /// A direct-path decode error must NOT leave non-decoded bytes in `output`. + /// The fast path resizes `output` to the declared content size before + /// decoding; if decode fails, the enlarged (zeroed) tail must be truncated + /// away so callers never observe bytes that were never decoded. + #[cfg(feature = "std")] + #[test] + fn read_to_end_truncates_output_on_direct_decode_error() { + use crate::encoding::{CompressionLevel, FrameCompressor}; + use alloc::vec::Vec; + + let payload: Vec = (0..5000u32).map(|i| (i & 0xFF) as u8).collect(); + let mut compressor = FrameCompressor::new(CompressionLevel::Default); + compressor.set_source(payload.as_slice()); + let mut compressed = Vec::new(); + compressor.set_drain(&mut compressed); + compressor.compress(); + // Truncate the block bytes (the FCS-bearing header at the front stays + // intact) so the header parses but the direct-path block decode hits a + // premature end → error after `output` was already resized. + compressed.truncate(compressed.len() - 40); + + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + let mut out = b"SENTINEL".to_vec(); + let result = decoder.read_to_end(&mut out); + assert!(result.is_err(), "truncated block must fail the decode"); + assert_eq!( + out, b"SENTINEL", + "failed direct decode must not append non-decoded bytes to output" + ); + } + /// An empty (`Frame_Content_Size = 0`) frame decodes to nothing through the /// `read_to_end` fast path — the declared-size validation accepts the valid /// case (produced == 0) instead of erroring. From d1af8558aef7ea967eec971717073f3264bddf55 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 01:00:50 +0300 Subject: [PATCH 04/12] fix(decode): preserve dictionary + consume to EOF in read_to_end Three correctness fixes in the read_to_end decode-in-place paths: - Concatenated frames now re-initialise via init_with_dict_handle when the StreamingDecoder was built with a dictionary, so a forced dict is preserved for following frames (plain init resolves dicts by frame id only, losing the dict for frames that omit the id). StreamingDecoder retains the (cheap Arc/Rc) dict handle for this; the shared init+decode loop is factored into FrameDecoder::decode_concatenated_frames_to_vec. - The mid-frame fallback (mixed read + read_to_end) now drains the partial current frame and then decodes the remaining concatenated frames to true EOF, matching the fast path and the Read::read_to_end contract. - A direct-path decode error truncates the just-grown output tail before propagating, so callers never observe zeroed non-decoded bytes. --- zstd/src/decoding/frame_decoder.rs | 49 +++++++++++++++--- zstd/src/decoding/streaming_decoder.rs | 69 +++++++++++++++++++++----- 2 files changed, 99 insertions(+), 19 deletions(-) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 94f9e0b4d..348cd5630 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -2340,27 +2340,53 @@ impl FrameDecoder { &mut self, mut input: &[u8], output: &mut Vec, + dict: Option<&DictionaryHandle>, ) -> Result { let start_len = output.len(); // The current frame is already initialised (its header consumed by the - // caller). Decode it, then decode any FOLLOWING concatenated / skippable - // frames in `input` so the whole source is consumed to EOF and nothing - // is dropped (matching `read_to_end` semantics). + // caller, WITH `dict` applied if the decoder was constructed with one). + // Decode it, then decode any FOLLOWING concatenated / skippable frames + // in `input` so the whole source is consumed to EOF and nothing is + // dropped (matching `read_to_end` semantics). self.decode_one_frame_to_vec(&mut input, output)?; + self.decode_concatenated_frames_to_vec(&mut input, output, dict)?; + Ok(output.len() - start_len) + } + + /// Initialise and decode every frame remaining in `input` (concatenated / + /// skippable), APPENDING to `output`. `input` is advanced as frames are + /// consumed; on return it is empty. Re-initialisation honours `dict`: when + /// `Some`, each following frame is initialised via + /// [`Self::init_with_dict_handle`] so a forced dictionary is preserved even + /// for frames that omit the dictionary id (plain [`Self::init`] would + /// resolve dictionaries by id only). Backs the `read_to_end` fast path (the + /// frames after the current one) and its mid-frame fallback (the frames + /// after the partially-read one). + pub(crate) fn decode_concatenated_frames_to_vec( + &mut self, + input: &mut &[u8], + output: &mut Vec, + dict: Option<&DictionaryHandle>, + ) -> Result { + let start_len = output.len(); while !input.is_empty() { - match self.init(&mut input) { + let init_result = match dict { + Some(d) => self.init_with_dict_handle(&mut *input, d), + None => self.init(&mut *input), + }; + match init_result { Ok(_) => {} Err(FrameDecoderError::ReadFrameHeaderError( crate::decoding::errors::ReadFrameHeaderError::SkipFrame { length, .. }, )) => { - input = input + *input = input .get(length as usize..) .ok_or(FrameDecoderError::FailedToSkipFrame)?; continue; } Err(e) => return Err(e), } - self.decode_one_frame_to_vec(&mut input, output)?; + self.decode_one_frame_to_vec(&mut *input, output)?; } Ok(output.len() - start_len) } @@ -2394,8 +2420,17 @@ impl FrameDecoder { // `content_size` bytes (erroring otherwise), so the grown region is // fully written. output.resize(frame_end, 0); + // On error, drop the just-grown (zeroed) tail before propagating so + // callers never observe bytes that were never decoded. let written = - self.run_direct_decode(&mut *input, &mut output[frame_start..], content_size)?; + match self.run_direct_decode(&mut *input, &mut output[frame_start..], content_size) + { + Ok(n) => n, + Err(e) => { + output.truncate(frame_start); + return Err(e); + } + }; output.truncate(frame_start + written); #[cfg(feature = "hash")] self.verify_content_checksum()?; diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index 0a13e82df..114bdd3d8 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -46,6 +46,12 @@ use crate::io::{Error, Read}; pub struct StreamingDecoder> { pub decoder: DEC, source: READ, + /// Dictionary the decoder was constructed with, if any. Retained so the + /// `read_to_end` paths can re-initialise FOLLOWING concatenated frames with + /// the same forced dictionary (a plain re-init resolves dictionaries by + /// frame id only and would lose a forced dict for frames omitting the id). + /// Cheap to hold: `DictionaryHandle` is an `Arc`/`Rc` handle. + dict: Option, } impl> StreamingDecoder { @@ -54,7 +60,11 @@ impl> StreamingDecoder { mut decoder: DEC, ) -> Result, FrameDecoderError> { decoder.borrow_mut().init(&mut source)?; - Ok(StreamingDecoder { decoder, source }) + Ok(StreamingDecoder { + decoder, + source, + dict: None, + }) } } @@ -64,7 +74,11 @@ impl StreamingDecoder { ) -> Result, FrameDecoderError> { let mut decoder = FrameDecoder::new(); decoder.init(&mut source)?; - Ok(StreamingDecoder { decoder, source }) + Ok(StreamingDecoder { + decoder, + source, + dict: None, + }) } /// Create a streaming decoder using a pre-parsed dictionary handle. @@ -82,7 +96,11 @@ impl StreamingDecoder { ) -> Result, FrameDecoderError> { let mut decoder = FrameDecoder::new(); decoder.init_with_dict_handle(&mut source, dict)?; - Ok(StreamingDecoder { decoder, source }) + Ok(StreamingDecoder { + decoder, + source, + dict: Some(dict.clone()), + }) } /// Create a streaming decoder using a serialized dictionary blob. @@ -229,24 +247,28 @@ impl> Read for StreamingDecoder) -> Result { + let start_total = output.len(); // `new()` already read the frame header, so the fast path applies when // the decoder sits at the start of that frame with nothing decoded yet. let at_start = { let d = self.decoder.borrow_mut(); d.is_at_frame_start() && d.can_collect() == 0 }; + // Clone the (cheap Arc/Rc) dict handle out so the `decoder` borrow below + // does not conflict with borrowing `self.dict`. + let dict = self.dict.clone(); if at_start { let mut compressed = alloc::vec::Vec::new(); self.source.read_to_end(&mut compressed)?; - let written = self - .decoder + self.decoder .borrow_mut() - .decode_current_frame_to_vec(&compressed, output) + .decode_current_frame_to_vec(&compressed, output, dict.as_ref()) .map_err(Error::other)?; - return Ok(written); + return Ok(output.len() - start_total); } - // Mid-frame fallback: grow `output` and drain through the generic path. - let mut total = 0; + // Mid-frame fallback: drain the partially-read CURRENT frame through the + // generic path, then decode any FOLLOWING concatenated frames so + // read_to_end still consumes the source to true EOF. loop { let start = output.len(); output.resize(start + MAX_BLOCK_SIZE as usize, 0); @@ -255,9 +277,18 @@ impl> Read for StreamingDecoder> Read for StreamingDecoder> Read for StreamingDecoder Date: Wed, 17 Jun 2026 08:38:52 +0300 Subject: [PATCH 05/12] docs(decode): fix misattributed doc blocks on frame decoder - Split is_finished() doc from direct_frames(): the three drain-related /// lines were attached to the test-only direct_frames() accessor, leaving is_finished() undocumented. Move direct_frames() doc above it, restore is_finished()'s own block. - Remove duplicated frame-decode prose wrongly prefixed onto is_at_frame_start(); that behaviour is already documented on decode_current_frame_to_vec / decode_concatenated_frames_to_vec. --- zstd/src/decoding/frame_decoder.rs | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 348cd5630..d08e49ae2 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -1453,9 +1453,6 @@ impl FrameDecoder { state.bytes_read_counter } - /// Whether the current frames last block has been decoded yet - /// If this returns true you can call the drain* functions to get all content - /// (the read() function will drain automatically if this returns true) /// Test-only: number of frames decoded through the single-copy direct /// path (`run_direct_decode`). Lets cross-module tests assert that a /// given decode took the decode-in-place path rather than the ring drain. @@ -1464,6 +1461,9 @@ impl FrameDecoder { self.direct_frames } + /// Whether the current frames last block has been decoded yet + /// If this returns true you can call the drain* functions to get all content + /// (the read() function will drain automatically if this returns true) pub fn is_finished(&self) -> bool { let state = match &self.state { None => return true, @@ -2287,24 +2287,6 @@ impl FrameDecoder { } } - /// Decode every frame in `input`, APPENDING the decompressed bytes to - /// `output` (growing it as needed), and return the number of bytes - /// appended. - /// - /// This is the `Vec`-output sibling of [`Self::decode_all`]. A frame that - /// declares its content size decodes DIRECTLY into freshly-reserved - /// `output` capacity through the single-copy direct path - /// ([`Self::run_direct_decode`]) — bypassing the `Ring`/`FlatBuf` → - /// `read()` drain copy that the streaming loop pays. A frame WITHOUT a - /// declared size falls back to the window-bounded ring drain (still one - /// copy, into `output`). Skippable frames are skipped. - /// - /// Each frame is (re)initialised via [`Self::init`] (resolving any - /// attached dictionary by id), so the decoder must be at a frame boundary - /// (no partially-decoded frame in flight) when this is called. It backs - /// [`StreamingDecoder`](crate::decoding::StreamingDecoder)'s `read_to_end` - /// fast path. - /// /// Whether the decoder sits at the very start of an initialised frame: /// the header has been read (state populated) but no block has been /// decoded and the frame is not finished. In this state the wrapped From 23b8f6fcfb1296a883f03092776ebba0c7f02aaf Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 09:43:57 +0300 Subject: [PATCH 06/12] test(decode): regression for eager FCS allocation on untrusted frame A frame declaring a large content size with a tiny truncated body must not drive the direct path's output.resize() before the body is validated. Asserts decode_current_frame_to_vec falls to the ring drain (direct_frames stays 0) for an implausible declared size. Fails on current code (direct_frames == 1). --- zstd/src/decoding/frame_decoder.rs | 36 ++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index d08e49ae2..7e10c922b 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -3986,6 +3986,42 @@ mod tests { ); } + #[test] + fn implausible_content_size_skips_eager_alloc_direct_path() { + // Adversarial frame: a 1 KiB window (small ring) but a declared + // content size of 4 MiB, followed by a truncated raw block. The + // direct path would `resize` the caller's Vec to the pledged 4 MiB + // (allocating + zeroing it) BEFORE the truncated body is validated. + // The gate must reject the implausible size (4 MiB cannot come from + // 3 compressed bytes) and fall through to the window-bounded ring + // drain, which errors without ever allocating the pledged size. + // + // Hand-built so the declared size is fully decoupled from the real + // (tiny) input — the encoder always writes a truthful FCS. + let frame: &[u8] = &[ + 0x28, 0xB5, 0x2F, 0xFD, // magic + 0x80, // FHD: multi-segment, 4-byte FCS field, no dict + 0x00, // window descriptor -> 1 KiB window + 0x00, 0x00, 0x40, 0x00, // FCS = 4 MiB + 0x21, 0x03, 0x00, // raw block header: last, size 100, no body + ]; + + let mut dec = FrameDecoder::new(); + let mut src = frame; + dec.init(&mut src).expect("header must parse"); + // `src` now points past the header at the truncated 3-byte block. + let mut out = Vec::new(); + let err = dec.decode_current_frame_to_vec(src, &mut out, None); + assert!( + err.is_err(), + "truncated body must fail regardless of decode path" + ); + assert_eq!( + dec.direct_frames, 0, + "implausible FCS must NOT take the eager-alloc direct path" + ); + } + #[cfg(feature = "lsm")] mod expect_validation { use super::*; From 30b432c7672cf9a03f16bfcfc788ec48203981f7 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 09:44:45 +0300 Subject: [PATCH 07/12] fix(decode): gate direct-path pre-sizing on plausible content size The decode-in-place path resized the output Vec to the frame's declared content size before decoding/validating the body, so a tiny or truncated frame declaring a huge (usize-representable) FCS allocated and zeroed that whole size up front. Bound the eager resize to what the available compressed input could plausibly produce (input.len() * MAX_BLOCK_SIZE/4, zstd's per-block ceiling); larger declarations fall through to the window-bounded ring drain, which grows only as real bytes are produced and errors cheaply on truncated input. --- zstd/src/decoding/frame_decoder.rs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 7e10c922b..14e1d3ea2 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -2393,8 +2393,23 @@ impl FrameDecoder { // 32-bit / oversized-FCS truncation; an unrepresentable size falls // through to the window-bounded ring drain rather than allocating a // truncated buffer that would violate `run_direct_decode`'s precondition. + // + // Plausibility gate: the direct path `resize`s `output` to the declared + // size up front, so a tiny/truncated frame declaring a huge (but + // representable) FCS would allocate + zero that whole size before the + // body is validated. zstd's per-block ceiling is MAX_BLOCK_SIZE from as + // little as ~4 input bytes, so the declared size cannot legitimately + // exceed `input.len() * (MAX_BLOCK_SIZE / 4)`. Anything larger falls + // through to the ring drain, which grows only as real bytes are produced + // and errors out cheaply on truncated input. `input` spans the remaining + // source (this frame plus any following ones), so the bound only ever + // over-permits — a legitimate frame is never forced off the direct path. + // saturating_mul is intentional: an overflow means the available input + // is so large that any representable FCS is plausible (cap = "no limit"). + const MAX_DECOMPRESSION_RATIO: usize = (crate::common::MAX_BLOCK_SIZE / 4) as usize; if content_size > 0 && let Ok(cs) = usize::try_from(content_size) + && cs <= input.len().saturating_mul(MAX_DECOMPRESSION_RATIO) && let Some(frame_end) = frame_start.checked_add(cs) { // Reserve exactly the frame's content and decode straight into it From bea93d514a3d01324a4bf856eba8a63ac518e412 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 09:50:19 +0300 Subject: [PATCH 08/12] test(decode): regression for zero-filled tail on fallback read error read_to_end's mid-frame fallback grows output by MAX_BLOCK_SIZE before each self.read; a read error must not leave that zeroed tail in the caller's Vec. Forces a sub-input window so a partial read leaves the frame mid-decode with a truncated block ahead, then asserts the decoded prefix matches the payload (no appended non-decoded bytes). Fails on current code (zero tail mismatches the incompressible payload). --- zstd/src/decoding/streaming_decoder.rs | 61 ++++++++++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index 114bdd3d8..c9f45c278 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -586,6 +586,67 @@ mod tests { ); } + /// The mid-frame fallback grows `output` by `MAX_BLOCK_SIZE` before each + /// `self.read`. When that read errors (truncated current frame), the grown + /// zero-filled tail must be truncated away before the error propagates, so + /// the caller never observes `MAX_BLOCK_SIZE` worth of bytes that were never + /// decoded. + #[cfg(feature = "std")] + #[test] + fn read_to_end_truncates_output_on_midframe_fallback_error() { + use crate::encoding::{CompressionLevel, CompressionParameters, FrameCompressor}; + use alloc::vec; + use alloc::vec::Vec; + + // Incompressible payload with a window (128 KiB) SMALLER than the input, + // so the frame holds several blocks and bytes become collectable while + // the frame is still mid-decode. Without a sub-input window the decoder + // retains the whole input until the frame finishes, and a partial read + // could only ever finish or error, never leave a truncated remainder for + // the fallback to trip on. + let payload: Vec = (0..320_000u32) + .map(|i| (i.wrapping_mul(2654435761) >> 24) as u8) + .collect(); + let params = CompressionParameters::builder(CompressionLevel::Default) + .window_log(17) + .build() + .expect("window_log within bounds"); + let mut compressor = FrameCompressor::new(CompressionLevel::Default); + compressor.set_parameters(¶ms); + compressor.set_source(payload.as_slice()); + let mut compressed = Vec::new(); + compressor.set_drain(&mut compressed); + compressor.compress(); + // Truncate the tail so the final block decode fails partway through. + compressed.truncate(compressed.len() - 40); + + let mut decoder = StreamingDecoder::new(compressed.as_slice()).unwrap(); + // A partial `read` first: leaves the decoder mid-frame so `read_to_end` + // takes the grow-and-drain fallback (not the decode-in-place fast path). + let mut head = vec![0u8; 4096]; + let got = decoder.read(&mut head).unwrap(); + assert!(got > 0); + + let mut out = Vec::new(); + out.extend_from_slice(&head[..got]); + let result = decoder.read_to_end(&mut out); + assert!( + result.is_err(), + "truncated current frame must fail the decode" + ); + assert!( + out.len() <= payload.len(), + "failed fallback read must not leave a zero-filled tail (len {} > payload {})", + out.len(), + payload.len() + ); + assert_eq!( + out.as_slice(), + &payload[..out.len()], + "decoded prefix must match the payload, with no appended non-decoded bytes" + ); + } + /// An empty (`Frame_Content_Size = 0`) frame decodes to nothing through the /// `read_to_end` fast path — the declared-size validation accepts the valid /// case (produced == 0) instead of erroring. From c006d431ec5c85ca1e4485df083d51b6857db909 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 09:50:49 +0300 Subject: [PATCH 09/12] fix(decode): truncate output on read error in read_to_end fallback The mid-frame fallback grew output by MAX_BLOCK_SIZE before each self.read, but the `?` early-return on a read error left that zeroed tail in the caller's Vec. Restore the length to the pre-grow point before propagating the error, in both the std and no_std paths. --- zstd/src/decoding/streaming_decoder.rs | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index c9f45c278..bea8198a7 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -272,7 +272,15 @@ impl> Read for StreamingDecoder n, + Err(e) => { + output.truncate(start); + return Err(e); + } + }; output.truncate(start + n); if n == 0 { break; @@ -316,7 +324,15 @@ impl> Read for StreamingDecoder n, + Err(e) => { + output.truncate(start); + return Err(e); + } + }; output.truncate(start + n); if n == 0 { break; From 5769d29f75fd7e84dc37580eb65906b0917d62e0 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 09:51:33 +0300 Subject: [PATCH 10/12] docs(decode): clarify StreamingDecoder read vs read_to_end semantics The module caveat predated the read_to_end specialization: it claimed the decoder handles only a single frame and that no_std lacks read_to_end. Distinguish plain read / read_exact (single frame) from read_to_end (consumes concatenated + skippable frames to EOF), and drop the stale no_std note (read_to_end is implemented there too; only File in the example is std-only). --- zstd/src/decoding/streaming_decoder.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/zstd/src/decoding/streaming_decoder.rs b/zstd/src/decoding/streaming_decoder.rs index bea8198a7..837a26fd7 100644 --- a/zstd/src/decoding/streaming_decoder.rs +++ b/zstd/src/decoding/streaming_decoder.rs @@ -20,16 +20,18 @@ use crate::io::{Error, Read}; /// [FrameDecoder::decode_blocks] repeatedly to decode the entire frame. /// /// ## Caveat -/// [StreamingDecoder] expects the underlying stream to only contain a single frame, -/// yet the specification states that a single archive may contain multiple frames. +/// Plain `read` / `read_exact` operate on the single frame this decoder was +/// initialised with: they do not advance into following frames. `read_to_end`, +/// by contrast, is specialised to consume a finite source to EOF, decoding +/// concatenated frames and skipping skippable frames along the way. /// -/// To decode all the frames in a finite stream, the calling code needs to recreate -/// the instance of the decoder and handle +/// To recover the bytes that follow one frame WITHOUT consuming the rest of the +/// source, recreate the decoder manually and handle /// [crate::decoding::errors::ReadFrameHeaderError::SkipFrame] /// errors by skipping forward the `length` amount of bytes, see /// /// ```no_run -/// // `read_to_end` is not implemented by the no_std implementation. +/// // `File` is std-only; `read_to_end` itself is available under no_std too. /// #[cfg(feature = "std")] /// { /// use std::fs::File; From 6ad6086f525b96a4fce1ef6b7a01455579847e49 Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 10:35:01 +0300 Subject: [PATCH 11/12] test(decode): regression for single-segment implausible FCS reservation The direct-path gate routes an implausible declared size to the ring drain, but decode_blocks pre-reserves useful_window_size(), which for a single-segment frame equals the declared FCS. A truncated single-segment frame declaring a huge content size still allocates that window before the body errors. Asserts the implausible size is rejected up front with a content-size error rather than a post-reservation block-body error. Fails on current code (returns FailedToReadBlockBody after reserving). --- zstd/src/decoding/frame_decoder.rs | 37 ++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 14e1d3ea2..89719cfd4 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -4037,6 +4037,43 @@ mod tests { ); } + #[test] + fn implausible_single_segment_fcs_rejected_before_window_reservation() { + // Single-segment adversarial frame: the window equals the declared + // content size (4 MiB) by definition, so the fallback ring drain would + // pre-reserve that whole window via `useful_window_size()` before the + // truncated body errors — the multi-segment gate test does not cover + // this. The implausible size (4 MiB cannot come from 3 compressed + // bytes) must be rejected up front with a content-size error, NOT a + // block-body error after the reservation. + let frame: &[u8] = &[ + 0x28, 0xB5, 0x2F, 0xFD, // magic + 0xA0, // FHD: single-segment, 4-byte FCS field + 0x00, 0x00, 0x40, 0x00, // FCS = 4 MiB (== window for single-segment) + 0x21, 0x03, 0x00, // raw block header: last, size 100, no body + ]; + + let mut dec = FrameDecoder::new(); + let mut src = frame; + dec.init(&mut src).expect("header must parse"); + let mut out = Vec::new(); + let err = dec + .decode_current_frame_to_vec(src, &mut out, None) + .expect_err("implausible single-segment FCS must be rejected"); + match err { + super::FrameDecoderError::FrameContentSizeMismatch { declared, .. } => { + assert_eq!(declared, 4 * 1024 * 1024); + } + other => panic!( + "expected early FrameContentSizeMismatch (no window reservation), got {other:?}" + ), + } + assert_eq!( + dec.direct_frames, 0, + "implausible FCS must not take the eager-alloc direct path" + ); + } + #[cfg(feature = "lsm")] mod expect_validation { use super::*; From 36c1b289c3676504b676160831fb9ee04973a47d Mon Sep 17 00:00:00 2001 From: Dmitry Prudnikov Date: Wed, 17 Jun 2026 10:36:09 +0300 Subject: [PATCH 12/12] fix(decode): reject implausible single-segment FCS before window reserve The direct-path plausibility gate routed an implausibly-large declared size to the ring drain, but decode_blocks pre-reserves useful_window_size() (= window.min(FCS)), which for a single-segment frame is the declared FCS itself. A truncated single-segment frame lying about its size therefore still allocated the pledged window before the body errored. Reject such an FCS-bearing frame up front when its useful window exceeds the input's plausible output (input.len() * per-block ceiling); small-window multi-segment frames still fall through to the ring drain and error cheaply on the truncated body. --- zstd/src/decoding/frame_decoder.rs | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/zstd/src/decoding/frame_decoder.rs b/zstd/src/decoding/frame_decoder.rs index 89719cfd4..57c19afab 100644 --- a/zstd/src/decoding/frame_decoder.rs +++ b/zstd/src/decoding/frame_decoder.rs @@ -2433,6 +2433,26 @@ impl FrameDecoder { self.verify_content_checksum()?; return Ok(written); } + // The ring-drain fallback below pre-reserves `useful_window_size()` + // (= `window.min(FCS)`), which for a single-segment frame is the + // declared FCS itself — so a truncated single-segment frame lying about + // its size would still allocate the pledged window before the body + // errors, sidestepping the direct-path gate above. Reject such a frame + // up front when its declared (FCS-bearing) window exceeds what the + // available input could plausibly produce. Frames without a declared + // size keep their window-descriptor reservation (already capped at + // `MAXIMUM_ALLOWED_WINDOW_SIZE` at init); a small-window multi-segment + // frame still falls through to the ring drain, which errors cheaply on + // the truncated body. + if fcs_declared + && let Some(state) = self.state.as_ref() + && state.useful_window_size() > input.len().saturating_mul(MAX_DECOMPRESSION_RATIO) + { + return Err(FrameDecoderError::FrameContentSizeMismatch { + declared: content_size, + produced: 0, + }); + } // No declared size, explicit FCS=0, or an unrepresentable FCS: window- // bounded ring drain, appended directly to `output` via // `collect_to_writer` (no staging buffer).