Merge pull request #1129 from vangel76/fix-psd-nested-group-depth

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 }
 }
 

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);
+}

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<Node> {
+        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<Node>, usize) {
         let mut stack: Vec<Vec<Node>> = 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<Node>) {
             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 {

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<usize>) {
+    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::<Vec<_>>());
+    // 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);
+}