Merge pull request #7892 from dotCooCoo/fix/psd-cut-short-slices-warn

PSD: a slices resource cut short warns that the slices were left out

crates/engine/src/cmd/fileio/psdread.rs

@@ -174,23 +174,48 @@ pub fn resolution(bytes: &[u8]) -> Option<(f64, f64)> {
 /// blocks whose signature is `8BIM` are given.
 pub(super) fn resources(resources: &[u8], signed: bool) -> impl Iterator<Item = (u16, &[u8], &[u8])> {
     let mut r = Reader::new(resources);
-    std::iter::from_fn(move || {
-        if r.rest().is_empty() {
-            return None;
+    std::iter::from_fn(move || if r.rest().is_empty() { None } else { block(&mut r).ok() })
+        .filter(move |(ours, ..)| *ours || !signed)
+        .map(|(_, id, name, data)| (id, name, data))
+}
+
+/// The data of the first `8BIM` image resource block `id` of `resources`. `None` when there is
+/// none before the blocks end or before a block that is cut short ahead of it. An error when
+/// that block itself is cut short.
+pub(super) fn resource(resources: &[u8], id: u16) -> Result<Option<&[u8]>, String> {
+    let mut r = Reader::new(resources);
+    while !r.rest().is_empty() {
+        match block(&mut r) {
+            Ok((true, i, _, data)) if i == id => return Ok(Some(data)),
+            Ok(_) => {}
+            Err(Some((true, i))) if i == id => return Err(TRUNCATED.into()),
+            Err(_) => return Ok(None),
         }
-        let ours = r.take(4).ok()? == b"8BIM";
-        let id = r.u16().ok()?;
+    }
+    Ok(None)
+}
+
+/// An image resource block: whether its signature is `8BIM`, its id, its name (the bytes of its
+/// Pascal string) and its data.
+type Block<'a> = (bool, u16, &'a [u8], &'a [u8]);
+
+/// The image resource block that `r` starts with. When it is cut short, the error gives whether
+/// its signature is `8BIM` and its id, when those were read.
+fn block<'a>(r: &mut Reader<'a>) -> Result<Block<'a>, Option<(bool, u16)>> {
+    let ours = r.take(4).map_err(|_| None)? == b"8BIM";
+    let id = r.u16().map_err(|_| None)?;
+    let mut rest = || -> Result<(&'a [u8], &'a [u8]), String> {
         // A Pascal string padded to an even size, length byte included.
-        let len = usize::from(r.u8().ok()?);
-        let name = r.take(len).ok()?;
-        r.take((len + 1) % 2).ok()?;
-        let size = usize::try_from(r.u32().ok()?).ok()?;
-        let data = r.take(size).ok()?;
-        r.take(size % 2).ok()?;
-        Some((ours, id, name, data))
-    })
-    .filter(move |(ours, ..)| *ours || !signed)
-    .map(|(_, id, name, data)| (id, name, data))
+        let len = usize::from(r.u8()?);
+        let name = r.take(len)?;
+        r.take((len + 1) % 2)?;
+        let size = usize::try_from(r.u32()?).map_err(|_| TRUNCATED.to_string())?;
+        let data = r.take(size)?;
+        r.take(size % 2)?;
+        Ok((name, data))
+    };
+    let (name, data) = rest().map_err(|_| Some((ours, id)))?;
+    Ok((ours, id, name, data))
 }
 
 /// The resource naming the document's clipping path.

crates/engine/src/cmd/fileio/psdslices.rs

@@ -18,7 +18,7 @@
 use vectorcraft_doc::{SliceKind, SliceOptions};
 use vectorcraft_geom::Rect;
 
-use super::psdread::{Reader, TRUNCATED, header, resources};
+use super::psdread::{Reader, TRUNCATED, header, resource};
 
 /// The image resource that holds the slices.
 const SLICES: u16 = 1050;
@@ -51,12 +51,12 @@ struct Stored {
 }
 
 /// The slices PSD or PSB file `bytes` stores. `None` when it stores none (no `8BIM` resource 1050
-/// before the image resources end or are cut short). An error when the file isn't a PSD or PSB
-/// file, or its slices can't be read.
+/// before the image resources end, or before a block that is cut short ahead of it). An error when
+/// the file isn't a PSD or PSB file, resource 1050 is cut short, or its slices can't be read.
 fn read(bytes: &[u8]) -> Result<Option<Stored>, String> {
     let mut r = Reader::new(bytes);
     let h = header(&mut r)?;
-    let Some((_, _, data)) = resources(h.resources, true).find(|(id, _, _)| *id == SLICES) else { return Ok(None) };
+    let Some(data) = resource(h.resources, SLICES)? else { return Ok(None) };
     Ok(Some(Stored { slices: decode(data)?, size: (h.width, h.height) }))
 }
 

crates/engine/src/cmd/fileio/tests_psdslices.rs

@@ -371,6 +371,29 @@ fn slices_that_cant_be_read_warn_and_the_file_still_opens() {
     assert!(warnings(&r).is_empty() && slices(&s).is_empty(), "{r}");
 }
 
+#[test]
+fn a_slices_resource_cut_short_warns_and_the_file_still_opens() {
+    let good = v6(&[user("a", [0, 0, 5, 5])], &[]);
+    // Image resource `id`, the last of the image resources, declares 10 bytes more than they hold.
+    let cut = |layers: bool, id: u16, data: &[u8]| {
+        let mut f = with_resource(&psd(layers), id, data);
+        let sig = [&b"8BIM"[..], &id.to_be_bytes()].concat();
+        let at = f.windows(6).rposition(|w| w == sig.as_slice()).unwrap() + 8;
+        f[at..at + 4].copy_from_slice(&(data.len() as u32 + 10).to_be_bytes());
+        f
+    };
+    for layers in [true, false] {
+        let (s, r) = open(&cut(layers, 1050, &good), json!({}));
+        let w = warnings(&r);
+        assert!(w.iter().any(|w| w.starts_with("the slices can't be read (") && w.contains("cut short")), "{w:?}");
+        assert!(slices(&s).is_empty());
+        assert!(s.doc().unwrap().doc.art_bounds().is_some());
+        // Another resource cut short says nothing about slices.
+        let (s, r) = open(&cut(layers, 1051, &[0, 1]), json!({}));
+        assert!(warnings(&r).is_empty() && slices(&s).is_empty(), "{r}");
+    }
+}
+
 #[test]
 fn slices_are_clipped_to_the_canvas_and_those_outside_it_are_left_out() {
     let data = v6(&[user("over", [-5, -5, 5, 30]), user("out", [25, 0, 30, 5]), user("empty", [4, 4, 4, 9]), user("in", [1, 1, 2, 2])], &[]);