review: refuse an image whose mask can't be decoded at all

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<Vec<u8>> {
-    let r = s.dict.get(b"SMask")?.as_ref()?;
+fn soft_mask(doc: &Document, s: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Result<Option<Vec<u8>>, 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<Vec<u8>> {
-    let r = s.dict.get(b"Mask")?.as_ref()?;
+fn stencil_mask(doc: &Document, s: &Stream, w: usize, h: usize, budget: &mut ImageBudget) -> Result<Option<Vec<u8>>, 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<Vec<u8>> {
-    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<Vec<u8>, 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

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