Integration: the six open PSD hardening PRs merged, conflicts resolved (draft)

crates/engine/src/psd_import.rs

@@ -111,10 +111,46 @@ 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);
+    cx.warnings.extend(psd.warnings.iter().cloned());
+    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 =
+        if psd.layers.is_empty() && !psd.warnings.is_empty() { flattened_comp(proj, &mut cx, name) } else { build_comp(proj, &mut cx, name, &tree, None) };
     PsdImport { comp, folder, items: cx.items, warnings: cx.warnings }
 }
 
+/// The layers could not be read: the merged image as the composition's only layer.
+fn flattened_comp(proj: &mut Project, cx: &mut Ctx, name: &str) -> ItemId {
+    let cid = proj.add_item(name, Label::Sandstone, None, ItemKind::Comp(Arc::new(new_comp(cx))));
+    cx.items.push(cid);
+    let mut comp = new_comp(cx);
+    let file = std::path::Path::new(cx.path).file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_default();
+    let f = Footage {
+        path: cx.path.to_string(),
+        kind: FootageKind::Still,
+        width: cx.psd.width,
+        height: cx.psd.height,
+        pixel_aspect: 1.0,
+        frame_rate: cx.rate,
+        has_video: true,
+        alpha: if cx.psd.merged_alpha { AlphaMode::Straight } else { AlphaMode::Ignore },
+        loop_count: 1,
+        codec: "PSD".into(),
+        layer: None,
+        ..Default::default()
+    };
+    let id = proj.add_item(&file, Label::Lavender, Some(cx.folder), ItemKind::Footage(f));
+    cx.items.push(id);
+    let l = build::layer(proj, &comp, name, LayerSource::Footage { item: id }, (cx.psd.width, cx.psd.height), None);
+    comp.layers.push(l);
+    if let Some(item) = proj.item_mut(cid) {
+        item.kind = ItemKind::Comp(Arc::new(comp));
+    }
+    cid
+}
+
 fn new_comp(cx: &Ctx) -> Comp {
     // A PSB can be 300,000 px a side; a composition is at most 30,000 (#788).
     let size = |v: u32| effectcraft_project::limits::size(v.into());
@@ -202,7 +238,8 @@ fn smart_object_layer(proj: &mut Project, cx: &mut Ctx, comp: &Comp, index: usiz
         let bb = so.placed_bounds(cw as f64, ch as f64);
         let area = ((bb[2] - bb[0]) * (bb[3] - bb[1])).max(1.0);
         let k = ((cw as f64 * ch as f64) / area).sqrt().clamp(1.0, 4.0);
-        let (w, h) = so.placed_size(cw as f64, ch as f64, k);
+        // At most 4× the document across and 2^24 pixels, however far the file moves a corner.
+        let (k, (w, h)) = so.bake_scale(cw as f64, ch as f64, k, (cx.psd.width, cx.psd.height));
         (w, h, k, bb)
     } else {
         (cw, ch, 1.0, [0.0; 4])
@@ -333,13 +370,21 @@ fn layer_origin(pl: &effectcraft_psd::Layer, retain: bool, layer: &Layer) -> [f6
     [0.0, 0.0]
 }
 
+/// Sets a scalar property; a non-finite value (from corrupt transforms) keeps the default.
 fn set_scalar(l: &mut Layer, path: &str, v: f64) {
+    if !v.is_finite() {
+        return;
+    }
     if let Some(p) = l.props.prop_mut(path) {
         p.value = Value::Scalar(v);
     }
 }
 
+/// Sets a 2D property; a non-finite component (from corrupt transforms) keeps the default.
 fn set_vec2(l: &mut Layer, path: &str, v: [f64; 2]) {
+    if !v.iter().all(|c| c.is_finite()) {
+        return;
+    }
     if let Some(p) = l.props.prop_mut(path) {
         p.value = match p.value {
             Value::Vec3(o) => Value::Vec3([v[0], v[1], o[2]]),
@@ -361,8 +406,10 @@ fn text_doc(t: &effectcraft_psd::TextInfo) -> TextDoc {
     d.set_text(&t.text);
     let [xx, xy, yx, yy, _, _] = t.transform;
     let scale = (xx * yy - xy * yx).abs().sqrt().max(1e-6);
-    if let Some(s) = t.style.size {
-        d.size = s * scale;
+    // A transform too large to multiply (inf) falls back to 1×; products below stay finite.
+    let scale = if scale.is_finite() { scale } else { 1.0 };
+    if let Some(s) = t.style.size.map(|s| s * scale).filter(|s| s.is_finite() && *s > 0.0) {
+        d.size = s;
     }
     if let Some(f) = &t.style.font {
         let (fam, style) = font_names(f);
@@ -381,19 +428,20 @@ fn text_doc(t: &effectcraft_psd::TextInfo) -> TextDoc {
         Some(6) => Justify::JustifyAll,
         _ => Justify::Left,
     };
-    if let Some(tr) = t.style.tracking {
+    if let Some(tr) = t.style.tracking.filter(|v| v.is_finite()) {
         d.tracking = tr;
     }
-    if let Some(l) = t.style.leading {
-        d.leading = Some(l * scale);
-    }
+    d.leading = t.style.leading.map(|l| l * scale).filter(|l| l.is_finite());
     d.faux_bold = t.style.faux_bold;
     d.faux_italic = t.style.faux_italic;
     if t.style.box_text
         && let Some([l, top, r, b]) = t.bounds
     {
-        d.box_size = Some([(r - l) * scale, (b - top) * scale]);
-        d.box_pos = [l * scale, top * scale];
+        let (size, pos) = ([(r - l) * scale, (b - top) * scale], [l * scale, top * scale]);
+        if size.iter().chain(&pos).all(|v| v.is_finite()) {
+            d.box_size = Some(size);
+            d.box_pos = pos;
+        }
     }
     d
 }
@@ -716,4 +764,32 @@ mod tests {
         assert_eq!(desc_blend(Some("linearDodge")), Some(BlendMode::LinearDodge));
         assert_eq!(font_names("Inter-Bold"), ("Inter".into(), "Bold".into()));
     }
+
+    #[test]
+    fn non_finite_text_values_never_reach_the_layer() {
+        let style = effectcraft_psd::engine_data::TextStyle {
+            size: Some(f64::NAN),
+            tracking: Some(f64::INFINITY),
+            leading: Some(f64::NEG_INFINITY),
+            ..Default::default()
+        };
+        let t = effectcraft_psd::TextInfo {
+            text: "x".into(),
+            transform: [f64::INFINITY, 0.0, 0.0, 1.0, f64::NAN, 1.0],
+            style,
+            bounds: Some([0.0, 0.0, f64::INFINITY, 1.0]),
+        };
+        let d = text_doc(&t);
+        assert!(d.size.is_finite() && d.tracking.is_finite());
+        assert!(d.leading.is_none_or(f64::is_finite));
+        assert!(d.box_size.is_none_or(|b| b.iter().all(|v| v.is_finite())));
+        let mut proj = Project::default();
+        let comp = Comp::new(64, 64, FrameRate::FPS_24, Tick::ZERO);
+        let l = text_layer(&mut proj, &comp, "t", &t);
+        match l.props.prop("transform/position").map(|p| &p.value) {
+            Some(Value::Vec2(v)) => assert!(v.iter().all(|c| c.is_finite())),
+            Some(Value::Vec3(v)) => assert!(v.iter().all(|c| c.is_finite())),
+            other => panic!("unexpected position {other:?}"),
+        }
+    }
 }

crates/engine/src/tests_vector_import.rs

@@ -283,3 +283,57 @@ 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);
+}
+
+#[test]
+fn psd_with_damaged_layer_section_imports_the_flattened_image() {
+    let mut d = WDoc::new(30, 20);
+    d.layers = vec![
+        WLayer::solid("Back", Rect::new(0, 0, 30, 20), [0.0, 0.0, 1.0, 1.0]),
+        WLayer::solid("Box", Rect::new(2, 2, 8, 8), [1.0, 0.0, 0.0, 1.0]).with_blend(b"mul "),
+    ];
+    let mut bytes = write(&d);
+    let at = bytes.windows(8).position(|w| w == b"8BIMmul ").unwrap();
+    bytes[at] = b'X';
+    let path = tmp("damaged.psd");
+    std::fs::write(&path, bytes).unwrap();
+    let mut s = Session::default();
+    let r = s.execute_checked("file.import", json!({"paths": [path], "importAs": "composition"})).unwrap();
+    // Photoshop import warnings are reported with the import's `errors` (as for adjustment layers).
+    let warnings = r["errors"].as_array().unwrap();
+    assert_eq!(warnings.len(), 1, "{r}");
+    assert!(warnings[0].as_str().unwrap().contains("Layers could not be read"), "{r}");
+    let cid = effectcraft_project::ItemId(r["comps"][0].as_u64().unwrap());
+    let c = s.project.comp(cid).unwrap().clone();
+    assert_eq!((c.width, c.height), (30, 20));
+    assert_eq!(c.layers.len(), 1);
+    let LayerSource::Footage { item } = c.layers[0].source else { panic!("footage") };
+    let ItemKind::Footage(f) = &s.project.item(item).unwrap().kind else { panic!() };
+    assert!(f.layer.is_none(), "the merged image, not a Photoshop layer");
+}

crates/psd/src/descriptor.rs

@@ -174,17 +174,17 @@ fn read_value(r: &mut Reader, ty: &[u8; 4], depth: usize) -> Result<DValue> {
             let n = r.u32()?;
             DValue::ObjectArray(n, read_descriptor_at(r, depth + 1)?)
         }
-        b"doub" => DValue::Double(r.f64()?),
+        b"doub" => DValue::Double(r.f64_finite()?),
         b"UntF" => {
             let unit = String::from_utf8_lossy(&r.tag()?).into_owned();
-            DValue::UnitFloat(unit, r.f64()?)
+            DValue::UnitFloat(unit, r.f64_finite()?)
         }
         b"UnFl" => {
             let unit = String::from_utf8_lossy(&r.tag()?).into_owned();
             let n = r.u32()? as usize;
             let mut v = Vec::with_capacity(n.min(1024));
             for _ in 0..n {
-                v.push(r.f64()?);
+                v.push(r.f64_finite()?);
             }
             DValue::UnitFloats(unit, v)
         }
@@ -248,7 +248,11 @@ fn read_value(r: &mut Reader, ty: &[u8; 4], depth: usize) -> Result<DValue> {
 
 // ---------------------------------------------------------------- writing
 
-fn put_u32(out: &mut Vec<u8>, v: u32) {
+pub(crate) fn put_u16(out: &mut Vec<u8>, v: u16) {
+    out.extend_from_slice(&v.to_be_bytes());
+}
+
+pub(crate) fn put_u32(out: &mut Vec<u8>, v: u32) {
     out.extend_from_slice(&v.to_be_bytes());
 }
 
@@ -261,14 +265,21 @@ fn put_id(out: &mut Vec<u8>, id: &str) {
     out.extend_from_slice(id.as_bytes());
 }
 
-pub(crate) fn put_unicode(out: &mut Vec<u8>, s: &str) {
-    let v: Vec<u16> = s.encode_utf16().chain(std::iter::once(0)).collect();
+/// A length-prefixed UTF-16BE string (the length counts code units, including the NUL when
+/// `terminated`).
+pub(crate) fn put_utf16(out: &mut Vec<u8>, s: &str, terminated: bool) {
+    let v: Vec<u16> = s.encode_utf16().chain(terminated.then_some(0)).collect();
     put_u32(out, v.len() as u32);
     for c in v {
-        out.extend_from_slice(&c.to_be_bytes());
+        put_u16(out, c);
     }
 }
 
+/// A descriptor Unicode string: NUL-terminated.
+pub(crate) fn put_unicode(out: &mut Vec<u8>, s: &str) {
+    put_utf16(out, s, true);
+}
+
 /// Serialise a descriptor (without the version field).
 pub fn write_descriptor(out: &mut Vec<u8>, d: &Descriptor) {
     put_unicode(out, &d.class_name);

crates/psd/src/engine_data.rs

@@ -2,6 +2,9 @@
 //! tree of dictionaries (`<< /Key value >>`), arrays (`[ ]`), numbers, names, booleans and
 //! strings (`(þÿ` + UTF-16BE with backslash escapes `)`). Read best-effort.
 
+/// Bytes that end a bare token or a `/Name` (besides whitespace).
+const DELIMITERS: &[u8] = b"[]<>/(";
+
 #[derive(Clone, Debug, PartialEq)]
 pub enum EValue {
     Dict(Vec<(String, EValue)>),
@@ -121,7 +124,7 @@ impl P<'_> {
             b'(' => Some(EValue::Str(self.string())),
             _ => {
                 let start = self.i;
-                while self.i < self.b.len() && !self.b[self.i].is_ascii_whitespace() && !b"[]<>/(".contains(&self.b[self.i]) {
+                while self.i < self.b.len() && !self.b[self.i].is_ascii_whitespace() && !DELIMITERS.contains(&self.b[self.i]) {
                     self.i += 1;
                 }
                 let tok = std::str::from_utf8(&self.b[start..self.i]).unwrap_or("");
@@ -132,15 +135,19 @@ impl P<'_> {
                 Some(match tok {
                     "true" => EValue::Bool(true),
                     "false" => EValue::Bool(false),
-                    t => t.parse::<f64>().map(EValue::Num).unwrap_or_else(|_| EValue::Name(t.to_string())),
+                    // "nan", "inf" and overflowing literals parse as f64 but are not numbers here.
+                    t => match t.parse::<f64>() {
+                        Ok(n) if n.is_finite() => EValue::Num(n),
+                        _ => EValue::Name(t.to_string()),
+                    },
                 })
             }
         }
     }
     fn name(&mut self) -> String {
         self.i += 1; // '/'
         let start = self.i;
-        while self.i < self.b.len() && !self.b[self.i].is_ascii_whitespace() && !b"[]<>/(".contains(&self.b[self.i]) {
+        while self.i < self.b.len() && !self.b[self.i].is_ascii_whitespace() && !DELIMITERS.contains(&self.b[self.i]) {
             self.i += 1;
         }
         String::from_utf8_lossy(&self.b[start..self.i]).into_owned()
@@ -213,6 +220,11 @@ pub struct TextStyle {
     pub box_text: bool,
 }
 
+/// Sane upper bound for a font size in points (Photoshop's own limit is 1296 pt).
+const MAX_FONT_SIZE: f64 = 10_000.0;
+/// Sane bound for tracking (1/1000 em) and leading (points).
+const MAX_SPACING: f64 = 100_000.0;
+
 pub fn text_style(ed: &EValue) -> TextStyle {
     let engine = ed.get("EngineDict").unwrap_or(ed);
     let resources = ed.get("ResourceDict").or_else(|| ed.get("DocumentResources"));
@@ -233,11 +245,11 @@ pub fn text_style(ed: &EValue) -> TextStyle {
     let box_text = engine.path("Rendered/Shapes/Children/0/ShapeType").and_then(EValue::num) == Some(1.0);
     TextStyle {
         font,
-        size: attr("FontSize").and_then(EValue::num),
+        size: attr("FontSize").and_then(EValue::num).filter(|s| *s > 0.0).map(|s| s.min(MAX_FONT_SIZE)),
         color,
         justification,
-        tracking: attr("Tracking").and_then(EValue::num),
-        leading: if auto_leading { None } else { attr("Leading").and_then(EValue::num) },
+        tracking: attr("Tracking").and_then(EValue::num).map(|t| t.clamp(-MAX_SPACING, MAX_SPACING)),
+        leading: if auto_leading { None } else { attr("Leading").and_then(EValue::num).map(|l| l.clamp(-MAX_SPACING, MAX_SPACING)) },
         faux_bold: attr("FauxBold") == Some(&EValue::Bool(true)),
         faux_italic: attr("FauxItalic") == Some(&EValue::Bool(true)),
         box_text,
@@ -285,4 +297,20 @@ mod tests {
         let _ = parse(b"]]]>>>> /x");
         let _ = parse(&[0xFF; 64]);
     }
+
+    #[test]
+    fn non_finite_and_out_of_range_numbers_are_not_numbers() {
+        let b = b"<< /FontSize nan /Tracking inf /Leading -inf /Other 1e999 >>";
+        let v = parse(b);
+        assert_eq!(v.get("FontSize").and_then(EValue::num), None);
+        assert_eq!(v.get("Other").and_then(EValue::num), None);
+        let ed = parse(b"<< /EngineDict << /StyleRun << /RunArray [ << /StyleSheet << /StyleSheetData << /FontSize nan /Tracking inf /Leading -inf /AutoLeading false >> >> >> ] >> >> >>");
+        let st = text_style(&ed);
+        assert_eq!((st.size, st.tracking, st.leading), (None, None, None));
+        let ed = parse(b"<< /EngineDict << /StyleRun << /RunArray [ << /StyleSheet << /StyleSheetData << /FontSize 1e300 /Tracking -1e300 /AutoLeading false /Leading 1e300 >> >> >> ] >> >> >>");
+        let st = text_style(&ed);
+        assert_eq!(st.size, Some(MAX_FONT_SIZE));
+        assert_eq!(st.tracking, Some(-MAX_SPACING));
+        assert_eq!(st.leading, Some(MAX_SPACING));
+    }
 }