From 7c86e166bd007cf7d8b64ecfb3e72664e4ae7a5d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 21:08:18 -0800 Subject: [PATCH 1/2] craft-doc: check rendition copy costs before decompressing --- crates/doc/README.md | 21 +++++++------ crates/doc/src/pcraft.rs | 6 ++-- crates/doc/src/vectorcraft.rs | 57 ++++++++++++++++++++++++++--------- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/crates/doc/README.md b/crates/doc/README.md index 03ee07e1..cdec96ad 100644 --- a/crates/doc/README.md +++ b/crates/doc/README.md @@ -35,7 +35,8 @@ let outline = craft_doc::outline(&bytes, &limits)?; - `sniff(head) -> Option` reads the first bytes. The first 64 KiB are enough for every file the apps write. A re-zipped `.pcraft` whose `manifest.json` entry starts after the head is recognized only when `head` holds the whole file. When `head` holds a whole ZIP archive, the - names in its central directory decide. A VectorCraft file is recognized by its top-level + reader checks its central-directory names when the archive opens. Otherwise it checks its local + headers. A VectorCraft file is recognized by its top-level `format` member, as `craft_vectorcraft_format::sniff` recognizes it, so a file written as a JSON array, which `File::read` also reads, is not. - `info(bytes, &Limits) -> Result`: the kind, the version the file declares, the @@ -119,9 +120,10 @@ references, and the limits. manifest or the EffectCraft project. The names and other strings returned are parts of it. - `max_entry` (256 MiB): one rendition or one ZIP entry. The size a file states is checked before any of it is copied or inflated. -- `max_total` (1 GiB): every byte one call decompresses from gzip, reads out of ZIP entries or - returns as a rendition, together. A rendition decompressed from gzip is counted once, and a - second rendition of the same bytes is counted as another copy. +- `max_total` (1 GiB): the limit counts decompressed gzip bytes a call uses, bytes it reads out of + ZIP entries, and bytes it returns as renditions. A rendition decompressed from gzip is counted once, and a + second rendition of the same bytes is counted as another copy. The gzip reader may decode a + 64 KiB chunk past the JSON or final rendition. Unused bytes in that chunk do not count. - `max_depth` (400, also the largest value it takes, `MAX_DEPTH`): the nesting of JSON arrays and objects, checked before parsing. PhotoCraft nests up to 100 groups of layers, three levels each. `renditions` and `outline` refuse a PhotoCraft manifest nested deeper than 367 levels @@ -131,11 +133,12 @@ A PhotoCraft manifest with more than 4,194,304 JSON items (`craft_pcraft_format::MAX_MANIFEST_ITEMS`) is refused with `Error::LimitExceeded`, as PhotoCraft reads it. -On a compressed `.vectorcraft` file, `info` and `outline` decompress only as far as the end of -the JSON. `renditions` checks the size of each member after the JSON against `max_entry` -before it decompresses any of that data, and the offset where the last member ends against -`max_total`. It then decompresses only as far as the members end, and keeps only their bytes. The -struct is `#[non_exhaustive]`: start from the defaults and set the fields you +On a compressed `.vectorcraft` file, `info` and `outline` read the JSON from the decompressed +stream. The reader decodes chunks of up to 64 KiB, so the chunk holding the end of the JSON may also hold +data after its marker. `renditions` checks each member's size against `max_entry` and the last +member's ending offset plus duplicate-copy sizes against the remaining `max_total` budget before +reading the tail. Beyond the buffered chunk, it decompresses only as far as the members end and +keeps only their bytes. The struct is `#[non_exhaustive]`: start from the defaults and set the fields you change. ```rust diff --git a/crates/doc/src/pcraft.rs b/crates/doc/src/pcraft.rs index bfbb53ff..0d2fb297 100644 --- a/crates/doc/src/pcraft.rs +++ b/crates/doc/src/pcraft.rs @@ -14,9 +14,9 @@ use serde::de::{DeserializeOwned, Deserializer, IgnoredAny, MapAccess, Visitor}; use crate::zip::{Zip, local_names_include}; use crate::{Budget, Canvas, Error, Kind, Layer, Outline, Rendition, RenditionKind, counted, lenient, malformed, parse, too_new}; -/// A ZIP archive with a `manifest.json` entry. When `head` holds the whole archive, the names in -/// its central directory decide, as [`Zip::find`] reads them; otherwise the names in the local -/// headers that `head` holds. +/// The reader recognizes a ZIP archive with a `manifest.json` entry. When the archive opens, +/// [`Zip::find`] checks its central-directory names. Otherwise the reader checks the local headers +/// that `head` holds. pub(crate) fn sniff(head: &[u8]) -> bool { head.starts_with(b"PK\x03\x04") && match Zip::open(head) { diff --git a/crates/doc/src/vectorcraft.rs b/crates/doc/src/vectorcraft.rs index 83b06750..5472d2f4 100644 --- a/crates/doc/src/vectorcraft.rs +++ b/crates/doc/src/vectorcraft.rs @@ -64,10 +64,10 @@ enum Tail<'a> { } impl Tail<'_> { - /// The bytes each `[offset, length]` in `ranges` names, or `None` for a range the file ends - /// before. A compressed file is decompressed only as far as the last range ends, and that - /// offset is checked against the budget before anything past the marker is decompressed. - /// Every byte decompressed is counted; only the bytes inside the ranges are kept. + /// Each `[offset, length]` in `ranges` gives its bytes, or `None` if the file ends before it. + /// The final offset and duplicate-copy cost are checked against the remaining budget before + /// reading the tail. Bytes consumed from the tail and additional copies count against the + /// budget. Only bytes inside the ranges are kept. fn ranges(self, ranges: &[[u64; 2]], budget: &mut Budget) -> Result>>, Error> { const WHAT: &str = "the decompressed document"; let (mut dec, read) = match self { @@ -75,12 +75,6 @@ impl Tail<'_> { Tail::Stream { dec, read } => (dec, read), }; let end = ranges.iter().map(|&[offset, len]| offset.saturating_add(len)).max().unwrap_or(0); - if end > budget.left() { - return Err(Error::LimitExceeded(format!( - "the file states data up to byte {end} after its JSON, more than max_total ({} bytes) in all", - budget.limits.max_total - ))); - } // The ranges in file order, overlapping ones merged. let mut spans: Vec<[u64; 2]> = ranges.iter().map(|&[offset, len]| [offset, offset.saturating_add(len)]).collect(); spans.sort_unstable(); @@ -93,9 +87,14 @@ impl Tail<'_> { } // Each range is returned as its own copy: bytes that more than one range names are // decompressed once and counted again here for each further copy. - let stated = ranges.iter().fold(0u64, |n, &[_, len]| n.saturating_add(len)); + let over_budget = || Error::LimitExceeded(format!("{WHAT}: more than max_total ({} bytes) in all", budget.limits.max_total)); + let stated = ranges.iter().try_fold(0u64, |n, &[_, len]| n.checked_add(len)).ok_or_else(over_budget)?; let spanned = merged.iter().fold(0u64, |n, &[start, end]| n.saturating_add(end - start)); - budget.take(stated.saturating_sub(spanned), WHAT)?; + let copies = stated.saturating_sub(spanned); + if end.checked_add(copies).is_none_or(|required| required > budget.left()) { + return Err(over_budget()); + } + budget.take(copies, WHAT)?; let mut src = read.as_slice().chain(&mut *dec); let (mut at, mut kept) = (0u64, Vec::<(u64, Vec)>::new()); for [start, end] in merged { @@ -121,9 +120,8 @@ impl Tail<'_> { } } -/// A file's JSON and where the data after it is. A compressed file is decompressed only as far -/// as the end of the marker, every byte used counted against the budget, and its JSON is -/// checked against `max_json` as it grows. +/// The reader extracts a file's JSON. For gzip input, it buffers any tail bytes in the same chunk of up to 64 KiB. +/// JSON and marker bytes count against the budget. The JSON is checked against `max_json` as it grows. fn unpack<'a>(bytes: &'a [u8], budget: &mut Budget) -> Result, Error> { const JSON: &str = "the document's JSON"; let too_long = |budget: &Budget| Error::LimitExceeded(format!("{JSON} is more than max_json ({} bytes)", budget.limits.max_json)); @@ -554,6 +552,35 @@ mod tests { use crate::tests::{unread_array, unread_object}; use serde::de::value::{Error as ValueError, MapDeserializer, SeqDeserializer, StrDeserializer}; + #[test] + fn duplicate_copy_cost_is_checked_before_the_stream_is_read() { + let limits = crate::Limits { max_total: 12, ..Default::default() }; + let mut budget = Budget::new(&limits); + let tail = Tail::Stream { dec: Box::new(GzDecoder::new(&b"not a gzip stream"[..])), read: vec![] }; + let result = tail.ranges(&[[0, 8], [0, 8]], &mut budget); + assert!(matches!(result, Err(Error::LimitExceeded(_))), "{result:?}"); + assert_eq!(budget.left(), 12); + } + + #[test] + fn duplicate_copies_can_use_exactly_the_remaining_budget() { + let limits = crate::Limits { max_total: 16, ..Default::default() }; + let mut budget = Budget::new(&limits); + let tail = Tail::Stream { dec: Box::new(GzDecoder::new(&b"not a gzip stream"[..])), read: b"12345678".to_vec() }; + let result = tail.ranges(&[[0, 8], [0, 8]], &mut budget).unwrap(); + assert_eq!(result, [Some(b"12345678".to_vec()), Some(b"12345678".to_vec())]); + assert_eq!(budget.left(), 0); + } + + #[test] + fn rendition_copy_size_overflow_is_rejected_before_reading() { + let limits = crate::Limits { max_total: u64::MAX, ..Default::default() }; + let mut budget = Budget::new(&limits); + let tail = Tail::Stream { dec: Box::new(GzDecoder::new(&b"not a gzip stream"[..])), read: vec![] }; + assert!(matches!(tail.ranges(&[[0, u64::MAX], [0, 1]], &mut budget), Err(Error::LimitExceeded(_)))); + assert_eq!(budget.left(), u64::MAX); + } + #[test] fn an_image_of_the_wrong_shape_is_skipped_without_reading_it_whole() { assert!(Lenient::::deserialize(unread_object(1000)).unwrap().0.is_none()); From c813b55bf7f4e712ab1750028c26d84d80e553bf Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 7 Nov 2026 06:29:48 -0800 Subject: [PATCH 2/2] craft-doc: say what the total budget counts The max_total and Budget docs now say the budget counts bytes consumed from gzip, and that unused lookahead from the initial gzip read (chunks of up to 64 KiB) is not counted. --- crates/doc/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index ed5daa36..52571d7e 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -134,9 +134,10 @@ pub struct Limits { pub max_json: u64, /// Most bytes of one rendition or one ZIP entry. Default 256 MiB. pub max_entry: u64, - /// Most bytes one call decompresses from gzip, reads out of ZIP entries and returns as + /// Most bytes one call consumes from gzip, reads out of ZIP entries and returns as /// renditions, together; a rendition decompressed from gzip is counted once, and a second - /// rendition of the same bytes as another copy. Default 1 GiB. + /// rendition of the same bytes as another copy. Unused lookahead from initial gzip reads + /// (chunks of up to 64 KiB) is not counted. Default 1 GiB. pub max_total: u64, /// Deepest nesting of JSON arrays and objects, at most [`MAX_DEPTH`] (a larger value counts /// as [`MAX_DEPTH`]). Default [`MAX_DEPTH`]. [`renditions`] and [`outline`] also refuse a @@ -313,7 +314,7 @@ fn malformed(m: impl std::fmt::Display) -> Error { Error::Malformed(m.to_string()) } -/// The bytes one call may still decompress, read out of ZIP entries or return as renditions. +/// The bytes one call may still consume from gzip or ZIP entries, or return as renditions. pub(crate) struct Budget<'a> { pub(crate) limits: &'a Limits, left: u64,