From cc03fade6e8d18c10846b9c3a602a6fdef703b0c Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 9 Nov 2026 22:00:04 -0800 Subject: [PATCH] Scripts: the growth budget counts every art store, with CLI and source fuzz tests A document a script opens now counts the art in its graph designs and in the original of the pattern being edited toward the growth allowance. `Document::art_roots` and `art_roots_mut` list every tree of a document's art; `any_node`, `edit_nodes_where` and the script's object count use them. `script.run` checks the depth of `args` without recursion before it copies or writes them out, so `args` nested 10,000 levels deep is refused with the place it went past 64 levels instead of overflowing the stack. New tests: graph-design art and the growth allowance, `args` nested 10,000 levels deep from a script (a RangeError) and from a caller, `run --cmd script.run` on the command line with a report and with a thrown error, and a property test that sends arbitrary source through `script.run`. --- apps/vectorcraft-cli/tests/it/main.rs | 1 + apps/vectorcraft-cli/tests/it/script.rs | 39 +++++++ crates/doc/src/document_ext.rs | 38 ++++--- crates/engine/src/cmd/scripts.rs | 4 + crates/engine/src/script/mod.rs | 47 +++++++- crates/engine/src/tests/script_run.rs | 139 ++++++++++++++++++++++++ crates/js/src/host_tests.rs | 11 ++ 7 files changed, 259 insertions(+), 20 deletions(-) create mode 100644 apps/vectorcraft-cli/tests/it/script.rs diff --git a/apps/vectorcraft-cli/tests/it/main.rs b/apps/vectorcraft-cli/tests/it/main.rs index 6918ea19b..07e5a3a70 100644 --- a/apps/vectorcraft-cli/tests/it/main.rs +++ b/apps/vectorcraft-cli/tests/it/main.rs @@ -5,3 +5,4 @@ mod cli; mod fileio; mod mcp_e2e; +mod script; diff --git a/apps/vectorcraft-cli/tests/it/script.rs b/apps/vectorcraft-cli/tests/it/script.rs new file mode 100644 index 000000000..d68716f7b --- /dev/null +++ b/apps/vectorcraft-cli/tests/it/script.rs @@ -0,0 +1,39 @@ +use std::process::Command; + +use serde_json::{Value, json}; + +const BIN: &str = env!("CARGO_BIN_EXE_vectorcraft-cli"); + +/// `run` with `script.run` after `file.new`, with `params`. +fn run_script(params: &Value) -> std::process::Output { + Command::new(BIN).args(["run", "--cmd", "file.new", "--cmd", "script.run", "--params", ¶ms.to_string()]).output().unwrap() +} + +/// `run` runs a script headless: the reply holds the report, with what the script logged and the +/// documents it changed. +#[test] +fn run_gives_the_report_of_a_script() { + let source = "vc.run('shape.rectangle', {x: 0, y: 0, width: 10, height: 10}); vc.log(vc.args.n * 2);"; + let out = run_script(&json!({"source": source, "name": "double.js", "args": {"n": 21}})); + assert!(out.status.success(), "{}", String::from_utf8_lossy(&out.stderr)); + let lines: Vec = String::from_utf8_lossy(&out.stdout).lines().map(|l| serde_json::from_str(l).unwrap()).collect(); + assert_eq!(lines.len(), 2, "{lines:?}"); + assert_eq!(lines[1]["command"], "script.run"); + let report = &lines[1]["result"]; + assert_eq!(report["ok"], true, "{report}"); + assert_eq!(report["name"], "double.js"); + assert_eq!(report["log"], json!([42]), "{report}"); + assert_eq!(report["changed"].as_array().map(Vec::len), Some(1), "{report}"); +} + +/// A script that throws fails its step with the script's message, and the steps before it ran. +#[test] +fn run_fails_the_step_of_a_script_that_throws() { + let out = run_script(&json!({"source": "vc.run('shape.rectangle', {x: 0, y: 0, width: 10, height: 10}); throw new Error('no margin given');"})); + assert!(!out.status.success()); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!(stderr.contains("step 1 (script.run)") && stderr.contains("no margin given"), "{stderr}"); + let printed: Vec = String::from_utf8_lossy(&out.stdout).lines().map(|l| serde_json::from_str(l).unwrap()).collect(); + assert_eq!(printed.len(), 1, "only file.new printed: {printed:?}"); + assert_eq!(printed[0]["command"], "file.new"); +} diff --git a/crates/doc/src/document_ext.rs b/crates/doc/src/document_ext.rs index c40dcf9f2..ebb2fc44e 100644 --- a/crates/doc/src/document_ext.rs +++ b/crates/doc/src/document_ext.rs @@ -123,18 +123,33 @@ impl Document { std::borrow::Cow::Owned(d) } - /// Does `wants` hold for a node anywhere in the document's art: the layers, symbol definitions, - /// graph designs, pattern swatches (the one being edited included) and their opacity masks? - pub fn any_node(&self, wants: impl Fn(&Node) -> bool) -> bool { - let roots = self - .layers + /// The roots of every tree of the document's art: the layers, symbol definitions, graph designs, + /// pattern swatches and the original of the pattern being edited. Opacity masks hang off nodes + /// in these trees. + pub fn art_roots(&self) -> impl Iterator> { + self.layers .iter() .chain(self.symbols.iter().map(|s| &s.art)) .chain(self.graph_designs.iter().map(|g| &g.art)) .chain(self.patterns.iter().flat_map(|p| p.art.iter())) - .chain(self.pattern_edit.iter().filter_map(|pe| pe.original.as_ref()).flat_map(|p| p.art.iter())); + .chain(self.pattern_edit.iter().filter_map(|pe| pe.original.as_ref()).flat_map(|p| p.art.iter())) + } + + /// [`Self::art_roots`], to change. + pub fn art_roots_mut(&mut self) -> impl Iterator> { + self.layers + .iter_mut() + .chain(self.symbols.iter_mut().map(|s| &mut s.art)) + .chain(self.graph_designs.iter_mut().map(|g| &mut g.art)) + .chain(self.patterns.iter_mut().flat_map(|p| p.art.iter_mut())) + .chain(self.pattern_edit.iter_mut().filter_map(|pe| pe.original.as_mut()).flat_map(|p| p.art.iter_mut())) + } + + /// Does `wants` hold for a node anywhere in the document's art ([`Self::art_roots`]) or its + /// opacity masks? + pub fn any_node(&self, wants: impl Fn(&Node) -> bool) -> bool { let mut any = false; - for root in roots { + for root in self.art_roots() { crate::swatches::walk_with_masks(root, &mut |n| any |= wants(n)); } any @@ -144,14 +159,7 @@ impl Document { /// [`Self::any_node`] looks). Only the trees that hold such a node are copied out of the art /// they share. pub fn edit_nodes_where(&mut self, wants: impl Fn(&Node) -> bool, mut f: impl FnMut(&mut Node)) { - let roots = self - .layers - .iter_mut() - .chain(self.symbols.iter_mut().map(|s| &mut s.art)) - .chain(self.graph_designs.iter_mut().map(|g| &mut g.art)) - .chain(self.patterns.iter_mut().flat_map(|p| p.art.iter_mut())) - .chain(self.pattern_edit.iter_mut().filter_map(|pe| pe.original.as_mut()).flat_map(|p| p.art.iter_mut())); - for root in roots { + for root in self.art_roots_mut() { edit_branches_where(root, &wants, &mut f); } } diff --git a/crates/engine/src/cmd/scripts.rs b/crates/engine/src/cmd/scripts.rs index 692de92f3..b8b544881 100644 --- a/crates/engine/src/cmd/scripts.rs +++ b/crates/engine/src/cmd/scripts.rs @@ -92,6 +92,10 @@ fn run(s: &mut Session, p: &Value) -> Result { Some(_) => Err(param(format!("`{key}` must be a string"))), }; let (path, source, given_name) = (optional_str("path")?, optional_str("source")?, optional_str("name")?); + // Checked before anything copies or writes out `args`, which would go one call deeper per level. + if let Some(place) = p.get("args").and_then(script::too_deep) { + return Err(param(format!("`args{place}` is nested more than 64 levels deep"))); + } let args = match p.get("args") { None => serde_json::Map::new(), Some(Value::Object(o)) => o.clone(), diff --git a/crates/engine/src/script/mod.rs b/crates/engine/src/script/mod.rs index 643f7ea31..632d45de0 100644 --- a/crates/engine/src/script/mod.rs +++ b/crates/engine/src/script/mod.rs @@ -503,6 +503,46 @@ fn to_json(d: &Data) -> Value { } } +/// A step into a JSON value: a key or an index. +enum Step<'a> { + Name(&'a str), + Index(usize), +} + +/// The place in `v` that is more than [`DataLimits`]' depth levels deep, such as `.layers[2].name`, +/// or `None`. It walks `v` with a list of its own, so any depth fits on the stack. +pub(crate) fn too_deep(v: &Value) -> Option { + let limit = DataLimits::default().depth; + // `steps[i - 1]` is the entry that holds entry `i` and the step into it; entry 0 is `v`. + let mut steps: Vec<(usize, Step<'_>)> = Vec::new(); + let mut todo = vec![(v, 0usize, 0usize)]; + while let Some((v, depth, entry)) = todo.pop() { + if depth > limit { + let mut place = Vec::new(); + let mut at = entry; + while let Some((parent, step)) = at.checked_sub(1).and_then(|i| steps.get(i)) { + place.push(match step { + Step::Name(k) => format!(".{k}"), + Step::Index(i) => format!("[{i}]"), + }); + at = *parent; + } + place.reverse(); + return Some(place.concat()); + } + let children: Box, &Value)>> = match v { + Value::Array(a) => Box::new(a.iter().enumerate().map(|(i, x)| (Step::Index(i), x))), + Value::Object(o) => Box::new(o.iter().map(|(k, x)| (Step::Name(k.as_str()), x))), + _ => continue, + }; + for (step, child) in children { + steps.push((entry, step)); + todo.push((child, depth + 1, steps.len())); + } + } + None +} + /// Estimated bytes of `d`. fn data_size(d: &Data) -> u64 { match d { @@ -527,12 +567,9 @@ fn raise_ids(s: &mut Session) { s.script_high_water = marks; } -/// Every object in `d`: the art of its layers, its symbols and its patterns, opacity masks included. +/// Every object in `d`: all its art ([`crate::Document::art_roots`]), opacity masks included. fn all_objects(d: &crate::Document) -> u64 { - let layers = d.layers.iter().map(|n| n.count_with_masks()); - let symbols = d.symbols.iter().map(|s| s.art.count_with_masks()); - let patterns = d.patterns.iter().flat_map(|p| &p.art).map(|n| n.count_with_masks()); - layers.chain(symbols).chain(patterns).map(|n| n as u64).sum() + d.art_roots().map(|n| n.count_with_masks() as u64).sum() } fn note_ids(s: &mut Session) { diff --git a/crates/engine/src/tests/script_run.rs b/crates/engine/src/tests/script_run.rs index bc3cc67a9..81b224628 100644 --- a/crates/engine/src/tests/script_run.rs +++ b/crates/engine/src/tests/script_run.rs @@ -640,6 +640,81 @@ fn a_document_a_run_opens_counts_toward_its_growth_allowance() { assert_eq!(state(&s), before); } +#[test] +fn graph_design_art_counts_toward_the_growth_allowance() { + TEST_GROWTH.with(|g| g.set(Some(20))); + let mut s = two_docs(); + for x in 0..30 { + s.execute("shape.rectangle", &rect(x)).unwrap(); + } + let before = state(&s); + let designs = |s: &Session| s.documents().iter().map(|d| d.doc.graph_designs.len()).collect::>(); + let designs_before = designs(&s); + // A design saved from the selection in an open document. + let e = run(&mut s, "vc.run('select.all'); vc.run('graph.design', {new: 'Many'});").unwrap_err().to_string(); + assert!(e.contains("objects and data"), "{e}"); + assert_eq!(state(&s), before); + assert_eq!(designs(&s), designs_before); + + // A document a run opens whose art is all in a graph design. + let mut maker = Session::new(); + maker.execute("file.new", &json!({})).unwrap(); + let ids: Vec = (0..30).map(|x| maker.execute("shape.rectangle", &rect(x)).unwrap()["id"].as_u64().unwrap()).collect(); + maker.execute("select.all", &json!({})).unwrap(); + maker.execute("graph.design", &json!({"new": "Many"})).unwrap(); + maker.execute("edit.clear", &json!({"ids": ids})).unwrap(); + assert_eq!(objects(&maker, 0), 0); + assert_eq!(maker.documents()[0].doc.graph_designs[0].art.count_with_masks(), 31); + let data = maker.execute("document.serialize", &json!({})).unwrap()["dataBase64"].as_str().unwrap().to_string(); + let source = format!("vc.run('document.open', {{name: 'g.vectorcraft', dataBase64: '{data}'}});"); + let e = run(&mut s, &source).unwrap_err().to_string(); + TEST_GROWTH.with(|g| g.set(None)); + assert!(e.contains("objects and data"), "{e}"); + assert_eq!(state(&s), before); +} + +/// Drops `v` one level at a time. +fn drop_flat(v: Value) { + let mut stack = vec![v]; + while let Some(mut v) = stack.pop() { + match &mut v { + Value::Object(o) => stack.extend(std::mem::take(o).into_iter().map(|(_, x)| x)), + Value::Array(a) => stack.append(a), + _ => {} + } + } +} + +#[test] +fn args_nested_ten_thousand_deep_in_a_script_throw_a_range_error_with_their_place() { + let mut s = two_docs(); + let before = state(&s); + let source = "var v = 1; for (var i = 0; i < 10000; i++) v = {a: v}; var m = 'none'; \ + try { vc.run('script.run', {source: '', args: v}); } catch (e) { m = e.name + '|' + e.message; } vc.log(m);"; + let r = run(&mut s, source).unwrap(); + let m = r["log"][0].as_str().unwrap_or_default(); + assert!(m.starts_with("RangeError|") && m.contains(".args.a.a.a") && m.contains("nested too deep"), "{r}"); + assert_eq!(state(&s), before); +} + +#[test] +fn args_nested_ten_thousand_deep_from_a_caller_are_refused_with_their_place() { + let mut s = two_docs(); + let before = state(&s); + // Built by moving each level in: `json!` would copy the value through `to_value`, one call per level. + let wrap = |key: &str, v: Value| Value::Object(std::iter::once((key.to_string(), v)).collect()); + let mut deep = json!(1); + for _ in 0..10_000 { + deep = wrap("a", deep); + } + let mut params = wrap("args", wrap("b", deep)); + params["source"] = json!(""); + let e = s.execute("script.run", ¶ms).unwrap_err().to_string(); + drop_flat(params); + assert!(e.contains("`args.b.a.a.a") && e.contains("nested more than 64 levels deep"), "{e}"); + assert_eq!(state(&s), before); +} + #[test] fn a_person_can_let_a_growing_run_go_on_or_stop_it() { TEST_GROWTH.with(|g| g.set(Some(20))); @@ -765,3 +840,67 @@ fn a_cancel_from_the_caller_stops_a_headless_run() { assert!(!stop.is_set()); assert_eq!(state(&s), before); } + +/// Script text: pieces that change documents, throw or loop, mixed with arbitrary text. +fn source_text() -> impl proptest::strategy::Strategy { + use proptest::prelude::*; + let piece = prop_oneof![ + Just("vc.run('shape.rectangle', {x:1,y:1,width:5,height:5});".to_string()), + Just("vc.run('select.all'); vc.run('edit.clear');".to_string()), + Just("vc.run('file.new', {});".to_string()), + Just("vc.run('document.activate', {index:0});".to_string()), + Just("vc.run('no.such');".to_string()), + Just("vc.log(vc.document);".to_string()), + Just("throw new Error('x');".to_string()), + Just("vc.run('shape.rectangle', {x:2,y:2,width:5,height:5}); throw new Error('y');".to_string()), + Just("while (true) {}".to_string()), + Just("var o = {}; o.o = o; vc.run('shape.rectangle', o);".to_string()), + Just("{".to_string()), + Just("try {".to_string()), + Just("} catch (e) {}".to_string()), + "[ -~]{0,30}", + "\\PC{0,10}", + ]; + proptest::collection::vec(piece, 0..12).prop_map(|p| p.concat()) +} + +/// Every field of each open document but its next id, and the rest of what a rollback puts back. +/// A failed run raises the next id past the ids it gave out, which [`next_ids`] reads. +fn saved(s: &Session) -> (Value, Vec) { + let docs = s + .documents() + .iter() + .map(|d| { + let mut doc = (*d.doc).clone(); + doc.raise_next_id(1 << 50); + format!("{doc:?}") + }) + .collect(); + (state(s), docs) +} + +fn next_ids(s: &Session) -> Vec { + s.documents().iter().map(|d| d.doc.peek_next_id()).collect() +} + +proptest::proptest! { + #![proptest_config(proptest::prelude::ProptestConfig::with_cases(48))] + + /// Any script text ends with a report or a script error, and a run that fails leaves every + /// document as it was. + #[test] + fn any_source_ends_with_a_report_or_a_script_error(source in source_text()) { + let mut s = two_docs(); + let (before, ids) = (saved(&s), next_ids(&s)); + match s.execute("script.run", &json!({"source": source, "maxFuel": 200_000})) { + Ok(r) => proptest::prop_assert_eq!(&r["ok"], &json!(true), "{}", r), + Err(e) => { + proptest::prop_assert!(matches!(e, EngineError::Other(_)), "{:?}", e); + let kind = s.script_report.as_ref().map(|r| r["kind"].clone()); + proptest::prop_assert!(kind.as_ref().is_some_and(|k| k != "internal"), "{:?}: {}", kind, e); + proptest::prop_assert_eq!(saved(&s), before); + proptest::prop_assert!(next_ids(&s).iter().zip(&ids).all(|(after, was)| after >= was), "{:?} after {:?}", next_ids(&s), ids); + } + } + } +} diff --git a/crates/js/src/host_tests.rs b/crates/js/src/host_tests.rs index d43e26f5b..85e178d87 100644 --- a/crates/js/src/host_tests.rs +++ b/crates/js/src/host_tests.rs @@ -171,6 +171,17 @@ fn values_that_are_not_plain_data_are_refused_with_their_place() { refused("var value = 'x'; for (var i = 0; i < 12; i++) value += value;", "RangeError", "too large"); } +#[test] +fn a_value_nested_ten_thousand_deep_is_refused_with_its_place() { + let mut r = realm(100_000_000); + let seen = with_take(&mut r); + let probe = "var v = 1; for (var i = 0; i < 10000; i++) v = {a: v}; var m = 'none'; \ + try { host.take({args: v}); } catch (e) { m = e.name + '|' + e.message; } m"; + let JsValue::String(out) = r.run(probe).unwrap() else { panic!("not a string") }; + assert!(out.starts_with("RangeError|") && out.contains(".args.a.a.a") && out.contains("nested too deep"), "{out}"); + assert!(seen.borrow().is_empty(), "the function ran"); +} + #[test] fn a_shared_object_is_allowed_and_sharing_cannot_multiply_the_work() { let mut r = realm(10_000_000);