From bbe234aaa6cad692a057362065f3adc1ca063cce Mon Sep 17 00:00:00 2001 From: jahala Date: Sat, 10 Oct 2026 06:50:29 +0100 Subject: [PATCH] PDF: a damaged PNG, GIF or WebP is skipped, not fatal (#147) A placed PNG, GIF or WebP that is cut off (an interrupted download) or has a bad checksum made the whole PDF export fail with "PDF writer error: Image(Image(ImageRepr {..}), None, "unexpected end of file")", which names no file. A damaged TIFF or BMP in the same place is skipped with the warning "image `name` could not be decoded and was skipped", and the rest of the document is exported. The exporter decides whether an image can be embedded by asking krilla to open it. krilla opens PNG, GIF and WebP lazily: it reads the header, accepts the file, and decodes the pixels only when the PDF is written, where an error ends the export. TIFF and BMP are decoded by us first, so they took the skip path. lossless() now decodes the image first for every format. A file that does not decode returns None and takes the existing skip-with-warning path. This costs one extra decode per PNG, GIF or WebP asset: the sample magazine (two PNGs, 820 KB) exports in 0.19 s instead of 0.17 s in a debug build. Co-authored-by: Claude Opus 5.5 (1M context) Co-authored-by: Brandon Thomas --- crates/pdf/src/export.rs | 5 ++++- crates/pdf/src/tests.rs | 25 +++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/crates/pdf/src/export.rs b/crates/pdf/src/export.rs index 095ea77c..e5ebb676 100644 --- a/crates/pdf/src/export.rs +++ b/crates/pdf/src/export.rs @@ -1281,6 +1281,10 @@ fn relabel_pdf_header(data: &[u8], from: &[u8], to: &[u8]) -> Option> { } fn lossless(data: &Arc>, fmt: Option, interpolate: bool) -> Option { + // krilla reads PNG, GIF and WebP lazily: a damaged file is accepted here and fails the whole + // export when the PDF is written. Decoding it first means it is skipped, like any other + // image that can't be decoded. + let rgba = designcraft_images::decode_rgba(data)?; let direct = match fmt { Some(image::ImageFormat::Png) => Image::from_png(data.clone().into(), interpolate).ok(), Some(image::ImageFormat::Gif) => Image::from_gif(data.clone().into(), interpolate).ok(), @@ -1289,7 +1293,6 @@ fn lossless(data: &Arc>, fmt: Option, interpolate: b }; direct.or_else(|| { // TIFF, BMP, PSD composites… - let rgba = designcraft_images::decode_rgba(data)?; let (w, h) = rgba.dimensions(); Some(Image::from_rgba8(rgba.into_raw(), w, h)) }) diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 83744a3e..18aa5b41 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -240,6 +240,31 @@ fn place(d: &mut Document, data: Vec, px: (u32, u32)) { d.insert_item(SpreadRef::Doc(0), it, None).unwrap(); } +/// krilla reads PNG, GIF and WebP lazily, so a damaged one passed the exporter's "does it decode" +/// check and failed the whole export when the PDF was written ("PDF writer error: … unexpected +/// end of file"). A cut-off download must be skipped with a warning, as a damaged TIFF is. +#[test] +fn a_cut_off_image_is_skipped_not_fatal() { + let px = image::RgbaImage::from_fn(64, 64, |x, y| image::Rgba([(x * 37 + y * 11) as u8, (x * 5) as u8, (y * 91) as u8, 255])); + for format in [image::ImageFormat::Png, image::ImageFormat::Gif, image::ImageFormat::WebP] { + let mut whole = Vec::new(); + px.write_to(&mut std::io::Cursor::new(&mut whole), format).unwrap(); + let cut = whole[..whole.len() * 9 / 10].to_vec(); + let mut d = doc_with_text("still here"); + place(&mut d, cut, (64, 64)); + let r = export_pdf_with_report(&d, &Cache::new(), &PdfOptions::default()).unwrap_or_else(|e| panic!("{format:?}: {e}")); + assert!(r.warnings.iter().any(|w| w.contains("could not be decoded and was skipped")), "{format:?}: {:?}", r.warnings); + assert!(image_xobjects(&r.bytes).is_empty(), "{format:?}"); + assert!(extract_text(&r.bytes).concat().contains("still"), "{format:?}: the rest of the page is exported"); + // The undamaged file still goes in. + let mut d = doc_with_text("x"); + place(&mut d, whole, (64, 64)); + let r = export_pdf_with_report(&d, &Cache::new(), &PdfOptions::default()).unwrap(); + assert!(r.warnings.is_empty(), "{format:?}: {:?}", r.warnings); + assert_eq!(image_xobjects(&r.bytes).len(), 1, "{format:?}"); + } +} + /// Every image XObject in a PDF: its /ColorSpace name (`?` when not a name) and decoded samples. fn image_xobjects(bytes: &[u8]) -> Vec<(String, Vec)> { use hayro_syntax::object::Name;