From 38fb2a5c48912efdf7a482de71cc70dd5e6efa31 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 8 Nov 2026 20:48:17 -0500 Subject: [PATCH] review: refuse an image whose mask can't be decoded at all --- crates/create/src/extract.rs | 76 +++++++++++++++++++----------------- crates/create/src/tests.rs | 31 +++++++++------ 2 files changed, 59 insertions(+), 48 deletions(-) diff --git a/crates/create/src/extract.rs b/crates/create/src/extract.rs index dea09b9549..2429c9dca7 100644 --- a/crates/create/src/extract.rs +++ b/crates/create/src/extract.rs @@ -179,13 +179,16 @@ fn image(doc: &Document, s: &Stream, budget: &mut ImageBudget) -> Result<(&'stat }; }; let mask = s.dict.get(b"ImageMask").is_some_and(|m| matches!(&*doc.resolve(m), Object::Bool(true))); - // A stencil mask (`ImageMask true`) has no /SMask or /Mask of its own (8.9.6.2). - let refused = budget.refused(); - let mut alpha = if mask { None } else { soft_mask(doc, s, w, h, budget).or_else(|| stencil_mask(doc, s, w, h, budget)) }; - // Without its mask the image would come out opaque, showing what the mask hides. - if budget.refused() > refused { - return Err(FilterError::Spent.to_string()); - } + // A stencil mask (`ImageMask true`) has no /SMask or /Mask of its own (8.9.6.2). A mask that + // can't be decoded takes the image with it: without it the image would come out opaque, + // showing what the mask hides. + let mut alpha = if mask { + None + } else if let Some(a) = soft_mask(doc, s, w, h, budget)? { + Some(a) + } else { + stencil_mask(doc, s, w, h, budget)? + }; let key = if mask { None } else { colour_key(doc, s) }; // Keep the existing JPEG and stencil-mask Decode handling separate from lossless colour images. let endpoint = |value: Option<&Object>| value.map(|v| doc.resolve(v)).and_then(|v| v.as_f64()).filter(|v| v.is_finite()); @@ -542,40 +545,46 @@ fn cmyk(c: u8, m: u8, y: u8, k: u8) -> [u8; 3] { [f(c), f(m), f(y)] } +/// Why a mask that is there can't be used: exported without it the image is opaque, showing what +/// the mask hides, so the image goes with it. +fn unreadable_mask() -> String { + "the image's mask can't be decoded, and without it the image would show what it hides".into() +} + /// The soft mask (`/SMask`) as 8-bit alpha, scaled to the image's size when it has another. -fn soft_mask(doc: &Document, s: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Option> { - let r = s.dict.get(b"SMask")?.as_ref()?; +fn soft_mask(doc: &Document, s: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Result>, String> { + let Some(r) = s.dict.get(b"SMask").and_then(Object::as_ref) else { return Ok(None) }; let obj = doc.get(r); - let Object::Stream(m) = &*obj else { return None }; - // A soft mask that cannot be decoded leaves the image showing what the mask hides, so the - // image is refused rather than written out opaque. - gray_plane(doc, m, w, h, budget, true) + let Object::Stream(m) = &*obj else { return Ok(None) }; + gray_plane(doc, m, w, h, budget).map(Some) } /// A stencil `/Mask` (an image mask stream, 8.9.6.3) as 8-bit alpha: its 1 samples (0 with /// `/Decode [1 0]`) hide the image. -fn stencil_mask(doc: &Document, s: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Option> { - let r = s.dict.get(b"Mask")?.as_ref()?; +fn stencil_mask(doc: &Document, s: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Result>, String> { + let Some(r) = s.dict.get(b"Mask").and_then(Object::as_ref) else { return Ok(None) }; let obj = doc.get(r); - let Object::Stream(m) = &*obj else { return None }; - Some(gray_plane(doc, m, w, h, budget, false)?.into_iter().map(|v| 255 - v).collect()) + let Object::Stream(m) = &*obj else { return Ok(None) }; + gray_plane(doc, m, w, h, budget).map(|px| Some(px.into_iter().map(|v| 255 - v).collect())) } /// A one-component mask image as 8-bit values (its `/Decode` interval applied), scaled to /// `w` × `h`. `None` when it can't be decoded (JPEG 2000 masks). -fn gray_plane(doc: &Document, m: &Stream, w: usize, h: usize, budget: &mut ImageBudget, soft: bool) -> Option> { - let (mw, mh) = size(m.dict.int(b"Width")?, m.dict.int(b"Height")?)?; +fn gray_plane(doc: &Document, m: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Result, String> { + let (Some(mw), Some(mh)) = (m.dict.int(b"Width"), m.dict.int(b"Height")) else { return Err(unreadable_mask()) }; + let (mw, mh) = size(mw, mh).ok_or_else(unreadable_mask)?; let stencil = m.dict.get(b"ImageMask").is_some_and(|v| matches!(&*doc.resolve(v), Object::Bool(true))); let f = filters(m); + let refused = budget.refused(); let mut px = match f.last().map(Vec::as_slice) { Some(b"DCTDecode" | b"DCT") if f.len() == 1 => { - // Past the allowance the image goes too: `image` sees this one refused. + // Past the allowance the image goes too: `refused` has this one counted. if !budget.pay_jpeg((mw as u64).saturating_mul(mh as u64), &m.raw) { - return None; + return Err(FilterError::Spent.to_string()); } - jpeg_of_size(&m.raw, mw, mh)?.into_luma8().into_raw() + jpeg_of_size(&m.raw, mw, mh).ok_or_else(unreadable_mask)?.into_luma8().into_raw() } - Some(b"DCTDecode" | b"DCT" | b"JPXDecode") => return None, + Some(b"DCTDecode" | b"DCT" | b"JPXDecode") => return Err(unreadable_mask()), _ => { let bpc = if stencil { 1 } else { m.dict.int(b"BitsPerComponent").unwrap_or(8) }; let bpc = match bpc { @@ -584,21 +593,16 @@ fn gray_plane(doc: &Document, m: &Stream, w: usize, h: usize, budget: &mut Image 4 => 4, 8 => 8, 16 => 16, - _ => return None, + _ => return Err(unreadable_mask()), }; let max = (1u32 << bpc.min(8)) - 1; let data = match decoded(doc, m, (mw, mh, 1, bpc), budget) { Ok(data) => data, - Err(_) => { - // A stage limit stops the mask being decoded: a soft mask leaves the image - // refusing it rather than showing what the mask hides. - if soft { - budget.refuse(); - } - return None; - } + // A stage limit stops the mask being decoded, and has it counted already. + Err(_) if budget.refused() > refused => return Err(FilterError::Spent.to_string()), + Err(_) => return Err(unreadable_mask()), }; - unpack(&data, mw, mh, 1, bpc)?.into_iter().map(|v| (v as u32 * 255 / max) as u8).collect() + unpack(&data, mw, mh, 1, bpc).ok_or_else(unreadable_mask)?.into_iter().map(|v| (v as u32 * 255 / max) as u8).collect() } }; if let Some([lo, hi]) = decode(doc, m, 1).and_then(|pairs| pairs.first().copied()) { @@ -608,10 +612,10 @@ fn gray_plane(doc: &Document, m: &Stream, w: usize, h: usize, budget: &mut Image } } if (mw, mh) == (w, h) { - return Some(px); + return Ok(px); } - let img = image::GrayImage::from_raw(mw as u32, mh as u32, px)?; - Some(scaled(&img, w as u32, h as u32).into_raw()) + let img = image::GrayImage::from_raw(mw as u32, mh as u32, px).ok_or_else(unreadable_mask)?; + Ok(scaled(&img, w as u32, h as u32).into_raw()) } /// `img` scaled to `w` × `h`. Scaling goes down first, through a buffer of `img`'s width by `h` at diff --git a/crates/create/src/tests.rs b/crates/create/src/tests.rs index 39cfd7dcbf..23fad4a450 100644 --- a/crates/create/src/tests.rs +++ b/crates/create/src/tests.rs @@ -2159,11 +2159,12 @@ fn jbig2_images_export_as_their_picture() { large[11..15].copy_from_slice(&20_000u32.to_be_bytes()); large[15..19].copy_from_slice(&10_000u32.to_be_bytes()); assert!(skipped(&page(20_000, 4, &large, None, false)), "a page far taller than the image"); - // Refused the same ways as a stencil /Mask, it is left out: the image exports opaque. + // A stencil /Mask that can't be decoded takes its image with it: left out, the image would + // export opaque, showing what the mask hides. for (width, height, mask) in [(w + 8, h, &jbig2), (20_000, 4, &large)] { - let rgba = exported_rgba(&page(width, height, mask, None, true)).1; - assert_eq!(rgba.len() as i64, width * height * 4, "a {width}-wide mask"); - assert!(alphas(&rgba).iter().all(|a| *a == 255), "a refused {width}-wide mask is left out"); + let out = extract_images(&page(width, height, mask, None, true), &[0], 0); + assert!(out.images.is_empty(), "a {width}-wide mask is left out"); + assert!(out.skipped.iter().any(|(_, _, why)| why.contains("mask")), "a {width}-wide mask's image is skipped: {:?}", out.skipped); } // Damaged data never panics. for end in [0, 1, 10, 20, 30, jbig2.len() / 2] { @@ -2425,7 +2426,8 @@ fn a_jpeg_mask_pays_for_its_samples() { /// What decoding a JPEG takes goes by its header, so that is what has to agree with the /// dictionary before it is decoded: one image of a pixel can hold a page of 500 megapixels. The -/// file comes out as it is either way, so what shows is whether it was decoded. +/// file comes out as it is either way, so what shows is whether it was decoded; read as a mask +/// that can't be decoded, it takes the image with it. #[test] fn a_jpeg_larger_than_its_dictionary_says_isnt_decoded() { use pdfcraft_cos::{Dict, Document, ImageBudget, Object, Stream}; @@ -2452,7 +2454,8 @@ fn a_jpeg_larger_than_its_dictionary_says_isnt_decoded() { let (honest, lying) = (doc.add(Object::Stream(keyed(4))), doc.add(Object::Stream(keyed(1)))); assert_eq!(decodes(&doc, honest), 1); assert_eq!(decodes(&doc, lying), 0); - // The same as a mask: read as none, without being decoded. + // The same as a mask: its header disagreeing with its dictionary, it isn't decoded, and the + // image goes with it rather than being written out opaque. let mut mask = keyed(1); mask.dict.remove(b"Mask"); let mask = doc.add(Object::Stream(mask)); @@ -2464,19 +2467,23 @@ fn a_jpeg_larger_than_its_dictionary_says_isnt_decoded() { image.set(b"ColorSpace".to_vec(), Object::name("DeviceGray")); image.set(b"SMask".to_vec(), Object::Ref(mask)); let image = doc.add(Object::Stream(Stream::from_raw(image, vec![0x80]))); - assert_eq!(decodes(&doc, image), 0); + let unmasked = || { + crate::extract::JPEGS_DECODED.with(|n| n.set(0)); + let why = crate::extract::image_file_within(&doc, image, &mut ImageBudget::with_work(u64::MAX)).unwrap_err(); + (crate::extract::JPEGS_DECODED.with(std::cell::Cell::get), why) + }; + let (decoded, why) = unmasked(); + assert_eq!(decoded, 0, "read as none, without being decoded"); + assert!(why.contains("mask"), "{why}"); // Reading the header to find that out is paid for: a mask can be mostly tables, shared by // every image of a file, and with nothing left it isn't read. - let mut budget = ImageBudget::with_work(u64::MAX); - assert!(crate::extract::image_file_within(&doc, image, &mut budget).is_ok()); - assert_eq!(u64::MAX - budget.work_left(), 2 + 4 * jpeg.len() as u64 + 2, "its one declared sample twice, its bytes, the image's byte and pixel"); let mut budget = ImageBudget::with_work(0); assert!(crate::extract::image_file_within(&doc, image, &mut budget).is_err()); assert_eq!(budget.refused(), 1, "the image goes, with its mask unread"); } -/// A mask past the allowance can't be left out as an unreadable one is, or the image comes out -/// opaque, showing what the mask hides: the image goes, with the reason. +/// A mask past the allowance, like one that can't be decoded, can't be left out either: the image +/// would come out opaque, showing what the mask hides. The image goes, with the reason. #[test] fn an_image_whose_jbig2_mask_is_past_the_allowance_is_left_out() { use pdfcraft_cos::{Dict, Document, ImageBudget, Object, Stream};