From 5480dbe163ea5327bbd738ce0c679b69a9f57c7d Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Fri, 30 Oct 2026 19:29:27 +0100 Subject: [PATCH 1/2] M2.6: shared JBIG2 globals are read once a document; Copy Image is tested on JBIG2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From CodeRabbit's review of the stack: - Every JBIG2 image read its /JBIG2Globals afresh, within 32 MiB each but with no bound across an export or a redaction. A globals stream of the file, not edited since it was read and with its /DecodeParms direct, now has its filters undone once a document and is kept for the images that share it (in every clone, which share the file), up to 64 MiB in all. An edited one, one added in memory, or one with /DecodeParms by reference (another object, which can change under it) is read each time as before. This saves the filters only: the decoder reads the segments afresh for every image, so a real symbol dictionary still costs its decoding once an image. - view.copy-image dropped "JBIG2 inside other filters" from its missing list on the strength of Export's test; Copy Image now has its own: a JBIG2 picture behind ASCIIHexDecode is found under the pointer and copied as its pixels and as a PNG, and one whose page information is in its globals is read through them. A JBIG2 image's /Width and /Height are read as given directly; one by reference is an error rather than an image of no rows. A /JBIG2Globals reference to no object still reads as no globals: it is a reference to null (ISO 32000-2 §7.3.10), which an entry may hold for being absent (§7.3.7), and the renderer draws the page so. An image that needed those globals then mostly fails to decode; in a probe of twelve pictures that decoded with them, eleven were refused and one decoded to other pixels. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- Cargo.lock | 1 + crates/cos/README.md | 4 +- crates/cos/src/document.rs | 3 + crates/cos/src/document/objects.rs | 57 ++++++++--- crates/cos/src/document/open.rs | 2 + crates/cos/src/document/tests.rs | 153 +++++++++++++++++++++++++++++ crates/engine/Cargo.toml | 2 + crates/engine/src/tests.rs | 99 +++++++++++++++++++ parity/acrobat-features.toml | 4 +- 9 files changed, 310 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bf3be727f4..410b347b5f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4395,6 +4395,7 @@ dependencies = [ "pdfcraft-create", "pdfcraft-edit", "pdfcraft-export", + "pdfcraft-filters", "pdfcraft-fonts", "pdfcraft-forms", "pdfcraft-geom", diff --git a/crates/cos/README.md b/crates/cos/README.md index 8362012225..1e2551036f 100644 --- a/crates/cos/README.md +++ b/crates/cos/README.md @@ -91,5 +91,7 @@ recognisable in a diff and byte-stable when re-serialized. - Structural validation against the Arlington model (`core.arlington-validation`), which will live in the reserved `arlington` crate. - Image codecs. `pdfcraft-filters` carries the non-image filters and CCITT fax and JBIG2 decoding - (`Document::decoded_image`, a JBIG2 image's `/JBIG2Globals` within `MAX_JBIG2_GLOBALS`, 32 MiB); + (`Document::decoded_image`, a JBIG2 image's `/JBIG2Globals` within `MAX_JBIG2_GLOBALS`, 32 MiB, + their filters undone once a document for the images that share them, up to 64 MiB kept, while + their segments are decoded for every image); DCT and JPX are still decoded through the bootstrap renderer (`core.image-filters`, M2.6). diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index c0bd656536..082d1910d1 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -49,6 +49,9 @@ pub struct Document { repair_log: Arc>, cache: Arc>>>, objstms: Arc>>>, + /// What `/JBIG2Globals` streams of the file decode to, by object number: the images of a scan + /// share them, and each would otherwise decode them again (see `Document::jbig2_globals`). + jbig2_globals: Arc>>>>, stream_limit: usize, overlay: BTreeMap, next_num: u32, diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index bec4a90717..39a4d9a6da 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -5,6 +5,10 @@ use super::{ Arc, CosError, Cow, Dict, Document, Lexer, MAX_DECODED, MAX_JBIG2_GLOBALS, ObjRef, Object, Stream, is_whitespace, parse_indirect_shared, }; +/// The most decoded `/JBIG2Globals` a document keeps for its images to share (see +/// `Document::jbig2_globals`): two streams at [`MAX_JBIG2_GLOBALS`]. +const JBIG2_GLOBALS_HELD: usize = 2 * MAX_JBIG2_GLOBALS; + impl Document { pub fn bytes(&self) -> &Arc> { &self.data @@ -301,13 +305,13 @@ impl Document { }; // `/Width` and `/Height` are read as given directly, as every caller reads them first: one // given by reference has the image refused there, before it gets here. - let height = s.dict.int(b"Height").and_then(|h| u32::try_from(h).ok()).unwrap_or(0); + let height = s.dict.int(b"Height").and_then(|h| u32::try_from(h).ok()); if let Some(((pdfcraft_filters::Filter::Jbig2, _), before)) = chain.split_last() { - let Some(columns) = s.dict.int(b"Width").and_then(|w| u32::try_from(w).ok()) else { - return Err(CosError::Filter("a JBIG2 image's /Width isn't a size".into())); + let (Some(columns), Some(height)) = (s.dict.int(b"Width").and_then(|w| u32::try_from(w).ok()), height) else { + return Err(CosError::Filter("a JBIG2 image's /Width or /Height isn't a size".into())); }; let globals = self.jbig2_globals(s)?; - return pdfcraft_filters::decode_jbig2(&undo(before)?, globals.as_deref(), columns, height, max).map_err(filter); + return pdfcraft_filters::decode_jbig2(&undo(before)?, globals.as_deref().map(Vec::as_slice), columns, height, max).map_err(filter); } let Some(p) = self.fax_params(s) else { return s.decoded_within(max) }; // As many rows as the image has: a /Rows below /Height would cut the picture short, and @@ -317,24 +321,53 @@ impl Document { return Err(CosError::Filter("a fax image's /Columns aren't its /Width".into())); } let stride = usize::try_from(p.columns).unwrap_or(usize::MAX).div_ceil(8).max(1); - let rows = u32::try_from(max / stride).unwrap_or(u32::MAX).min(height); + let rows = u32::try_from(max / stride).unwrap_or(u32::MAX).min(height.unwrap_or(0)); let data = undo(chain.split_last().map_or(&[][..], |(_, before)| before))?; pdfcraft_filters::decode_ccitt(&data, &p, rows).map_err(filter) } /// What the `/JBIG2Globals` stream of JBIG2 image `s` (in its last filter's `/DecodeParms`) - /// decodes to, within [`MAX_JBIG2_GLOBALS`]; `None` without one. Globals that can't be read - /// are an error, rather than the image decoded without them. - fn jbig2_globals(&self, s: &Stream) -> Result>, CosError> { - let globals = self.last_filter_parms(s).get(b"JBIG2Globals").map(|g| self.resolve(g)); - let Some(Object::Stream(g)) = globals.as_deref() else { return Ok(None) }; - self.with_direct_parms(g).decoded_within(MAX_JBIG2_GLOBALS).map(Some).map_err(|e| { + /// decodes to, within [`MAX_JBIG2_GLOBALS`]; `None` without one (a reference to no object is + /// one to null, ISO 32000-2 §7.3.10, which is an absent entry, §7.3.7). The image is then read + /// without them, as the renderer draws it, so an image that needed them mostly fails to decode + /// and can decode to other pixels. A stream that is there and can't be read is an error, rather + /// than the image decoded without it. + /// + /// The images of a scan share one globals stream, so a stream of the file, not edited since + /// it was read (the same in every clone) and with its `/DecodeParms` direct (by reference they + /// are another object, which can change while this one doesn't), has its filters undone once a + /// document and is kept, up to [`JBIG2_GLOBALS_HELD`] in all; others are read each time. That + /// saves the filters only: the segments it holds are decoded again for every image. + fn jbig2_globals(&self, s: &Stream) -> Result>>, CosError> { + let Some(entry) = self.last_filter_parms(s).get(b"JBIG2Globals").cloned() else { return Ok(None) }; + let held = match entry { + Object::Ref(r) if !self.overlay.contains_key(&r.num) => Some(r.num), + _ => None, + }; + if let Some(hit) = held.and_then(|num| self.jbig2_globals.lock().ok()?.get(&num).cloned()) { + return Ok(Some(hit)); + } + let Object::Stream(g) = &*self.resolve(&entry) else { return Ok(None) }; + let readable = self.with_direct_parms(g); + let held = held.filter(|_| matches!(readable, Cow::Borrowed(_))); + let mut data = readable.decoded_within(MAX_JBIG2_GLOBALS).map_err(|e| { let detail = match e { CosError::Filter(detail) => detail, e => e.to_string(), }; CosError::Filter(format!("the JBIG2 globals can't be read: {detail}")) - }) + })?; + // What is kept counts by its length, so that's all it may hold on to. + data.shrink_to_fit(); + let data = Arc::new(data); + // A poisoned lock only means nothing is kept. + if let (Some(num), Ok(mut kept)) = (held, self.jbig2_globals.lock()) { + let total = kept.values().map(|g| g.len()).fold(data.len(), usize::saturating_add); + if total <= JBIG2_GLOBALS_HELD { + kept.insert(num, data.clone()); + } + } + Ok(Some(data)) } pub fn try_get(&self, num: u32) -> Result, CosError> { diff --git a/crates/cos/src/document/open.rs b/crates/cos/src/document/open.rs index 57d697ed66..395424b771 100644 --- a/crates/cos/src/document/open.rs +++ b/crates/cos/src/document/open.rs @@ -19,6 +19,7 @@ impl Document { repair_log: Arc::new(Vec::new()), cache: Arc::default(), objstms: Arc::default(), + jbig2_globals: Arc::default(), stream_limit: crate::object::MAX_DECODED, overlay: BTreeMap::new(), next_num: 1, @@ -102,6 +103,7 @@ impl Document { repair_log: Arc::new(Vec::new()), cache: Arc::default(), objstms: Arc::default(), + jbig2_globals: Arc::default(), stream_limit: stream_limit.min(crate::object::MAX_DECODED), overlay: BTreeMap::new(), next_num: 1, diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index 7110fa6b72..cca4b87326 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -173,6 +173,159 @@ fn fax_images_decode_with_their_parameters() { assert!(behind(&[Object::Int(5), fax], &[null, own], coded.clone()).is_err(), "a /Filter entry that isn't a name"); } +/// Images that share a `/JBIG2Globals` stream of the file decode it once a document: it is kept +/// for the next, in every clone. A globals stream edited since is read afresh (the kept one is the +/// file's), and objects added in memory aren't kept. +#[test] +fn shared_jbig2_globals_are_read_once() { + let bits = [0b1111_1000, 0, 0b1111_1000, 0, 0b1111_1000, 0]; + let coded = pdfcraft_filters::encode_jbig2_generic(&bits, 10, 3).unwrap(); + let mut doc = Document::new_empty(); + let mut d = Dict::new(); + d.set(b"Filter".to_vec(), Object::name("FlateDecode")); + let skipped = [0, 0, 0, 0, 0x3E, 0, 0, 0, 0, 0, 4, 0, 0, 0, 1]; + let globals = doc.add(Object::Stream(Stream::from_raw(d, pdfcraft_filters::encode_flate(&skipped)))); + let image = |doc: &mut Document| { + let mut d = Dict::new(); + d.set(b"Width".to_vec(), Object::Int(10)); + d.set(b"Height".to_vec(), Object::Int(3)); + d.set(b"Filter".to_vec(), Object::name("JBIG2Decode")); + let mut parms = Dict::new(); + parms.set(b"JBIG2Globals".to_vec(), Object::Ref(globals)); + d.set(b"DecodeParms".to_vec(), Object::Dict(parms)); + doc.add(Object::Stream(Stream::from_raw(d, coded.clone()))) + }; + let (a, b) = (image(&mut doc), image(&mut doc)); + let stream = |doc: &Document, r: ObjRef| match &*doc.get(r) { + Object::Stream(s) => s.clone(), + _ => panic!("not a stream"), + }; + assert_eq!(doc.decoded_image(&stream(&doc, a)).unwrap(), bits); + assert!(doc.jbig2_globals.lock().unwrap().is_empty(), "an object added in memory isn't kept"); + + // The same objects read from a file, the catalog holding on to the images (a full save may + // number them anew). + let root = doc.root().unwrap(); + doc.update_dict(root, |c| c.set(b"Images".to_vec(), Object::Array(vec![Object::Ref(a), Object::Ref(b)]))).unwrap(); + let file = crate::write_full(&doc, &crate::SaveOptions::default()).unwrap(); + let mut doc = Document::open(Arc::new(file)).unwrap(); + let images = doc.get(doc.root().unwrap()).as_dict().unwrap().get(b"Images").unwrap().as_array().unwrap().clone(); + let (a, b) = (images[0].as_ref().unwrap(), images[1].as_ref().unwrap()); + let parms = stream(&doc, a).dict.get(b"DecodeParms").unwrap().as_dict().unwrap().clone(); + let globals = parms.reference(b"JBIG2Globals").unwrap(); + let clone = doc.clone(); + assert_eq!(doc.decoded_image(&stream(&doc, a)).unwrap(), bits); + assert_eq!(doc.jbig2_globals.lock().unwrap().len(), 1, "kept"); + let kept = doc.jbig2_globals.lock().unwrap()[&globals.num].clone(); + assert_eq!(clone.decoded_image(&stream(&clone, b)).unwrap(), bits, "the clone's other image"); + let shared = clone.jbig2_globals.lock().unwrap()[&globals.num].clone(); + assert!(Arc::ptr_eq(&kept, &shared), "the clone uses the one kept, not a second decode"); + doc.set(globals, Object::Stream(Stream::from_raw(Dict::new(), b"not JBIG2".to_vec()))); + assert!(doc.decoded_image(&stream(&doc, b)).is_err(), "the edited globals are read afresh"); + assert_eq!(clone.decoded_image(&stream(&clone, b)).unwrap(), bits, "the clone keeps the file's"); +} + +/// A JBIG2 image's size is read as given directly, as every caller reads it first, and an image +/// with neither is an error, not an image of no rows. +#[test] +fn a_jbig2_image_needs_its_size_given_directly() { + let coded = pdfcraft_filters::encode_jbig2_generic(&[0xFF, 0xFF], 8, 2).unwrap(); + let mut doc = Document::new_empty(); + let size = doc.add(Object::Int(2)); + let image = |width: Object, height: Object| { + let mut d = Dict::new(); + d.set(b"Width".to_vec(), width); + d.set(b"Height".to_vec(), height); + d.set(b"Filter".to_vec(), Object::name("JBIG2Decode")); + doc.decoded_image(&Stream::from_raw(d, coded.clone())) + }; + assert!(image(Object::Int(8), Object::Int(2)).is_ok()); + assert!(image(Object::Ref(size), Object::Int(2)).is_err(), "/Width by reference"); + assert!(image(Object::Int(8), Object::Ref(size)).is_err(), "/Height by reference"); +} + +/// A document that kept globals of 64 MiB keeps no more, and the images past that still read: +/// three streams of 22 MiB, shared by three images, are two kept and one decoded each time. +#[test] +fn kept_jbig2_globals_are_bounded_in_total() { + let bits = [0b1111_1000, 0, 0b1111_1000, 0, 0b1111_1000, 0]; + let coded = pdfcraft_filters::encode_jbig2_generic(&bits, 10, 3).unwrap(); + let length = 22 * 1024 * 1024; + let mut skipped = vec![0, 0, 0, 0, 0x3E, 0, 0]; + skipped.extend_from_slice(&u32::try_from(4 + length).unwrap().to_be_bytes()); + skipped.extend_from_slice(&[0, 0, 0, 1]); + skipped.resize(skipped.len() + length, 0); + let mut doc = Document::new_empty(); + let mut images = Vec::new(); + for _ in 0..3 { + let mut d = Dict::new(); + d.set(b"Filter".to_vec(), Object::name("FlateDecode")); + let globals = doc.add(Object::Stream(Stream::from_raw(d, pdfcraft_filters::encode_flate(&skipped)))); + let mut d = Dict::new(); + d.set(b"Width".to_vec(), Object::Int(10)); + d.set(b"Height".to_vec(), Object::Int(3)); + d.set(b"Filter".to_vec(), Object::name("JBIG2Decode")); + let mut parms = Dict::new(); + parms.set(b"JBIG2Globals".to_vec(), Object::Ref(globals)); + d.set(b"DecodeParms".to_vec(), Object::Dict(parms)); + images.push(Object::Ref(doc.add(Object::Stream(Stream::from_raw(d, coded.clone()))))); + } + let root = doc.root().unwrap(); + doc.update_dict(root, |c| c.set(b"Images".to_vec(), Object::Array(images))).unwrap(); + let doc = Document::open(Arc::new(crate::write_full(&doc, &crate::SaveOptions::default()).unwrap())).unwrap(); + let images = doc.get(doc.root().unwrap()).as_dict().unwrap().get(b"Images").unwrap().as_array().unwrap().clone(); + for image in &images { + let Object::Stream(s) = &*doc.get(image.as_ref().unwrap()) else { panic!("not a stream") }; + assert_eq!(doc.decoded_image(s).unwrap(), bits, "every image reads"); + } + let kept = doc.jbig2_globals.lock().unwrap(); + assert_eq!(kept.len(), 2, "two fit in 64 MiB, a third doesn't"); + assert!(kept.values().all(|g| g.capacity() == g.len()), "what is kept holds no more than it counts"); +} + +/// Globals whose `/DecodeParms` are given by reference aren't kept: that's another object, so +/// editing it changes what they decode to while the stream itself, the key kept by, stays the same. +#[test] +fn jbig2_globals_with_parameters_by_reference_are_read_afresh() { + let bits = [0b1111_1000, 0, 0b1111_1000, 0, 0b1111_1000, 0]; + let coded = pdfcraft_filters::encode_jbig2_generic(&bits, 10, 3).unwrap(); + let skipped = [0, 0, 0, 0, 0x3E, 0, 0, 0, 0, 0, 4, 0, 0, 0, 1]; + let params = pdfcraft_filters::Params { predictor: 12, colors: 1, bits_per_component: 8, columns: 15, early_change: 1 }; + let mut doc = Document::new_empty(); + let mut parms = Dict::new(); + parms.set(b"Predictor".to_vec(), Object::Int(12)); + parms.set(b"Columns".to_vec(), Object::Int(15)); + let parms = doc.add(Object::Dict(parms)); + let mut d = Dict::new(); + d.set(b"Filter".to_vec(), Object::name("FlateDecode")); + d.set(b"DecodeParms".to_vec(), Object::Ref(parms)); + let predicted = pdfcraft_filters::encode(&pdfcraft_filters::Filter::Flate, ¶ms, &skipped).unwrap(); + let globals = doc.add(Object::Stream(Stream::from_raw(d, predicted))); + let mut d = Dict::new(); + d.set(b"Width".to_vec(), Object::Int(10)); + d.set(b"Height".to_vec(), Object::Int(3)); + d.set(b"Filter".to_vec(), Object::name("JBIG2Decode")); + let mut image_parms = Dict::new(); + image_parms.set(b"JBIG2Globals".to_vec(), Object::Ref(globals)); + d.set(b"DecodeParms".to_vec(), Object::Dict(image_parms)); + let image = doc.add(Object::Stream(Stream::from_raw(d, coded))); + let root = doc.root().unwrap(); + doc.update_dict(root, |c| c.set(b"Image".to_vec(), Object::Ref(image))).unwrap(); + let mut doc = Document::open(Arc::new(crate::write_full(&doc, &crate::SaveOptions::default()).unwrap())).unwrap(); + let image = doc.get(doc.root().unwrap()).as_dict().unwrap().reference(b"Image").unwrap(); + let stream = |doc: &Document| match &*doc.get(image) { + Object::Stream(s) => s.clone(), + _ => panic!("not a stream"), + }; + assert_eq!(doc.decoded_image(&stream(&doc)).unwrap(), bits); + assert!(doc.jbig2_globals.lock().unwrap().is_empty(), "not kept"); + let globals = stream(&doc).dict.get(b"DecodeParms").unwrap().as_dict().unwrap().reference(b"JBIG2Globals").unwrap(); + let Object::Stream(g) = &*doc.get(globals) else { panic!("not a stream") }; + let parms = g.dict.reference(b"DecodeParms").unwrap(); + doc.set(parms, Object::Dict(Dict::new())); + assert!(doc.decoded_image(&stream(&doc)).is_err(), "without the predictor they read otherwise"); +} + /// A JBIG2 image reads through its decoder, behind other filters too, with the `/JBIG2Globals` of /// its own `/DecodeParms` (a dictionary or its place in an array, /Crypt counting, by reference /// too): globals that can't be read are an error, and ones at another filter's place aren't its diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 6dae6adf0a..fdee0bcf82 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -58,3 +58,5 @@ getrandom = { version = "0.4", features = ["wasm_js"] } [dev-dependencies] pdfcraft-testkit = { workspace = true } +# JBIG2 fixtures (`encode_jbig2_generic`). +pdfcraft-filters = { workspace = true } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 833fa16b9f..2cd226ee1e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -8079,6 +8079,105 @@ fn copy_image_finds_a_picture_inside_form_artwork_under_text() { assert!(d.page_image_copy_file(0, 1).is_err()); } +/// Copy Image reads a JBIG2 image's `/JBIG2Globals`: here they hold the page information, and the +/// image's own stream only an extension segment decoders skip, so it has a picture only through +/// them. Its default pixel is black. +#[test] +fn copy_image_reads_the_globals_of_a_jbig2_picture() { + let mut globals = vec![0, 0, 0, 0, 48, 0, 1, 0, 0, 0, 19]; + globals.extend_from_slice(&16u32.to_be_bytes()); + globals.extend_from_slice(&2u32.to_be_bytes()); + globals.extend_from_slice(&[0, 0, 0, 0, 0, 0, 0, 0, 4, 0, 0]); // resolution, flags, striping + let image = [0, 0, 0, 1, 62, 0, 1, 0, 0, 0, 0]; + let stream = |dict: &str, data: &[u8]| { + let mut o = format!("<< {dict} /Length {} >>\nstream\n", data.len()).into_bytes(); + o.extend_from_slice(data); + o.extend_from_slice(b"\nendstream"); + o + }; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /XObject << /Im1 5 0 R >> >> >>".to_vec(), + stream("", b"q 160 0 0 20 20 100 cm /Im1 Do Q"), + stream( + "/Type /XObject /Subtype /Image /Width 16 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 1 /Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 6 0 R >>", + &image, + ), + stream("", &globals), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let mut s = Session::new(); + let id = s.open("scan.pdf", None, Arc::new(out), None).unwrap(); + let d = s.get(id).unwrap(); + let (w, h, rgba) = d.page_image_rgba(0, 0).unwrap(); + assert_eq!((w, h), (16, 2)); + assert!(rgba.chunks(4).all(|p| p == [0, 0, 0, 255]), "the page's default pixel, from the globals"); +} + +/// Copy Image takes a JBIG2 picture as its pixels and as a PNG file, behind another filter +/// (ASCIIHexDecode) too: Export's decoding, through `Document::decoded_image`. +#[test] +fn copy_image_copies_a_jbig2_picture_behind_another_filter() { + // 16 × 2: the left half of the top row black, the rest white. + let bits = [0x00, 0xFF, 0xFF, 0xFF]; + let coded = pdfcraft_filters::encode_jbig2_generic(&bits, 16, 2).unwrap(); + let hex: Vec = coded.iter().flat_map(|b| format!("{b:02x}").into_bytes()).chain(*b">").collect(); + let stream = |dict: &str, data: &[u8]| { + let mut o = format!("<< {dict} /Length {} >>\nstream\n", data.len()).into_bytes(); + o.extend_from_slice(data); + o.extend_from_slice(b"\nendstream"); + o + }; + let objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /XObject << /Im1 5 0 R >> >> >>".to_vec(), + stream("", b"q 160 0 0 20 20 100 cm /Im1 Do Q"), + stream( + "/Type /XObject /Subtype /Image /Width 16 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 1 /Filter [/ASCIIHexDecode /JBIG2Decode]", + &hex, + ), + ]; + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n", i + 1).as_bytes()); + out.extend_from_slice(o); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let mut s = Session::new(); + let id = s.open("scan.pdf", None, Arc::new(out), None).unwrap(); + let d = s.get(id).unwrap(); + assert_eq!(d.page_image_at(0, [30.0, 115.0]), Some(0)); + assert_eq!(d.page_image_copy_file(0, 0).unwrap().0, "png"); + let (w, h, rgba) = d.page_image_rgba(0, 0).unwrap(); + assert_eq!((w, h), (16, 2)); + let grey: Vec = rgba.chunks(4).map(|px| px[0]).collect(); + let expected: Vec = (0..32).map(|i| if i < 8 { 0 } else { 255 }).collect(); + assert_eq!(grey, expected, "the picture, black as 0"); +} + /// `fixture(2)` with the catalog pointing at a page tree object the file does not have (#1656). fn fixture_without_page_tree() -> Vec { let bytes = fixture(2); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 4878eac627..2187f33c81 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -130,7 +130,7 @@ milestone = "M1" status = "partial" commands = [] tools = [] -evidence = ["crates/filters/src/ccitt.rs::decoding_gives_the_rows_back", "crates/filters/src/ccitt.rs::group_3_rows_on_bytes_read_by_the_codes_in_the_data", "crates/cos/src/document/tests.rs::fax_images_decode_with_their_parameters", "crates/filters/src/jbig2/decode.rs::both_codings_decode_to_their_samples", "crates/filters/src/jbig2/decode.rs::what_it_cannot_decode_is_an_error", "crates/cos/src/document/tests.rs::jbig2_images_decode_with_their_globals"] +evidence = ["crates/filters/src/ccitt.rs::decoding_gives_the_rows_back", "crates/filters/src/ccitt.rs::group_3_rows_on_bytes_read_by_the_codes_in_the_data", "crates/cos/src/document/tests.rs::fax_images_decode_with_their_parameters", "crates/filters/src/jbig2/decode.rs::both_codings_decode_to_their_samples", "crates/filters/src/jbig2/decode.rs::what_it_cannot_decode_is_an_error", "crates/cos/src/document/tests.rs::jbig2_images_decode_with_their_globals", "crates/cos/src/document/tests.rs::shared_jbig2_globals_are_read_once"] notes = "decoded through the hayro bootstrap for display; own codecs are M1.1/M1.2. CCITT fax decoding is in filters (decode_ccitt, through hayro-ccitt), which Document::decoded_image uses for Export ▸ Images, the optimizer and redaction; so is JBIG2 decoding (decode_jbig2, through hayro-jbig2, the page bounded before it is decoded), which it uses with the image's /JBIG2Globals (within 32 MiB) for Export ▸ Images, Copy Image and redaction. Byte-aligned Group 3 is read by its end-of-line codes when the data starts with one and on bytes otherwise; still limited: data with codes but none before the first row (not as T.4 sends them) is read on bytes, as the renderer draws it, and at /K 0 may come out as other rows, which the optimizer then writes back. The bootstrap renderer draws byte-aligned Group 3 whose fill ends each end-of-line code on a byte as other rows (black box, at every /K), which decode_ccitt reads right; redaction removes Group 3 on bytes whole" [[feature]] @@ -1598,7 +1598,7 @@ milestone = "M3" status = "partial" commands = [] tools = [] -evidence = ["crates/ui-egui/tests/it/viewer.rs::a_clicked_picture_is_selected_and_copied_in_its_original_format", "crates/ui-egui/tests/it/viewer.rs::right_click_on_a_picture_copies_it_at_full_size", "crates/ui-egui/tests/it/viewer.rs::a_picture_inside_form_artwork_under_text_is_selected_and_copied", "crates/engine/src/tests.rs::copy_image_finds_a_picture_inside_form_artwork_under_text"] +evidence = ["crates/ui-egui/tests/it/viewer.rs::a_clicked_picture_is_selected_and_copied_in_its_original_format", "crates/ui-egui/tests/it/viewer.rs::right_click_on_a_picture_copies_it_at_full_size", "crates/ui-egui/tests/it/viewer.rs::a_picture_inside_form_artwork_under_text_is_selected_and_copied", "crates/engine/src/tests.rs::copy_image_finds_a_picture_inside_form_artwork_under_text", "crates/engine/src/tests.rs::copy_image_copies_a_jbig2_picture_behind_another_filter", "crates/engine/src/tests.rs::copy_image_reads_the_globals_of_a_jbig2_picture"] notes = "Select tool: a click selects a raster image (also inside Form artwork or under text) and ⌘C copies it as a file in its original format (JPEG as stored, else PNG); right-click ▸ Copy Image copies its pixels at full resolution. Missing: inline images (BI/ID/EI), JPEG 2000 images, multi-format clipboard (file and bitmap at once), the web build" [[feature]] From d1677b4b3ff39117405e8fb3f92d7e73a5a15cca Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Sat, 31 Oct 2026 00:20:10 +0100 Subject: [PATCH 2/2] M11: an indexed image also drawn where the optimizer doesn't read keeps its resolution From CodeRabbit's review: default colour spaces apply to an indexed colour space's device base, so one in force where an annotation's appearance (or a pattern, or another image's /Alternates) draws the image may change which palette entry is darker, and the tie a resampled image keeps. Only black-and-white RGB went through used_elsewhere; indexed images now do too when downsampling is on, and one used beyond the page and form resources the walk read keeps its resolution, as such RGB keeps its colour space. A grey image stays out of this check, since sample 0 stays dark in a grey space, and #5426's rule for grey is unchanged: the test now has a grey image drawn on the page and in an annotation. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/optimize/README.md | 3 ++- crates/optimize/src/images.rs | 18 ++++++++++++---- crates/optimize/tests/optimize.rs | 35 +++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/optimize/README.md b/crates/optimize/README.md index b8346a626a..184053c797 100644 --- a/crates/optimize/README.md +++ b/crates/optimize/README.md @@ -65,7 +65,8 @@ let report = optimize_with_progress(&mut doc, &settings, &mut |stage| keep_going RGB packed to one bit among them), image masks, and indexed images over `DeviceGray` or `DeviceRGB`, whose palette is read (Rec. 601 luma for RGB), with no decode array or one of two different numbers, and not drawn or listed where `/DefaultGray` or `/DefaultRGB` is or may be - in force. Not a separation or a DeviceN, whose tint transform isn't read, calibrated or ICC + in force (an indexed image referred to from beyond the page and form resources read, as an + annotation's appearance, may be). Not a separation or a DeviceN, whose tint transform isn't read, calibrated or ICC grey, or an indexed image over another base or whose two entries look alike. Not one with a soft mask, whose own size would have to follow; not a halftone or a dither, most of whose new pixels would cover both black and white, so that taking the sample covering more would turn its tones solid (a 50% screen black, a 25% diff --git a/crates/optimize/src/images.rs b/crates/optimize/src/images.rs index 2929b98845..843486f917 100644 --- a/crates/optimize/src/images.rs +++ b/crates/optimize/src/images.rs @@ -300,6 +300,14 @@ fn device_rgb(doc: &Document, r: ObjRef) -> bool { && cs.as_deref().and_then(Object::as_name).is_some_and(|n| matches!(n, b"DeviceRGB" | b"RGB")) } +/// Whether image `r` is indexed: its palette says which of its samples is dark ([`dark_is_one`]), +/// which default colour spaces in force where it is drawn may change. +fn indexed(doc: &Document, r: ObjRef) -> bool { + let Object::Stream(s) = &*doc.get(r) else { return false }; + let cs = s.dict.get(b"ColorSpace").map(|cs| doc.resolve(cs)); + matches!(cs.as_deref(), Some(Object::Array(a)) if matches!(a.first().and_then(Object::as_name), Some(b"Indexed" | b"I"))) +} + /// Call `f` with every reference in `o`, a dictionary's or array's included, a stream's dictionary /// too, but not those in objects referred to. fn each_ref(o: &Object, mut f: impl FnMut(ObjRef)) { @@ -1144,11 +1152,13 @@ pub(crate) fn run( progress: &mut dyn FnMut(Stage) -> bool, ) -> Result<(), OptimizeError> { let (ppi, mut defaults, read) = walk_pages(doc, pages); - // Black-and-white RGB becomes grey, which default colour spaces would draw otherwise: one used - // where the walk didn't look may be drawn so. + // Black-and-white RGB becomes grey, which default colour spaces would draw otherwise, and an + // indexed image's palette says which of its samples a resampled one keeps, which they may + // change: one used where the walk didn't look may be drawn so. let packs = settings.mono.compression != MonoCompression::Retain; - let rgb: HashSet = ppi.keys().filter(|r| packs && device_rgb(doc, **r)).map(|r| r.num).collect(); - defaults.extend(used_elsewhere(doc, &rgb, &read)); + let checked: HashSet = + ppi.keys().filter(|r| (packs && device_rgb(doc, **r)) || (settings.mono.downsample && indexed(doc, **r))).map(|r| r.num).collect(); + defaults.extend(used_elsewhere(doc, &checked, &read)); // Soft masks are processed with their image, not on their own. let masks: HashSet = ppi .keys() diff --git a/crates/optimize/tests/optimize.rs b/crates/optimize/tests/optimize.rs index 92dc2e2d86..3fe59a1325 100644 --- a/crates/optimize/tests/optimize.rs +++ b/crates/optimize/tests/optimize.rs @@ -3504,6 +3504,41 @@ fn indexed_thin_rules_survive_averaging_for_both_palette_orders_and_decodes() { } } +/// An indexed image also drawn where the optimizer doesn't read (an annotation's appearance, where +/// other default colour spaces may be in force and change which palette entry is darker) keeps its +/// resolution, as black-and-white RGB keeps its colour space; drawn only on the page, it is +/// averaged. A grey image is averaged either way: sample 0 stays dark in a grey space, and only an +/// indexed image's palette depends on what's in force where it is drawn. +#[test] +fn an_indexed_image_used_beyond_the_page_content_read_is_not_averaged() { + let (w, h) = (1200u32, 480u32); + let stride = (w as usize).div_ceil(8); + let mut data = vec![0xFF; stride * h as usize]; + for y in 0..h as usize { + data[y * stride + 101 / 8] &= !(0x80 >> (101 % 8)); + } + let palette = Object::String(pdfcraft_cos::PdfString::literal(vec![0, 0, 0, 255, 255, 255])); + let indexed = Object::Array(vec![Object::name("Indexed"), Object::name("DeviceRGB"), 1.into(), palette]); + let cases = [(&indexed, false, 600), (&indexed, true, 1200), (&Object::name("DeviceGray"), false, 600), (&Object::name("DeviceGray"), true, 600)]; + for (cs, in_annotation, width) in cases { + let mut doc = Document::new_empty(); + let image = mono_image(&mut doc, w, h, &data, None, &[("BitsPerComponent", 1.into()), ("ColorSpace", cs.clone())]); + // 144 × 57.6 pt is 600 ppi. + let p = page(&mut doc, &[("Im", image)], "q 144 0 0 57.6 20 600 cm /Im Do Q"); + if in_annotation { + let resources = Object::Dict({ + let mut r = Dict::new(); + r.set(b"XObject".to_vec(), Object::Dict(xobject_dict(&[("Im", image)]))); + r + }); + let appearance = drawing(&mut doc, Some(resources), &form_entries()); + annotate(&mut doc, p, appearance); + } + optimize(&mut doc, &Settings::default()).unwrap(); + assert_eq!(stream(&doc, image).dict.int(b"Width"), Some(width), "{cs:?}, in an annotation: {in_annotation}"); + } +} + /// White margins must not hide a small screened photograph from the tone guard. #[test] fn mixed_text_and_small_halftone_regions_keep_their_tones_with_wide_white_margins() {