From 4f9a04fcf57ef11cf044fb4d9a12284fe5d81ea0 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Mon, 12 Oct 2026 22:56:56 +0200 Subject: [PATCH] PSD import: bound group nesting at 64 so thousands of nested groups cannot overflow the stack (#1128) Psd::tree flattens groups nested deeper than MAX_GROUP_DEPTH into their depth-64 parent, keeping every layer, so the importer recursion and the tree's Drop/Clone/PartialEq are bounded. The import result reports how many groups were flattened. Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/psd_import.rs | 6 ++- crates/engine/src/tests_vector_import.rs | 27 ++++++++++ crates/psd/src/lib.rs | 37 +++++++++++-- crates/psd/src/tests.rs | 66 ++++++++++++++++++++++++ 4 files changed, 132 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/psd_import.rs b/crates/engine/src/psd_import.rs index b025a7a5..7f0fbcac 100644 --- a/crates/engine/src/psd_import.rs +++ b/crates/engine/src/psd_import.rs @@ -111,7 +111,11 @@ struct Ctx<'a> { pub fn import(proj: &mut Project, psd: &Psd, path: &str, name: &str, retain: bool, rate: FrameRate, duration: Tick) -> PsdImport { let folder = proj.add_item(&format!("{name} Layers"), Label::Yellow, None, ItemKind::Folder); let mut cx = Ctx { psd, path, retain, folder, rate, duration, items: vec![], warnings: vec![] }; - let comp = build_comp(proj, &mut cx, name, &psd.tree(), None); + let (tree, flattened) = psd.tree_report(); + if flattened > 0 { + cx.warnings.push(format!("{flattened} group(s) nested deeper than {} levels were flattened into their parent", effectcraft_psd::MAX_GROUP_DEPTH)); + } + let comp = build_comp(proj, &mut cx, name, &tree, None); PsdImport { comp, folder, items: cx.items, warnings: cx.warnings } } diff --git a/crates/engine/src/tests_vector_import.rs b/crates/engine/src/tests_vector_import.rs index e33a38b0..8656dfaa 100644 --- a/crates/engine/src/tests_vector_import.rs +++ b/crates/engine/src/tests_vector_import.rs @@ -283,3 +283,30 @@ fn paste_svg_rejects_text_that_is_not_usable_svg() { assert!(s.execute_checked("edit.pasteSvg", json!({})).is_err()); assert_eq!(s.active_comp().unwrap().layers.len(), 1, "nothing pasted"); } + +#[test] +fn psd_with_thousands_of_nested_groups_imports_on_a_small_stack() { + let mut d = WDoc::new(8, 8); + let n = 5000; + d.layers = (0..n).map(|_| WLayer::group_end()).collect(); + for i in 0..n { + d.layers.push(WLayer::solid(&format!("L{i}"), Rect::new(0, 0, 2, 2), [1.0; 4])); + d.layers.push(WLayer::group(&format!("G{i}"), true, *b"norm")); + } + let path = tmp("nested.psd"); + std::fs::write(&path, effectcraft_psd::write::write(&d)).unwrap(); + let h = std::thread::Builder::new() + .stack_size(2 << 20) + .spawn(move || { + let mut s = Session::default(); + let r = s.execute_checked("file.import", json!({"paths": [path], "importAs": "composition"})).unwrap(); + let warned = r["errors"].as_array().is_some_and(|e| e.len() == 1 && e[0].as_str().is_some_and(|m| m.contains("flattened"))); + // 64 nested group comps plus the root comp. + let comps = s.project.items.values().filter(|i| matches!(i.kind, ItemKind::Comp(_))).count(); + (warned, comps) + }) + .unwrap(); + let (warned, comps) = h.join().unwrap(); + assert_eq!(comps, 65); + assert!(warned); +} diff --git a/crates/psd/src/lib.rs b/crates/psd/src/lib.rs index 1c0fd3d4..e148c1c5 100644 --- a/crates/psd/src/lib.rs +++ b/crates/psd/src/lib.rs @@ -306,6 +306,10 @@ impl Layer { } } +/// How deep groups nest in [`Psd::tree`]; deeper ones are flattened (a hostile file can nest +/// thousands, and walking or dropping that tree would overflow the stack). +pub const MAX_GROUP_DEPTH: usize = 64; + /// The layer tree, top of the stack first. #[derive(Clone, Debug, PartialEq)] pub enum Node { @@ -742,13 +746,39 @@ impl Psd { Ok(vm) } - /// The layer tree, top of the stack first. + /// The layer tree, top of the stack first. Groups nest at most [`MAX_GROUP_DEPTH`] deep (see + /// [`Psd::tree_report`]). pub fn tree(&self) -> Vec { + self.tree_report().0 + } + + /// The layer tree plus the number of groups nested deeper than [`MAX_GROUP_DEPTH`]. Those + /// are flattened into their deepest allowed ancestor: their children stay, in order, and + /// the group record itself becomes a plain [`Node::Layer`], so no layer is lost and the + /// tree (and its `Drop`, `Clone` and `PartialEq`) cannot recurse deeper than the limit. + pub fn tree_report(&self) -> (Vec, usize) { let mut stack: Vec> = vec![vec![]]; + // Dividers seen while `MAX_GROUP_DEPTH` groups were already open. + let mut flattened_open = 0usize; + let mut flattened = 0usize; for l in &self.layers { match l.section { - Section::Divider => stack.push(vec![]), + Section::Divider => { + if stack.len() > MAX_GROUP_DEPTH { + flattened_open += 1; + } else { + stack.push(vec![]); + } + } Section::OpenFolder | Section::ClosedFolder => { + if flattened_open > 0 { + flattened_open -= 1; + flattened += 1; + if let Some(top) = stack.last_mut() { + top.push(Node::Layer(l.index)); + } + continue; + } let children = if stack.len() > 1 { stack.pop().unwrap_or_default() } else { vec![] }; if let Some(top) = stack.last_mut() { top.push(Node::Group { layer: l.index, children }); @@ -767,6 +797,7 @@ impl Psd { top.extend(c); } } + // Depth is at most `MAX_GROUP_DEPTH`, so recursion is bounded. fn rev(v: &mut Vec) { v.reverse(); for n in v { @@ -777,7 +808,7 @@ impl Psd { } let mut out = stack.pop().unwrap_or_default(); rev(&mut out); - out + (out, flattened) } fn bytes_per_sample(&self) -> usize { diff --git a/crates/psd/src/tests.rs b/crates/psd/src/tests.rs index 235163b6..1097ec58 100644 --- a/crates/psd/src/tests.rs +++ b/crates/psd/src/tests.rs @@ -252,3 +252,69 @@ fn huge_layer_bounds_error_instead_of_allocating() { let p = Psd::parse(bytes).unwrap(); assert!(matches!(p.layer_pixels(0, false), Err(Error::Unsupported(_)))); } + +/// `n` groups nested in each other, each holding one pixel layer (file order: bottom first). +fn nested_groups_doc(n: usize) -> WDoc { + let mut d = WDoc::new(4, 4); + for _ in 0..n { + d.layers.push(WLayer::group_end()); + } + for i in 0..n { + d.layers.push(WLayer::solid(&format!("L{i}"), Rect::new(0, 0, 1, 1), [1.0; 4])); + d.layers.push(WLayer::group(&format!("G{i}"), true, *b"norm")); + } + d +} + +fn depth_of(nodes: &[Node]) -> usize { + nodes.iter().map(|n| if let Node::Group { children, .. } = n { 1 + depth_of(children) } else { 0 }).max().unwrap_or(0) +} + +fn layer_indices(nodes: &[Node], out: &mut Vec) { + for n in nodes { + match n { + Node::Layer(i) => out.push(*i), + Node::Group { layer, children } => { + out.push(*layer); + layer_indices(children, out); + } + } + } +} + +#[test] +fn deeply_nested_groups_are_flattened_at_the_depth_limit() { + assert_eq!(MAX_GROUP_DEPTH, 64); + let n = 70; + let p = Psd::parse(write(&nested_groups_doc(n))).unwrap(); + let (t, flattened) = p.tree_report(); + assert_eq!(depth_of(&t), MAX_GROUP_DEPTH); + assert_eq!(flattened, 6); + // No layer or group record is lost, each appears exactly once (dividers are not nodes). + let mut seen = vec![]; + layer_indices(&t, &mut seen); + seen.sort_unstable(); + assert_eq!(seen, (n..p.layers.len()).collect::>()); + // At the limit nothing is flattened. + let (t, flattened) = Psd::parse(write(&nested_groups_doc(MAX_GROUP_DEPTH))).unwrap().tree_report(); + assert_eq!((depth_of(&t), flattened), (MAX_GROUP_DEPTH, 0)); +} + +#[test] +fn thousands_of_nested_groups_do_not_overflow_a_small_stack() { + let bytes = write(&nested_groups_doc(5000)); + let h = std::thread::Builder::new() + .stack_size(2 << 20) + .spawn(move || { + let p = Psd::parse(bytes).unwrap(); + let t = p.tree(); + let c = t.clone(); + assert_eq!(t, c); + let (_, flattened) = p.tree_report(); + drop(t); + drop(c); + flattened + }) + .unwrap(); + assert_eq!(h.join().unwrap(), 5000 - MAX_GROUP_DEPTH); +}