From 7c86e166bd007cf7d8b64ecfb3e72664e4ae7a5d Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 2 Nov 2026 21:08:18 -0800 Subject: [PATCH] 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());