Fail closed on malformed PSD XMP and duplicate resources

crates/engine/src/tests_export.rs

@@ -673,6 +673,26 @@ fn remove_location_refuses_a_psd_that_embeds_gps() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+#[test]
+fn remove_location_refuses_a_psd_with_unreadable_xmp() {
+    let dir = temp_dir("export-noloc-psd-xmp");
+    let path = dir.join("Unreadable.psd");
+    let pixels = [40, 80, 120];
+    let image = lightcraft_codecs::EncodeImage::new(1, 1, 3, lightcraft_codecs::Samples::U8(&pixels));
+    let xmp = "<x:xmpmeta><broken";
+    let embedded = lightcraft_codecs::EncodeMeta { xmp: Some(xmp), ..Default::default() };
+    let bytes = lightcraft_codecs::encode_psd(&image, &embedded).unwrap();
+    assert_eq!(lightcraft_meta::extract(&bytes), lightcraft_meta::Metadata::default(), "ordinary extraction stays best-effort");
+    std::fs::write(&path, bytes).unwrap();
+
+    let mut session = Session::new().with_fs();
+    session.execute("library.import", &json!({"paths": [path.to_string_lossy()]})).unwrap();
+    let id = session.catalog.photos().next().unwrap().id;
+    let err = export_photo(&mut session, id, &ExportOptions::from_json(&json!({"format": "original", "removeLocation": true})), 1).err().unwrap();
+    assert!(err.contains("can't verify location metadata") && err.contains("malformed PSD/PSB"), "{err}");
+    let _ = std::fs::remove_dir_all(&dir);
+}
+
 #[test]
 fn remove_location_refuses_a_psd_with_unreadable_resources() {
     let dir = temp_dir("export-noloc-psd-malformed");

crates/meta/README.md

@@ -13,7 +13,7 @@ Photo metadata: EXIF, XMP (read + write), IPTC-IIM, and the containers that carr
 | `write_xmp(&Metadata, Option<&str>) -> String` | Complete `<?xpacket?>` packet; the second argument is LightCraft's full develop settings JSON. |
 | `parse_iptc(&[u8]) -> Metadata` | IIM record 2: title (2:5), keywords (2:25), by-line (2:80), copyright (2:116), caption (2:120), date/time created (2:55/2:60). |
 | `extract(&[u8]) -> Metadata` | Everything for a whole file. Precedence: EXIF → IPTC fills gaps → XMP overrides user fields (rating, label, title, caption, artist, copyright, keywords, GPS) and fills the rest. |
-| `try_extract(&[u8]) -> Result<Metadata, ExtractError>` | Like `extract`, but reports malformed PSD/PSB headers, image-resource sections, or embedded EXIF structure so callers can fail closed when location absence cannot be established. Other formats remain best-effort. |
+| `try_extract(&[u8]) -> Result<Metadata, ExtractError>` | Like `extract`, but reports malformed PSD/PSB headers, image-resource sections, or embedded EXIF/XMP structure (including duplicate EXIF/XMP resources) so callers can fail closed when location absence cannot be established. Other formats remain best-effort. |
 
 `Metadata` carries camera (make, model, serial, software), lens (make, model, serial, specification), capture
 (date-time with UTC offset and milliseconds, exposure time, f-number, ISO incl. extended ISO, focal length and

crates/meta/src/lib.rs

@@ -284,28 +284,35 @@ impl Metadata {
 pub fn extract(bytes: &[u8]) -> Metadata {
     let e = embedded(bytes);
     let exif = e.exif.as_deref().map(read_exif).unwrap_or_default();
-    extract_embedded(e, exif)
+    let xmp = e.xmp.as_deref().and_then(|xmp| parse_xmp(xmp).ok()).map(|xmp| xmp.metadata);
+    extract_embedded(e, exif, xmp)
 }
 
-/// Read metadata while reporting malformed PSD/PSB containers or embedded EXIF instead of treating them as empty.
-/// Other container formats retain [`extract`]'s best-effort behavior.
+/// Read metadata while reporting malformed PSD/PSB containers, embedded EXIF, or embedded XMP instead of treating
+/// them as empty. Other container formats retain [`extract`]'s best-effort behavior.
 pub fn try_extract(bytes: &[u8]) -> Result<Metadata, ExtractError> {
     let e = container::try_embedded(bytes)?;
+    let is_psd = bytes.starts_with(b"8BPS");
     let exif = match e.exif.as_deref() {
-        Some(exif) if bytes.starts_with(b"8BPS") => try_read_exif(exif).map_err(|_| ExtractError::MalformedPsd)?,
+        Some(exif) if is_psd => try_read_exif(exif).map_err(|_| ExtractError::MalformedPsd)?,
         Some(exif) => read_exif(exif),
         None => Metadata::default(),
     };
-    Ok(extract_embedded(e, exif))
+    let xmp = match e.xmp.as_deref() {
+        Some(xmp) if is_psd => Some(parse_xmp(xmp).map_err(|_| ExtractError::MalformedPsd)?.metadata),
+        Some(xmp) => parse_xmp(xmp).ok().map(|xmp| xmp.metadata),
+        None => None,
+    };
+    Ok(extract_embedded(e, exif, xmp))
 }
 
-fn extract_embedded(e: Embedded, mut m: Metadata) -> Metadata {
+fn extract_embedded(e: Embedded, mut m: Metadata, xmp: Option<Metadata>) -> Metadata {
     if let Some(i) = e.iptc.as_deref() {
         m.fill_missing(&parse_iptc(i));
     }
-    if let Some(x) = e.xmp.as_deref().and_then(|x| parse_xmp(x).ok()) {
-        m.overlay_user_fields(&x.metadata);
-        m.fill_missing(&x.metadata);
+    if let Some(xmp) = xmp {
+        m.overlay_user_fields(&xmp);
+        m.fill_missing(&xmp);
     }
     m
 }
@@ -313,14 +320,14 @@ fn extract_embedded(e: Embedded, mut m: Metadata) -> Metadata {
 /// A container-level error reported by [`try_extract`].
 #[derive(Clone, Copy, Debug, PartialEq, Eq)]
 pub enum ExtractError {
-    /// The PSD/PSB header, image-resource section, or embedded EXIF structure is malformed.
+    /// The PSD/PSB header, image-resource section, or embedded EXIF/XMP structure is malformed.
     MalformedPsd,
 }
 
 impl std::fmt::Display for ExtractError {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::MalformedPsd => f.write_str("malformed PSD/PSB header, image-resource section, or embedded EXIF structure"),
+            Self::MalformedPsd => f.write_str("malformed PSD/PSB header, image-resource section, or embedded EXIF/XMP structure"),
         }
     }
 }

crates/meta/src/psd.rs

@@ -61,7 +61,9 @@ fn parse_resources(bytes: &[u8]) -> Option<Embedded> {
             reader.take(1)?;
         }
         match id {
+            1058 if exif.is_some() => return None,
             1058 => exif = Some(data),
+            1060 if xmp.is_some() => return None,
             1060 => xmp = Some(data),
             _ => {}
         }
@@ -182,6 +184,32 @@ mod tests {
         assert!(crate::try_extract(&file).is_ok());
     }
 
+    #[test]
+    fn try_extract_rejects_malformed_psd_xmp_but_extract_remains_best_effort() {
+        let file = image(1, &[], &resource(1060, b"", b"<x:xmpmeta><broken"));
+
+        assert_eq!(crate::extract(&file), Metadata::default());
+        assert!(matches!(crate::try_extract(&file), Err(crate::ExtractError::MalformedPsd)));
+    }
+
+    #[test]
+    fn try_extract_rejects_duplicate_psd_exif_and_xmp_resources() {
+        let gps = dated_metadata();
+        let no_gps = Metadata::default();
+        let gps_exif = crate::write_exif(&gps);
+        let no_gps_exif = crate::write_exif(&no_gps);
+        let gps_xmp = crate::write_xmp(&gps, None);
+        let no_gps_xmp = crate::write_xmp(&no_gps, None);
+
+        let duplicate_exif = [resource(1058, b"", &gps_exif), resource(1058, b"", &no_gps_exif)].concat();
+        let duplicate_xmp = [resource(1060, b"", gps_xmp.as_bytes()), resource(1060, b"", no_gps_xmp.as_bytes())].concat();
+
+        for (id, resources) in [(1058, duplicate_exif), (1060, duplicate_xmp)] {
+            let file = image(1, &[], &resources);
+            assert!(matches!(crate::try_extract(&file), Err(crate::ExtractError::MalformedPsd)), "duplicate resource {id} must be rejected");
+        }
+    }
+
     #[test]
     fn psd_skips_unknown_resources_with_pascal_name_and_payload_padding() {
         let exif = crate::write_exif(&dated_metadata());