diff --git a/crates/io/src/lib.rs b/crates/io/src/lib.rs index a620ddcf33..24b8b3f948 100644 --- a/crates/io/src/lib.rs +++ b/crates/io/src/lib.rs @@ -275,7 +275,11 @@ fn import_stages(name: &str, bytes: &[u8], ctl: &photocraft_raster::Interrupt, m } if is_psd(bytes) { // A merged image the import reads is decoded with the parse: a ZIP one is inflated once. - let (file, merged) = PsdFile::from_bytes_and_merged(bytes, |f| psd_import::check_group_depth(f).is_ok() && psd_import::merged_needed(f))?; + let (file, merged) = PsdFile::from_bytes_and_merged_recovering( + bytes, + |f| psd_import::check_group_depth(f).is_ok() && psd_import::merged_needed(f), + psd_import::can_recover_layers, + )?; // Nesting past the document model's cap could never be saved; reject the file with the // actionable limit. psd_import::check_group_depth(&file)?; diff --git a/crates/io/src/psd_import.rs b/crates/io/src/psd_import.rs index c20c1b7fe7..12307f32ba 100644 --- a/crates/io/src/psd_import.rs +++ b/crates/io/src/psd_import.rs @@ -764,6 +764,12 @@ pub(crate) fn merged_needed(file: &PsdFile) -> bool { !layered || file.layers().is_empty() || usize::from(h.channels) > mode.color_channels() + usize::from(file.merged_has_alpha()) } +/// A damaged merged preview need not prevent opening independently stored layers (#6152). +/// Other modes are imported flattened, so their merged image cannot be discarded. +pub(crate) fn can_recover_layers(file: &PsdFile) -> bool { + matches!(file.header.color_mode, PsdMode::Grayscale | PsdMode::Rgb | PsdMode::Cmyk | PsdMode::Lab) && file.header.depth != 1 && !file.layers().is_empty() +} + /// [`psd_to_document_with`] with the merged image already decoded (by /// [`PsdFile::from_bytes_and_merged`] with [`merged_needed`]), so it is not decoded again. pub(crate) fn psd_to_document_decoded( @@ -901,9 +907,15 @@ pub(crate) fn psd_to_document_decoded( ctl.progress(0.1); if let Err(e) = &m { cx.warn(format!("merged image could not be decoded: {e}")); + if can_recover_layers(file) { + cx.warn("extra alpha/spot channels in the merged image could not be recovered"); + } } Some(m) } else { + if let Some(Err(e)) = decoded { + cx.warn(format!("merged image could not be decoded; opened the available layers instead: {e}")); + } None }; diff --git a/crates/io/tests/corpus.rs b/crates/io/tests/corpus.rs index dd96d96c7f..710b2a8644 100644 --- a/crates/io/tests/corpus.rs +++ b/crates/io/tests/corpus.rs @@ -132,7 +132,7 @@ thread_local! { } /// Files known to be invalid upstream: their parse error is expected. -const KNOWN_BAD: &[(&str, &str)] = &[("group-divider-blend-mode.psd", "psd-tools fixture stripped to 1906 bytes: merged image data missing")]; +const KNOWN_BAD: &[(&str, &str)] = &[("group-divider-blend-mode.psd", "malformed merged image data; recovery is tested in psd_damaged_merged.rs (#6152)")]; /// One corpus directory with its own floors. struct Source { diff --git a/crates/io/tests/psd_damaged_merged.rs b/crates/io/tests/psd_damaged_merged.rs new file mode 100644 index 0000000000..e3d0970dd9 --- /dev/null +++ b/crates/io/tests/psd_damaged_merged.rs @@ -0,0 +1,125 @@ +//! #6152: intact layers can be recovered when only the merged preview is damaged. + +use photocraft_color::{ColorMode, SampleType}; +use photocraft_doc::{Document, Layer}; +use photocraft_geom::Size; +use photocraft_io::{ExportOptions, export, import}; +use photocraft_psd::{Compression, GroupSpec, PsdBuilder, PsdFile}; + +fn layered(depth: SampleType, extension: &str) -> PsdFile { + layered_in_mode(ColorMode::Rgb, depth, extension) +} + +fn layered_in_mode(mode: ColorMode, depth: SampleType, extension: &str) -> PsdFile { + let mut doc = Document::new("recovery", Size::new(3, 2), mode, depth); + let format = doc.pixel_format(); + let mut layer = Layer::raster("red pixels", doc.pixel_format()); + let mut pixel = vec![0.0; format.channels()]; + photocraft_raster::from_rgba_into(&format, [1.0, 0.0, 0.0, 1.0], &mut pixel); + layer.surface_mut().unwrap().fill_rect(doc.bounds(), &pixel); + doc.layers.push(layer); + PsdFile::from_bytes(&export(&doc, extension, &ExportOptions::default()).unwrap().bytes).unwrap() +} + +#[test] +fn truncated_merged_image_keeps_layer_pixels_at_every_depth_and_compression() { + use ColorMode::{Cmyk, Grayscale, Lab, Rgb}; + use SampleType::{F32, U8, U16}; + for extension in ["x.psd", "x.psb"] { + for (mode, depth) in + [(Rgb, U8), (Rgb, U16), (Rgb, F32), (Grayscale, U8), (Grayscale, U16), (Grayscale, F32), (Cmyk, U8), (Cmyk, U16), (Lab, U8), (Lab, U16)] + { + for compression in Compression::ALL { + let mut file = layered_in_mode(mode, depth, extension); + let intact = import(extension, &file.to_bytes().unwrap()).unwrap(); + let expected = photocraft_compose::flatten(&intact.document).px; + let pixels = file.decode_merged().unwrap(); + file.image_data = photocraft_psd::ImageData::encode(compression, &pixels, &file.header).unwrap(); + file.image_data.data.truncate(file.image_data.data.len() / 2); + let bytes = file.to_bytes().unwrap(); + assert!(PsdFile::from_bytes(&bytes).is_err(), "strict parsing stays strict"); + let result = import(extension, &bytes).expect("recover the intact layers"); + assert_eq!(result.document.layer_count(), 1); + assert_eq!(result.document.depth, depth); + assert!(result.warnings.iter().any(|w| w.contains("merged image")), "{:?}", result.warnings); + let rendered = photocraft_compose::flatten(&result.document); + assert_eq!(rendered.px, expected, "{extension} {mode:?} {depth:?} {compression:?}"); + // Saving the recovered document replaces the bad preview with a valid one. + let saved = export(&result.document, extension, &ExportOptions::default()).unwrap(); + assert!(PsdFile::from_bytes(&saved.bytes).unwrap().decode_merged().is_ok()); + } + } + } +} + +#[test] +fn corrupt_zip_merged_image_keeps_the_layers() { + let mut file = layered(SampleType::U8, "x.psd"); + file.image_data = photocraft_psd::ImageData::encode(Compression::Zip, &file.decode_merged().unwrap(), &file.header).unwrap(); + *file.image_data.data.last_mut().unwrap() ^= 0xff; + let result = import("x.psd", &file.to_bytes().unwrap()).unwrap(); + assert_eq!(result.document.layer_count(), 1); + assert!(result.warnings.iter().any(|w| w.contains("merged image"))); +} + +#[test] +fn an_empty_group_survives_a_truncated_preview() { + // The reported psd-tools fixture has only the two records of an empty group. + let mut builder = PsdBuilder::new(100, 100); + builder.begin_group(GroupSpec::new("empty group")); + builder.end_group().unwrap(); + let mut file = builder.build().unwrap(); + file.image_data.data.truncate(file.image_data.data.len() / 2); + let result = import("empty.psd", &file.to_bytes().unwrap()).unwrap(); + assert_eq!(result.document.layer_count(), 1); + assert_eq!(result.document.layers[0].name, "empty group"); + assert!(result.warnings.iter().any(|w| w.contains("merged image"))); +} + +#[test] +fn a_flattened_file_with_no_recoverable_layers_still_fails() { + let mut file = layered(SampleType::U8, "x.psd"); + file.layer_info = None; + file.image_data.data.truncate(file.image_data.data.len() / 2); + assert!(import("flat.psd", &file.to_bytes().unwrap()).is_err()); +} + +#[test] +fn modes_imported_flattened_do_not_recover_from_unusable_layer_records() { + for mode in [photocraft_psd::ColorMode::Indexed, photocraft_psd::ColorMode::Multichannel, photocraft_psd::ColorMode::Duotone] { + let mut file = layered(SampleType::U8, "x.psd"); + file.header.color_mode = mode; + file.image_data = photocraft_psd::ImageData { compression: Compression::Raw, data: Vec::new() }; + assert!(import("flat-mode.psd", &file.to_bytes().unwrap()).is_err(), "{mode:?}"); + } +} + +#[test] +fn lost_extra_channels_are_reported_while_layers_remain_usable() { + let mut file = layered(SampleType::U8, "x.psd"); + file.header.channels = 5; + file.image_data = photocraft_psd::ImageData { compression: Compression::Raw, data: Vec::new() }; + let result = import("extra-channel.psd", &file.to_bytes().unwrap()).unwrap(); + assert_eq!(result.document.layer_count(), 1); + assert!(result.document.channels.is_empty()); + assert!(result.warnings.iter().any(|w| w.contains("alpha/spot channels")), "{:?}", result.warnings); +} + +#[test] +fn truncated_layer_sections_are_not_recovered() { + let file = layered(SampleType::U8, "x.psd"); + let mut bytes = file.to_bytes().unwrap(); + bytes.truncate(bytes.len() - file.image_data.data.len() - 3); + assert!(import("broken-layers.psd", &bytes).is_err()); +} + +#[cfg(feature = "corpus")] +#[test] +fn reported_group_divider_fixture_opens_with_its_group() { + let path = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../corpus/psd-tools/blend-modes/group-divider-blend-mode.psd"); + let bytes = std::fs::read(path).expect("run cargo xtask corpus --psd-tools"); + let result = import("group-divider-blend-mode.psd", &bytes).expect("#6152 opens"); + assert_eq!(result.document.layer_count(), 1); + assert!(matches!(result.document.layers[0].content, photocraft_doc::LayerContent::Group(_))); + assert!(result.warnings.iter().any(|w| w.contains("merged image"))); +} diff --git a/crates/psd/src/file.rs b/crates/psd/src/file.rs index 5a1ee15c1c..e14bc45aaf 100644 --- a/crates/psd/src/file.rs +++ b/crates/psd/src/file.rs @@ -102,6 +102,22 @@ impl PsdFile { /// Errors are those of [`Self::from_bytes`]; a merged image that passes the parse's check but /// cannot be decoded is the inner `Err`, as [`Self::decode_merged`] would return it. pub fn from_bytes_and_merged(data: &[u8], want: impl FnOnce(&PsdFile) -> bool) -> Result<(Self, Option>>)> { + Self::from_bytes_and_merged_recovering(data, want, |_| false) + } + + /// Parses like [`Self::from_bytes_and_merged`], but lets `recover` accept damage confined to + /// the merged image after all layer records and channel data have been read successfully. + /// Callers should opt in only when they can reconstruct the document from its layers. + /// + /// The original encoded image bytes are preserved and its parse error is returned as + /// `Some(Err(..))`, even when `want` did not request a decode. The caller must report that + /// error rather than silently discarding the merged image (which may hold extra channels). + /// Header/layer errors, missing compression fields and parse safety-limit errors stay fatal. + pub fn from_bytes_and_merged_recovering( + data: &[u8], + want: impl FnOnce(&PsdFile) -> bool, + recover: impl FnOnce(&PsdFile) -> bool, + ) -> Result<(Self, Option>>)> { let mut r = Reader::new(data); let header = Header::read(&mut r)?; let psb = header.version.is_psb(); @@ -161,10 +177,30 @@ impl PsdFile { let image_data = ImageData { compression: Compression::Raw, data: Vec::new() }; let mut file = PsdFile { header, color_mode_data, resources, layer_info, layer_info_placement, global_layer_mask, global_blocks, layer_mask_trailing, image_data }; - let merged = if want(&file) { Some(file.read_merged_decoded(&mut r)?) } else { None }; - if merged.is_none() { - file.image_data = ImageData::read(&mut r, &file.header)?; - } + let mut encoded = r.clone(); + let merged = if want(&file) { + file.read_merged_decoded(&mut r).map(Some) + } else { + ImageData::read(&mut r, &file.header).map(|image| { + file.image_data = image; + None + }) + }; + let merged = match merged { + Ok(merged) => merged, + Err(error) => { + if matches!(error, PsdError::LimitExceeded(_)) || !recover(&file) { + return Err(error); + } + let compression = Compression::from_u16(encoded.u16()?); + let source = encoded.peek_rest(); + let mut data = Vec::new(); + data.try_reserve_exact(source.len()).map_err(|_| PsdError::LimitExceeded("not enough memory to retain the damaged merged image"))?; + data.extend_from_slice(source); + file.image_data = ImageData { compression, data }; + Some(Err(error)) + } + }; Ok((file, merged)) } diff --git a/crates/psd/tests/corpus.rs b/crates/psd/tests/corpus.rs index 19b1e56600..90abb4fefd 100644 --- a/crates/psd/tests/corpus.rs +++ b/crates/psd/tests/corpus.rs @@ -21,7 +21,8 @@ fn collect(dir: &Path, out: &mut Vec) { /// Files known to be invalid upstream, with the reason. They are reported but /// do not fail the test. -const KNOWN_BAD: &[(&str, &str)] = &[("group-divider-blend-mode.psd", "psd-tools fixture stripped to 1906 bytes: merged image data missing")]; +const KNOWN_BAD: &[(&str, &str)] = + &[("group-divider-blend-mode.psd", "malformed merged image data; strict parsing rejects it, but the importer recovers its group (#6152)")]; #[test] fn corpus_parse_and_byte_stable() { diff --git a/crates/psd/tests/merged_recovery.rs b/crates/psd/tests/merged_recovery.rs new file mode 100644 index 0000000000..40e310d8a4 --- /dev/null +++ b/crates/psd/tests/merged_recovery.rs @@ -0,0 +1,43 @@ +//! Opt-in merged-image recovery must not weaken strict parsing or lose encoded bytes. + +use photocraft_psd::{Compression, PsdError, PsdFile, Version, compression, testgen}; + +#[test] +fn recovery_is_explicit_and_preserves_the_original_file() { + for version in [Version::Psd, Version::Psb] { + for compression in Compression::ALL { + let mut original = testgen::small(version, compression); + original.image_data.data.pop(); + let bytes = original.to_bytes().unwrap(); + let strict_error = PsdFile::from_bytes(&bytes).unwrap_err(); + for want in [false, true] { + let rejected = PsdFile::from_bytes_and_merged_recovering(&bytes, |_| want, |_| false).unwrap_err(); + assert_eq!(rejected, strict_error); + let (file, merged) = PsdFile::from_bytes_and_merged_recovering(&bytes, |_| want, |_| true).unwrap(); + assert_eq!(merged.unwrap().unwrap_err(), strict_error); + assert_eq!(file, original); + assert_eq!(file.to_bytes().unwrap(), bytes, "{version:?} {compression:?}"); + } + } + } +} + +#[test] +fn recovery_does_not_accept_missing_compression_or_incomplete_layers() { + let original = testgen::small(Version::Psd, Compression::Raw); + let bytes = original.to_bytes().unwrap(); + let merged_offset = bytes.len() - original.image_data.data.len() - 2; + for cut in [merged_offset - 1, merged_offset, merged_offset + 1] { + assert!(PsdFile::from_bytes_and_merged_recovering(&bytes[..cut], |_| true, |_| true).is_err(), "cut {cut}"); + } +} + +#[test] +fn recovery_never_suppresses_a_merged_stream_limit_error() { + let mut original = testgen::small(Version::Psd, Compression::Zip); + original.image_data.data = compression::zip_compress(&vec![0; (1 << 20) + 4096]); + let bytes = original.to_bytes().unwrap(); + for want in [false, true] { + assert!(matches!(PsdFile::from_bytes_and_merged_recovering(&bytes, |_| want, |_| true), Err(PsdError::LimitExceeded(_)))); + } +}