From c8390afbd25d5d3361b096794b6d67b95d0ff1fe Mon Sep 17 00:00:00 2001 From: ronibalthazar <61232335+ronibalthazar@users.noreply.github.com> Date: Thu, 22 Oct 2026 14:21:32 +0100 Subject: [PATCH] Fail closed on malformed PSD metadata during original export --- crates/engine/src/export.rs | 12 ++++++++---- crates/engine/src/tests_export.rs | 26 ++++++++++++++++++++++++++ crates/meta/README.md | 1 + crates/meta/src/container.rs | 6 ++++++ crates/meta/src/lib.rs | 29 ++++++++++++++++++++++++++++- crates/meta/src/psd.rs | 2 +- 6 files changed, 70 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/export.rs b/crates/engine/src/export.rs index bfeade59c..0e2254afd 100644 --- a/crates/engine/src/export.rs +++ b/crates/engine/src/export.rs @@ -1827,10 +1827,14 @@ impl PreparedExport { }; let Some(dng) = dng else { // the file is copied byte for byte: refuse rather than leak a location it embeds - if remove_location && lightcraft_meta::extract(&bytes).gps.is_some() { - return Err(format!( - "{label}: can't remove the location from an original file that embeds GPS; export as JPEG, TIFF or DNG instead" - )); + if remove_location { + let metadata = lightcraft_meta::try_extract(&bytes) + .map_err(|e| format!("{label}: can't verify location metadata in this original file: {e}"))?; + if metadata.gps.is_some() { + return Err(format!( + "{label}: can't remove the location from an original file that embeds GPS; export as JPEG, TIFF or DNG instead" + )); + } } return Ok(Exported { file_name, diff --git a/crates/engine/src/tests_export.rs b/crates/engine/src/tests_export.rs index 8113cd70c..74bd7269e 100644 --- a/crates/engine/src/tests_export.rs +++ b/crates/engine/src/tests_export.rs @@ -673,6 +673,32 @@ 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_resources() { + let dir = temp_dir("export-noloc-psd-malformed"); + let gps = lightcraft_meta::Metadata { gps: Some(lightcraft_meta::Gps { latitude: 45.0, longitude: 7.0, altitude: None }), ..Default::default() }; + let path = dir.join("Geo.psd"); + let valid_bytes = synthetic_psd_with_metadata(&gps); + assert_eq!(lightcraft_meta::extract(&valid_bytes).gps, gps.gps, "the fixture embeds EXIF GPS before corruption"); + let mut bytes = valid_bytes; + let colour_length = usize::try_from(u32::from_be_bytes(bytes[26..30].try_into().unwrap())).unwrap(); + let resource_length_at = 30 + colour_length; + let resource_length = usize::try_from(u32::from_be_bytes(bytes[resource_length_at..resource_length_at + 4].try_into().unwrap())).unwrap(); + let resource_start = resource_length_at + 4; + let resource_end = resource_start + resource_length; + bytes.splice(resource_end..resource_end, *b"BAD!"); + bytes[resource_length_at..resource_length_at + 4].copy_from_slice(&u32::try_from(resource_length + 4).unwrap().to_be_bytes()); + + assert!(lightcraft_meta::extract(&bytes).gps.is_none(), "the malformed trailing resource reproduces the metadata gap"); + 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 hdr_png_export_is_16_bit_pq_rec2020() { let mut s = Session::with_demo(); diff --git a/crates/meta/README.md b/crates/meta/README.md index b2642963c..401887c6d 100644 --- a/crates/meta/README.md +++ b/crates/meta/README.md @@ -13,6 +13,7 @@ Photo metadata: EXIF, XMP (read + write), IPTC-IIM, and the containers that carr | `write_xmp(&Metadata, Option<&str>) -> String` | Complete `` 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` | Like `extract`, but reports malformed PSD/PSB headers or image-resource sections 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 diff --git a/crates/meta/src/container.rs b/crates/meta/src/container.rs index ec233c52c..3cfb7e287 100644 --- a/crates/meta/src/container.rs +++ b/crates/meta/src/container.rs @@ -75,6 +75,12 @@ pub fn embedded(bytes: &[u8]) -> Embedded { } } +/// Locate metadata blocks while preserving PSD/PSB resource-structure errors for callers that +/// must establish whether embedded GPS is absent. Other containers keep their best-effort walk. +pub(crate) fn try_embedded(bytes: &[u8]) -> Result { + if bytes.starts_with(b"8BPS") { crate::psd::parse(bytes).ok_or(crate::ExtractError::MalformedPsd) } else { Ok(embedded(bytes)) } +} + const XMP_SIG: &[u8] = b"http://ns.adobe.com/xap/1.0/\0"; const XMP_EXT_SIG: &[u8] = b"http://ns.adobe.com/xmp/extension/\0"; diff --git a/crates/meta/src/lib.rs b/crates/meta/src/lib.rs index d54354838..21dfb3598 100644 --- a/crates/meta/src/lib.rs +++ b/crates/meta/src/lib.rs @@ -282,7 +282,17 @@ impl Metadata { /// Read everything we understand from a whole file (JPEG, PNG, WebP, TIFF-based incl. DNG/raw). /// Precedence: EXIF, then IPTC fills gaps, then XMP overrides user fields and fills the rest. pub fn extract(bytes: &[u8]) -> Metadata { - let e = embedded(bytes); + extract_embedded(embedded(bytes)) +} + +/// Read metadata while reporting malformed PSD/PSB resource sections instead of treating them as empty. +/// Other container formats retain [`extract`]'s best-effort behavior. +pub fn try_extract(bytes: &[u8]) -> Result { + let e = container::try_embedded(bytes)?; + Ok(extract_embedded(e)) +} + +fn extract_embedded(e: Embedded) -> Metadata { let mut m = e.exif.as_deref().map(read_exif).unwrap_or_default(); if let Some(i) = e.iptc.as_deref() { m.fill_missing(&parse_iptc(i)); @@ -294,6 +304,23 @@ pub fn extract(bytes: &[u8]) -> Metadata { m } +/// A container-level error reported by [`try_extract`]. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ExtractError { + /// The PSD/PSB header or image-resource section 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 or image-resource section"), + } + } +} + +impl std::error::Error for ExtractError {} + /// A lens name from its focal and aperture range (Exif LensSpecification, or a Nikon maker note's Lens), written the /// way Lightroom shows a lens it has no name for: `50.0 mm f/1.8`, `18.0-135.0 mm f/3.5-5.6`. pub fn lens_spec_name(spec: [f64; 4]) -> Option { diff --git a/crates/meta/src/psd.rs b/crates/meta/src/psd.rs index 52acaf735..3a3bfc6f7 100644 --- a/crates/meta/src/psd.rs +++ b/crates/meta/src/psd.rs @@ -30,7 +30,7 @@ pub(crate) fn embedded(bytes: &[u8]) -> Embedded { parse(bytes).unwrap_or_default() } -fn parse(bytes: &[u8]) -> Option { +pub(crate) fn parse(bytes: &[u8]) -> Option { let mut reader = Reader { bytes, offset: 0 }; if reader.take(4)? != b"8BPS" || !matches!(reader.u16()?, 1 | 2) { return None;