fix(psd): recover layered imports with damaged merged data (#6170)

This commit is contained in:
PerchenkoAnton 2026-10-16 04:14:05 +03:00 committed by GitHub
parent b630d7cb9a
commit 3cb3580a46
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 228 additions and 7 deletions

View File

@ -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)?;

View File

@ -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
};

View File

@ -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 {

View File

@ -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")));
}

View File

@ -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<Result<Vec<u8>>>)> {
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<Result<Vec<u8>>>)> {
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))
}

View File

@ -21,7 +21,8 @@ fn collect(dir: &Path, out: &mut Vec<PathBuf>) {
/// 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() {

View File

@ -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(_))));
}
}