From d60b6bf8836b43c82d17cd9718bad73a466ec972 Mon Sep 17 00:00:00 2001 From: Don Omondi <282319807+mockydevs@users.noreply.github.com> Date: Thu, 29 Oct 2026 04:44:48 +0300 Subject: [PATCH] fix(doc): count the values buffered before an object's type (#3587) NodeKind's reader buffers the fields that come before `type` as serde_json::Values. VectorCraft writes `type` first, but a crafted file can put a long array there: a group whose children held 50M numbers before its type (a 96 KB gzip) peaked at 1.6 GB, and the cost grows with the array up to MAX_JSON, before any object in it is counted. The buffer is read through a counting visitor: each JSON value counts against MAX_BUFFERED_VALUES (32M, 4M on wasm32) as it is read, and values replayed from the buffer aren't counted twice. load_file reads within it; the same file is now refused at a 1.1 GB peak. --- crates/doc/src/node.rs | 2 +- crates/doc/src/node/kind.rs | 148 ++++++++++++++++++++++++++++++++++- crates/doc/src/node/tests.rs | 18 +++++ crates/format/src/lib.rs | 50 ++++++++++-- 4 files changed, 207 insertions(+), 11 deletions(-) diff --git a/crates/doc/src/node.rs b/crates/doc/src/node.rs index b12c71cb6..3dd07351e 100644 --- a/crates/doc/src/node.rs +++ b/crates/doc/src/node.rs @@ -28,7 +28,7 @@ pub use attributes::{ForeignAttribute, ImageMap, MAX_SVG_SCRIPTS, ObjectAttribut pub use clip::{ClipPaint, Uniter}; pub use extras::{Extras, ExtrasMut, NodeExtras}; pub use ids::{LAYER_COLORS, LayerColor, NodeId}; -pub use kind::{ImageObject, NodeKind}; +pub use kind::{ImageObject, MAX_BUFFERED_VALUES, NodeKind, with_buffered_value_budget}; pub use live_shape::{LiveShape, Scaling}; pub use opacity::{Knockout, OpacityMask, whiten}; use wire::NodeWire; diff --git a/crates/doc/src/node/kind.rs b/crates/doc/src/node/kind.rs index 0994cf390..22470cbf3 100644 --- a/crates/doc/src/node/kind.rs +++ b/crates/doc/src/node/kind.rs @@ -129,6 +129,8 @@ pub enum NodeKind { /// anywhere among its fields. Files have it first (it is written first): then the kind is read /// straight from the file. Otherwise (hand-written files, values whose keys are sorted) the /// fields before `type` are buffered, and only those. +pub use kind_de::{MAX_BUFFERED_VALUES, with_buffered_value_budget}; + mod kind_de { use std::fmt; use std::sync::Arc; @@ -237,12 +239,150 @@ mod kind_de { let tag: String = map.next_value()?; return KindDef::deserialize(Tagged { tag, map: Buffered { before: before.into_iter(), value: None, map } }); } - before.push((key, map.next_value::()?)); + before.push((key, map.next_value::()?.0)); } Err(de::Error::missing_field("type")) } } + /// Most JSON values (each number, string, array, object…) the fields read before an object's + /// `type` may hold together while [`with_buffered_value_budget`] runs: 32M (about 1 GB as + /// `serde_json::Value`s, some 32 bytes each), 4M in the web build. VectorCraft writes `type` + /// first, so its files buffer nothing; a reordered file buffers what comes before it, and + /// a crafted one could fill memory with values two bytes each in the file before any object in + /// them is counted (#3587). + pub const MAX_BUFFERED_VALUES: usize = if cfg!(target_arch = "wasm32") { 4 << 20 } else { 32 << 20 }; + + std::thread_local! { + /// Values the fields buffered on this thread may still hold, and the budget they come from. + static VALUES_LEFT: std::cell::Cell> = const { std::cell::Cell::new(None) }; + /// Buffered fields being read back: what they hold was counted when it was buffered. + static REPLAYING: std::cell::Cell = const { std::cell::Cell::new(0) }; + } + + /// Run `f` (which reads objects with serde) letting the fields it buffers before an object's + /// `type` hold at most `max` values together; buffering more fails. + pub fn with_buffered_value_budget(max: usize, f: impl FnOnce() -> T) -> T { + struct Restore(Option<(usize, usize)>); + impl Drop for Restore { + fn drop(&mut self) { + VALUES_LEFT.set(self.0); + } + } + let _restore = Restore(VALUES_LEFT.replace(Some((max, max)))); + f() + } + + /// While buffered fields are read back ([`REPLAYING`]). + struct Replaying; + + impl Replaying { + fn enter() -> Self { + REPLAYING.set(REPLAYING.get().saturating_add(1)); + Replaying + } + } + + impl Drop for Replaying { + fn drop(&mut self) { + REPLAYING.set(REPLAYING.get().saturating_sub(1)); + } + } + + /// Count one buffered value against [`with_buffered_value_budget`]. + fn count() -> Result<(), E> { + if REPLAYING.get() > 0 { + return Ok(()); + } + if let Some((left, max)) = VALUES_LEFT.get() { + let left = left.checked_sub(1).ok_or_else(|| E::custom(format!("an object has more than {max} values before its type")))?; + VALUES_LEFT.set(Some((left, max))); + } + Ok(()) + } + + /// A field read before `type`, as a `serde_json::Value` counted value by value as it is read, + /// so the budget stops a large one before it is all in memory. + struct Counted(serde_json::Value); + + impl<'de> Deserialize<'de> for Counted { + fn deserialize>(d: D) -> Result { + d.deserialize_any(CountedVisitor) + } + } + + struct CountedVisitor; + + impl<'de> Visitor<'de> for CountedVisitor { + type Value = Counted; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("a JSON value") + } + + fn visit_bool(self, v: bool) -> Result { + count()?; + Ok(Counted(serde_json::Value::Bool(v))) + } + + fn visit_i64(self, v: i64) -> Result { + count()?; + Ok(Counted(v.into())) + } + + fn visit_u64(self, v: u64) -> Result { + count()?; + Ok(Counted(v.into())) + } + + fn visit_f64(self, v: f64) -> Result { + count()?; + Ok(Counted(serde_json::Number::from_f64(v).map_or(serde_json::Value::Null, serde_json::Value::Number))) + } + + fn visit_str(self, v: &str) -> Result { + count()?; + Ok(Counted(serde_json::Value::String(v.to_owned()))) + } + + fn visit_string(self, v: String) -> Result { + count()?; + Ok(Counted(serde_json::Value::String(v))) + } + + fn visit_none(self) -> Result { + self.visit_unit() + } + + fn visit_unit(self) -> Result { + count()?; + Ok(Counted(serde_json::Value::Null)) + } + + fn visit_some>(self, d: D) -> Result { + Counted::deserialize(d) + } + + fn visit_seq>(self, mut seq: S) -> Result { + count()?; + let mut out = Vec::new(); + while let Some(Counted(v)) = seq.next_element()? { + out.push(v); + } + Ok(Counted(serde_json::Value::Array(out))) + } + + fn visit_map>(self, mut map: M) -> Result { + count()?; + let mut out = serde_json::Map::new(); + while let Some(key) = map.next_key::()? { + let Counted(v) = map.next_value()?; + out.insert(key, v); + } + Ok(Counted(serde_json::Value::Object(out))) + } + } + /// The fields read before `type` (buffered), then the rest of the object. struct Buffered { before: std::vec::IntoIter<(String, serde_json::Value)>, @@ -265,7 +405,11 @@ mod kind_de { fn next_value_seed>(&mut self, seed: V) -> Result { match self.value.take() { - Some(value) => seed.deserialize(value).map_err(de::Error::custom), + Some(value) => { + // Counted when it was read: the objects it holds buffer it again as they read it. + let _replaying = Replaying::enter(); + seed.deserialize(value).map_err(de::Error::custom) + } None => self.map.next_value_seed(seed), } } diff --git a/crates/doc/src/node/tests.rs b/crates/doc/src/node/tests.rs index dbc95771f..0565c9e07 100644 --- a/crates/doc/src/node/tests.rs +++ b/crates/doc/src/node/tests.rs @@ -449,3 +449,21 @@ fn objects_are_read_within_the_node_budget() { assert!(with_node_budget(1, read).is_err()); assert!(with_node_budget(3, read).is_ok()); } + +/// #3587: fields read before an object's `type` are buffered value by value within +/// [`with_buffered_value_budget`], each counted once: a group read back from the buffer buffers +/// its own fields again without counting them twice. VectorCraft writes `type` first, which +/// buffers nothing. +#[test] +fn fields_before_the_type_are_buffered_within_the_value_budget() { + let reordered = r#"{"id":1,"kind":{"children":[{"id":2,"kind":{"children":[],"type":"group"}}],"type":"group"}}"#; + let read = |json: &str| serde_json::from_str::(json); + // The children's array, the inner object, its id, its kind, the kind's empty children and + // its type: six values. + assert!(with_buffered_value_budget(6, || read(reordered)).is_ok()); + let why = with_buffered_value_budget(5, || read(reordered)).unwrap_err().to_string(); + assert!(why.contains("more than 5 values before its type"), "{why}"); + let written = serde_json::to_string(&Node::group(NodeId(1), vec![Arc::new(Node::group(NodeId(2), vec![]))])).unwrap(); + assert!(with_buffered_value_budget(0, || read(&written)).is_ok(), "a saved object buffers nothing: {written}"); + assert!(read(reordered).is_ok(), "no budget outside a read"); +} diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 3081bd99f..0e65fcb66 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -552,16 +552,22 @@ pub fn load_info(bytes: &[u8]) -> Result<(Document, FileInfo), FormatError> { /// [`load`], with everything else the file carries. pub fn load_file(bytes: &[u8]) -> Result { - load_file_within(bytes, vectorcraft_doc::node::MAX_READ_NODES) + load_file_within(bytes, vectorcraft_doc::node::MAX_READ_NODES, vectorcraft_doc::node::MAX_BUFFERED_VALUES) } -/// [`load_file`] reading at most `max_nodes` objects, so a test can exercise the object budget -/// without a document of [`vectorcraft_doc::node::MAX_READ_NODES`] objects. -fn load_file_within(bytes: &[u8], max_nodes: usize) -> Result { +/// [`load_file`] reading at most `max_nodes` objects and buffering at most `max_values` values +/// before objects' types, so a test can exercise the budgets without documents that large +/// ([`vectorcraft_doc::node::MAX_READ_NODES`], [`vectorcraft_doc::node::MAX_BUFFERED_VALUES`]). +fn load_file_within(bytes: &[u8], max_nodes: usize, max_values: usize) -> Result { let file = unpack::Unpacked::new(bytes)?; // The JSON reader stops at 128 levels (a document's objects use three each), the paths read - // at `MAX_READ_ANCHORS` anchors together, and the document at `MAX_READ_NODES` objects. - let read = || vectorcraft_doc::node::with_node_budget(max_nodes, || serde_json::from_slice::(file.json())); + // at `MAX_READ_ANCHORS` anchors together, the document at `MAX_READ_NODES` objects, and the + // fields read before an object's type at `MAX_BUFFERED_VALUES` values. + let read = || { + vectorcraft_doc::node::with_node_budget(max_nodes, || { + vectorcraft_doc::node::with_buffered_value_budget(max_values, || serde_json::from_slice::(file.json())) + }) + }; let f: File = vectorcraft_doc::geom::path::with_anchor_budget(vectorcraft_doc::geom::path::MAX_READ_ANCHORS, read).map_err(|e| { let why = if e.to_string().contains("recursion limit") { format!("its objects are nested too deep ({e})") } else { e.to_string() }; FormatError::NotVectorcraft(why) @@ -797,12 +803,40 @@ mod tests { let n = d.node_count(); let bytes = save_with(&d, &SaveOptions { compress: true, ..SaveOptions::default() }).unwrap(); assert!(is_compressed(&bytes)); - assert_eq!(load_file_within(&bytes, n).unwrap().doc.node_count(), n, "a document of exactly the budget opens"); - let Err(FormatError::NotVectorcraft(why)) = load_file_within(&bytes, n - 1) else { panic!("one object past the budget must be refused") }; + assert_eq!( + load_file_within(&bytes, n, vectorcraft_doc::node::MAX_BUFFERED_VALUES).unwrap().doc.node_count(), + n, + "a document of exactly the budget opens" + ); + let Err(FormatError::NotVectorcraft(why)) = load_file_within(&bytes, n - 1, vectorcraft_doc::node::MAX_BUFFERED_VALUES) else { + panic!("one object past the budget must be refused") + }; assert!(why.contains(&format!("more than {} objects", n - 1)), "{why}"); assert_eq!(load(&bytes).unwrap().node_count(), n, "the real budget opens ordinary documents"); } + /// #3587: a reordered file (its objects' fields before their `type`, as a JSON tool that + /// sorts keys writes them) still opens; one whose fields before a `type` hold more values than + /// the budget, such as a long array of numbers, is refused as they are read. + #[test] + fn values_buffered_before_an_objects_type_are_capped() { + let mut d = Document::new(100.0, 100.0); + let l = d.add_layer(None); + let id = d.alloc_id(); + d.insert(Some(l), 0, Node::path(id, shapes::ellipse(Rect::new(0.0, 0.0, 10.0, 10.0)), Appearance::default_art())).unwrap(); + // Through `serde_json::Value`, whose objects sort their keys: `children` before `type`. + let sorted = serde_json::to_vec(&serde_json::from_slice::(&save(&d, false)).unwrap()).unwrap(); + assert_eq!(load(&sorted).unwrap().node_count(), d.node_count()); + let mut v: serde_json::Value = serde_json::from_slice(&save(&d, false)).unwrap(); + let zeros = vec![0; 1000]; + v["document"]["layers"][0]["kind"]["children"] = serde_json::json!([{"id": 90, "kind": {"children": zeros, "type": "group"}}]); + let bomb = serde_json::to_vec(&v).unwrap(); + let Err(FormatError::NotVectorcraft(why)) = load_file_within(&bomb, vectorcraft_doc::node::MAX_READ_NODES, 100) else { + panic!("past the budget must be refused") + }; + assert!(why.contains("more than 100 values before its type"), "{why}"); + } + #[test] fn rejects_foreign_and_future() { assert!(load(b"{}").is_err());