From 580abc51cfdc288313bb4d2e7a3218cf4d9a869a Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Sun, 1 Nov 2026 23:38:45 +0100 Subject: [PATCH 01/27] M8: a shared JBIG2 dictionary leaves with whatever used it, by any route Redaction starts from the images a run replaces or removes and the forms and soft masks under them, so a dictionary used by something that left another way stays, with apply returning Ok: a stamp or widget appearance, a Type 3 glyph, an /Alternates entry, a /Mask, or an image with /Filter by reference or /DecodeParms through a reference to a reference. A byte-identical copy stays too, which is what pdfseparate then pdfunite leaves, and Remove Hidden Information never takes a dictionary out. Compare what a full save would keep before and after the run instead, the same walk write_full makes, now cos::reachable. What left names the dictionaries, in any entry of its parameters; what stays that names one, or a copy, is stored again without it; and the walk is taken once more, so a dictionary still kept refuses the run, whatever keeps it. An image with /Width, /Height or /Filter by reference is read, and is one bit in the copy, as it was when drawn. /Alternates stays with what is stored again, and a stream that isn't JBIG2 refuses rather than being stored. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/cos/src/document/objects.rs | 11 - crates/cos/src/document/tests.rs | 21 ++ crates/cos/src/lib.rs | 2 +- crates/cos/src/writer.rs | 74 +++-- crates/redact/README.md | 14 +- crates/redact/src/globals.rs | 182 +++++++++++ crates/redact/src/image.rs | 41 +-- crates/redact/src/lib.rs | 77 +---- crates/redact/src/sanitize.rs | 2 + crates/redact/src/tests.rs | 464 +++++++++++++++++++++++++++++ parity/acrobat-features.toml | 4 +- 11 files changed, 738 insertions(+), 154 deletions(-) create mode 100644 crates/redact/src/globals.rs diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index bab99ac45e..39a4d9a6da 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -326,17 +326,6 @@ impl Document { pdfcraft_filters::decode_ccitt(&data, &p, rows).map_err(filter) } - /// The `/JBIG2Globals` stream a JBIG2 image `s` names by reference in its last filter's - /// `/DecodeParms`: `None` for a stream whose last filter isn't `JBIG2Decode`, or one without - /// globals. Redaction finds with it the other images that share a redacted image's globals. - pub fn jbig2_globals_ref(&self, s: &Stream) -> Option { - let chain = s.readable_filters().ok()?; - if !matches!(chain.last(), Some((pdfcraft_filters::Filter::Jbig2, _))) { - return None; - } - self.last_filter_parms(s).get(b"JBIG2Globals").and_then(Object::as_ref) - } - /// What the `/JBIG2Globals` stream of JBIG2 image `s` (in its last filter's `/DecodeParms`) /// 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 diff --git a/crates/cos/src/document/tests.rs b/crates/cos/src/document/tests.rs index cca4b87326..a443f0f9f0 100644 --- a/crates/cos/src/document/tests.rs +++ b/crates/cos/src/document/tests.rs @@ -1,5 +1,26 @@ use super::*; +/// What [`crate::reachable`] gives is what a full save keeps: not an object nothing refers to, nor one +/// a reference only dangles at, and a ring of references ends. +#[test] +fn what_is_reachable_is_what_a_full_save_keeps() { + let mut doc = Document::new_empty(); + let root = doc.root().unwrap(); + let kept = doc.add(Object::Int(7)); + let dangling = ObjRef::new(4_000, 0); + let list = doc.add(Object::Null); + let ring = doc.add(Object::Array(vec![Object::Ref(list)])); + doc.set(list, Object::Array(vec![Object::Ref(kept), Object::Ref(ring), Object::Ref(dangling)])); + let orphan = doc.add(Object::Int(8)); + doc.update_dict(root, |d| d.set(b"Held".to_vec(), Object::Ref(list))).unwrap(); + let found = crate::reachable(&doc); + assert!([root, kept, list, ring].iter().all(|r| found.contains(r))); + assert!(!found.contains(&orphan) && !found.contains(&dangling)); + let saved = Document::open(Arc::new(crate::write_full(&doc, &crate::SaveOptions::default()).unwrap())).unwrap(); + let holds = |n: i64| saved.scan_objects().any(|(_, o)| matches!(*o, Object::Int(v) if v == n)); + assert!(holds(7) && !holds(8)); +} + /// /DecodeParms given by reference, whole, as array entries or as the numbers inside, are read /// once made direct: here a Flate predictor, which the stream alone doesn't see. Only the entries the filters use are /// resolved, and nothing is for a chain longer than a decoder takes. diff --git a/crates/cos/src/lib.rs b/crates/cos/src/lib.rs index a35902a031..76dc0b6fe7 100644 --- a/crates/cos/src/lib.rs +++ b/crates/cos/src/lib.rs @@ -30,7 +30,7 @@ pub use parser::{Lexer, is_whitespace, parse_indirect}; pub use pdfcraft_crypt::{ Algorithm, Auth, Method as CryptMethod, NewEncryption, Permissions, SecurityHandler, effective_password, pdfdoc_byte, pdfdoc_char, }; -pub use writer::{SaveOptions, pdf_date, serialize, write_full, write_incremental}; +pub use writer::{SaveOptions, pdf_date, reachable, serialize, write_full, write_incremental}; #[derive(Debug, thiserror::Error, Clone, PartialEq)] pub enum CosError { diff --git a/crates/cos/src/writer.rs b/crates/cos/src/writer.rs index cd0137f452..e34799d5f9 100644 --- a/crates/cos/src/writer.rs +++ b/crates/cos/src/writer.rs @@ -276,36 +276,9 @@ pub fn write_full(doc: &Document, opts: &SaveOptions) -> Result, CosErro let mut reader = doc.object_reader(); stamp_mod_date(&mut reader.document, opts); let trailer_in = reader.document.trailer().clone(); - // Breadth-first walk from the trailer, assigning new numbers in visit order. - let mut map: HashMap = HashMap::new(); - // Record the type while traversing so classifying packed objects needs no second parse. - let mut order: Vec<(ObjRef, bool)> = Vec::new(); - let mut queue: VecDeque = VecDeque::new(); - let visit_refs = |o: &Object, queue: &mut VecDeque| collect_refs(o, &mut |r| queue.push_back(r)); - for key in [&b"Root"[..], b"Info", b"Encrypt"] { - if let Some(r) = trailer_in.reference(key) { - queue.push_back(r); - } - } - // Unknown trailer entries are document data. Objects they alone reference must be written - // too, or a full rewrite would keep the key and drop its value. - for (key, value) in trailer_in.iter() { - if !trailer_key_rewritten(key) { - collect_refs(value, &mut |r| queue.push_back(r)); - } - } - while let Some(r) = queue.pop_front() { - if map.contains_key(&r) { - continue; - } - let o = reader.get(r); - if matches!(*o, Object::Null) { - continue; // dangling reference: written as null by renumbering below - } - map.insert(r, output_number(order.len())?); - order.push((r, matches!(*o, Object::Stream(_)))); - visit_refs(&o, &mut queue); - } + // New numbers go in visit order. + let order = walk(&mut reader, &trailer_in); + let map: HashMap = order.iter().enumerate().map(|(i, (r, _))| Ok((*r, output_number(i)?))).collect::>()?; let version = reader.document.version().chars().take(3).collect::(); let version = if opts.object_streams && version.as_str() < "1.5" { "1.5".to_string() } else { version }; let mut out = Vec::new(); @@ -486,6 +459,47 @@ pub(crate) fn unlinked_additions(doc: &Document) -> HashSet { pending.into_keys().collect() } +/// The objects a full save keeps: breadth first from `trailer`, each with whether it is a stream, so +/// classifying packed objects needs no second parse. A dangling reference is left out, and written +/// as null by renumbering. +fn walk(reader: &mut ObjectReader, trailer: &Dict) -> Vec<(ObjRef, bool)> { + let mut seen: HashSet = HashSet::new(); + let mut order: Vec<(ObjRef, bool)> = Vec::new(); + let mut queue: VecDeque = VecDeque::new(); + for key in [&b"Root"[..], b"Info", b"Encrypt"] { + if let Some(r) = trailer.reference(key) { + queue.push_back(r); + } + } + // Unknown trailer entries are document data. Objects they alone reference must be written + // too, or a full rewrite would keep the key and drop its value. + for (key, value) in trailer.iter() { + if !trailer_key_rewritten(key) { + collect_refs(value, &mut |r| queue.push_back(r)); + } + } + while let Some(r) = queue.pop_front() { + if seen.contains(&r) { + continue; + } + let o = reader.get(r); + if matches!(*o, Object::Null) { + continue; + } + seen.insert(r); + order.push((r, matches!(*o, Object::Stream(_)))); + collect_refs(&o, &mut |r| queue.push_back(r)); + } + order +} + +/// The objects [`write_full`] would keep of `doc`: anything else leaves the file on a full save. +pub fn reachable(doc: &Document) -> HashSet { + let mut reader = doc.object_reader(); + let trailer = reader.document.trailer().clone(); + walk(&mut reader, &trailer).into_iter().map(|(r, _)| r).collect() +} + fn collect_refs(o: &Object, f: &mut impl FnMut(ObjRef)) { match o { Object::Ref(r) => f(*r), diff --git a/crates/redact/README.md b/crates/redact/README.md index 9c024d1b55..daaacc4442 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -24,10 +24,16 @@ clear_marks(&mut doc, None)?; // remove marks without app is Flate, or for a one-bit image of one component (a scan as a rule) CCITT Group 4, or JBIG2 when it was JBIG2, whichever is smallest as stored, coded from the cleared samples; it refers to no globals. Globals hold the shapes of every image that shares them (a scan's pages, often), - those of marks under the area included, so every other image that uses the globals of a removed - or replaced image is decoded and stored again in place without them, as a copy is (Flate, - Group 4 or JBIG2, whichever is smallest; `Report::images_restored`), and they leave the file on the full rewrite; one that can't be - decoded for certain refuses the redaction (`RedactError::SharedJbig2Globals`). Codecs redaction doesn't decode (DCT, JPX) are + those of marks under the area included, so whatever left the document, by any route, takes its + globals with it on the full rewrite: anything else the save would keep that names them, or a + byte-for-byte copy of them, is stored again without them, as a copy is (Flate, Group 4 or JBIG2, + whichever is smallest; `Report::images_restored`). When one can't be (it isn't JBIG2, it's past + the size redaction clears, or it can't be decoded), or something else refers to them, `apply` + refuses and changes nothing (`RedactError::SharedJbig2Globals`). `remove_hidden` takes them out + the same way. Another dictionary holding the same shapes in other bytes isn't found, nor is a copy + nothing names, and an image that hasn't left keeps its globals with its pixels: one another page + still draws, or one something that doesn't draw it still refers to. Each image is decoded once, + with no total across the document yet. Codecs redaction doesn't decode (DCT, JPX) are removed whole (fail-closed), and so is a JBIG2 image whose page isn't its size (the renderer draws the whole page into the image, so the rows cleared wouldn't be the ones shown) or of more than one bit or component, and Group 3 on bytes (`/K` 0 or more, `/EncodedByteAlign`): PdfCraft's diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs new file mode 100644 index 0000000000..349b9e267a --- /dev/null +++ b/crates/redact/src/globals.rs @@ -0,0 +1,182 @@ +//! A `/JBIG2Globals` stream is a dictionary of symbols that JBIG2 images can share, and scanners +//! and `jbig2enc -s` share one across a scan's pages. It holds the shape of every distinct mark of +//! those pages, including a mark found only under a redaction, so taking the image out isn't enough: +//! while something else uses the dictionary, a full save keeps it. [`unshare`] stores what uses it +//! again without it. + +use std::collections::hash_map::DefaultHasher; +use std::collections::{HashMap, HashSet}; +use std::hash::{Hash, Hasher}; +use std::sync::Arc; + +use pdfcraft_cos::{Document, MAX_JBIG2_GLOBALS, Object, Stream, reachable}; + +/// The references followed from `/JBIG2Globals` towards a stream, so that a ring of them ends. +const MAX_HOPS: usize = 32; + +/// The objects a full save would keep, by number: a reference reads the object of its number, +/// whatever generation it gives. +pub(crate) fn kept(doc: &Document) -> HashSet { + reachable(doc).into_iter().map(|r| r.num).collect() +} + +/// `o`, or what references lead to from it. A ring of them ends as a reference, which is no dictionary. +fn followed(doc: &Document, o: &Object) -> Arc { + let mut at = doc.resolve(o); + for _ in 0..MAX_HOPS { + let next = match &*at { + Object::Ref(next) => *next, + _ => break, + }; + at = doc.get(next); + } + at +} + +/// Every object `s` names as `/JBIG2Globals`, and each reference on the way to one: in any entry of +/// its parameters, whichever filter that entry belongs to and however the filters are given. A +/// reader may use less of this, but leaving a dictionary behind is the failure that matters here. +fn named(doc: &Document, s: &Stream) -> Vec { + let mut found = Vec::new(); + let mut read = |entry: &Object| { + let held = followed(doc, entry); + let Some(mut at) = held.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return }; + for _ in 0..=MAX_HOPS { + found.push(at.num); + match &*doc.get(at) { + Object::Ref(next) => at = *next, + _ => return, + } + } + }; + for key in [b"DecodeParms".as_slice(), b"DP"] { + let Some(parms) = s.dict.get(key).map(|p| followed(doc, p)) else { continue }; + match &*parms { + Object::Array(entries) => entries.iter().for_each(&mut read), + other => read(other), + } + } + found +} + +/// `v`, or what it refers to, inside an array too. +fn direct(doc: &Document, v: &Object) -> Object { + match &*doc.resolve(v) { + Object::Array(a) => Object::Array(a.iter().map(|e| (*doc.resolve(e)).clone()).collect()), + other => other.clone(), + } +} + +/// `s` with what `decoded_image` and `decoded_within` read as given made direct: its size and filters. +fn readable(doc: &Document, s: &Stream) -> Stream { + let mut readable = s.clone(); + for key in [b"Width".as_slice(), b"Height", b"Filter"] { + if let Some(v) = s.dict.get(key) { + readable.dict.set(key.to_vec(), direct(doc, v)); + } + } + readable +} + +/// What dictionary `num` holds, to know a copy of it by: a file put together from pages split apart +/// has one per page, each byte for byte the others. +fn content(doc: &Document, num: u32) -> Option { + let object = doc.try_get(num).ok()?; + let Object::Stream(s) = &*object else { return None }; + let readable = readable(doc, s); + let data = doc.with_direct_parms(&readable).decoded_within(MAX_JBIG2_GLOBALS).unwrap_or_else(|_| s.raw.to_vec()); + let mut hash = DefaultHasher::new(); + data.hash(&mut hash); + Some(hash.finish()) +} + +/// JBIG2 stream `s` stored alone, as a cleared copy is: what it shows unchanged and no dictionary +/// referred to. `None` for a stream that isn't JBIG2, whose decoder took no dictionary and whose +/// samples another filter may give otherwise; for one past the size of an image redaction clears, +/// or known to have more than one component, which JBIG2 is drawn as one of whatever the dictionary +/// says; and for one that can't be decoded. +fn without_dictionary(doc: &Document, s: &Stream) -> Option { + let readable = readable(doc, s); + let jbig2 = match readable.dict.get(b"Filter") { + Some(Object::Array(a)) => a.iter().any(|f| f.as_name() == Some(b"JBIG2Decode".as_slice())), + Some(f) => f.as_name() == Some(b"JBIG2Decode".as_slice()), + None => false, + }; + if !jbig2 || (s.dict.contains(b"ColorSpace") && crate::image::components(doc, s).is_some_and(|n| n > 1)) { + return None; + } + let side = |key: &[u8]| readable.dict.int(key).and_then(|v| usize::try_from(v).ok()); + let (w, h) = (side(b"Width")?, side(b"Height")?); + if (w as u64).saturating_mul(h as u64) > crate::image::MAX_PIXELS { + return None; + } + // As for `clear`: the filters before the JBIG2 decoder may give up to three bytes for each one + // of the raster and a byte a row. + let allowance = w.div_ceil(8).checked_mul(h)?.checked_add(h)?.checked_mul(3)?.checked_add(4096)?; + let data = doc.decoded_image_within(&readable, allowance).ok()?; + let mut dict = readable.dict.clone(); + for key in [b"Length".as_slice(), b"DecodeParms", b"DP"] { + dict.remove(key); + } + dict.set(b"BitsPerComponent".to_vec(), Object::Int(1)); + Some(crate::image::smallest(dict, &data, w, h, true, true)) +} + +/// Takes the `/JBIG2Globals` dictionaries of what left the document since `before` ([`kept`]) out of +/// the file: everything a full save would keep that names one, or a copy of one, is stored again +/// without it, in place, so what refers to that stream stays as it is. Gives the number stored +/// again. A copy is found byte for byte; another dictionary that happens to hold the same shapes isn't. +/// +/// What left, not what a run says it removed: an image goes with a stamp, a widget, a Type 3 glyph, +/// an `/Alternates` entry or a form that can't be read, none of which it was drawn through directly. +/// An image another page still draws hasn't left: it keeps its pixels, so its dictionary keeps nothing +/// more. +/// +/// Errors: [`RedactError::SharedJbig2Globals`](crate::RedactError::SharedJbig2Globals) when a dictionary would +/// still be kept, as what names it can't be stored without it or something else refers to it. Such +/// an image may still be drawn, on a page no mark reached, so it isn't removed, and `doc` is part +/// changed by then: a caller works on a copy it can drop. +pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result { + let now = kept(doc); + let mut dictionaries: HashSet = HashSet::new(); + for &num in before.difference(&now) { + if let Ok(object) = doc.try_get(num) + && let Object::Stream(s) = &*object + { + dictionaries.extend(named(doc, s)); + } + } + if dictionaries.is_empty() { + return Ok(0); + } + let contents: HashSet = dictionaries.iter().filter_map(|n| content(doc, *n)).collect(); + let mut read: HashMap> = HashMap::new(); + let mut users = Vec::new(); + for (r, object) in doc.scan_objects() { + let Object::Stream(s) = &*object else { continue }; + if !now.contains(&r.num) { + continue; + } + let names = named(doc, s); + let copy = + |n: &u32, read: &mut HashMap>| read.entry(*n).or_insert_with(|| content(doc, *n)).is_some_and(|c| contents.contains(&c)); + if names.iter().any(|n| dictionaries.contains(n) || copy(n, &mut read)) { + dictionaries.extend(names); + users.push(r); + } + } + let mut stuck = 0; + for &r in &users { + let object = doc.get(r); + let Object::Stream(s) = &*object else { continue }; + match without_dictionary(doc, s) { + Some(stored) => doc.set(r, Object::Stream(stored)), + None => stuck += 1, + } + } + let after = kept(doc); + if dictionaries.iter().any(|n| after.contains(n)) { + return Err(crate::RedactError::SharedJbig2Globals(stuck.max(1))); + } + Ok(users.len()) +} diff --git a/crates/redact/src/image.rs b/crates/redact/src/image.rs index a96f4715e6..39308839b9 100644 --- a/crates/redact/src/image.rs +++ b/crates/redact/src/image.rs @@ -11,9 +11,9 @@ use pdfcraft_content::{Matrix, overlaps}; use pdfcraft_cos::{Dict, Document, Object, Stream}; /// Most pixels an image may have to be cleared in place (larger ones are removed). -const MAX_PIXELS: u64 = 64 << 20; +pub(crate) const MAX_PIXELS: u64 = 64 << 20; -fn components(doc: &Document, s: &Stream) -> Option { +pub(crate) fn components(doc: &Document, s: &Stream) -> Option { let cs = doc.resolve(s.dict.get(b"ColorSpace")?); let cs = &*cs; let name = match cs { @@ -175,7 +175,7 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] /// is), whichever is smallest as stored, dictionary included (a tiny image can lose by Group 4's /// parameters). Both decode to `data`, cleared before it is coded, so nothing of the original's /// code is carried over; Group 4 or JBIG2 that can't code the rows (too wide) is left out. -fn smallest(dict: Dict, data: &[u8], w: usize, h: usize, bilevel: bool, jbig2: bool) -> Stream { +pub(crate) fn smallest(dict: Dict, data: &[u8], w: usize, h: usize, bilevel: bool, jbig2: bool) -> Stream { let flate = Stream::flate(dict.clone(), data); let Some((columns, rows)) = u32::try_from(w).ok().zip(u32::try_from(h).ok()).filter(|_| bilevel) else { return flate }; let g4 = pdfcraft_filters::encode_g4(data, columns, rows).ok().map(|coded| { @@ -202,38 +202,3 @@ fn stored_len(s: &Stream) -> usize { pdfcraft_cos::serialize(&Object::Dict(s.dict.clone()), &mut dict); dict.len().saturating_add(s.raw.len()) } - -/// JBIG2 image `s` stored again without its `/JBIG2Globals`: decoded through them, as it is drawn, -/// and written as the same samples, Flate, Group 4 or JBIG2 coded alone, whichever is smallest -/// (`smallest`), its other entries kept. `None` when it can't -/// be decoded for certain (too large, data that doesn't decode, a JBIG2 page of another size than -/// the image, or not a one-bit image of one component). Redaction does this to every image that -/// shares a redacted image's globals, since their symbol dictionary holds the shapes of marks drawn -/// under the area too. -pub(crate) fn without_globals(doc: &Document, s: &Stream) -> Option { - let (w, h) = (s.dict.int(b"Width")?, s.dict.int(b"Height")?); - if w <= 0 || h <= 0 || (w as u64) * (h as u64) > MAX_PIXELS { - return None; - } - let (w, h) = (w as usize, h as usize); - let mask = matches!(s.dict.get(b"ImageMask"), Some(Object::Bool(true))); - // JBIG2 codes one-bit images of one component (ISO 32000-2 §7.4.7). - if !mask && (components(doc, s)?, s.dict.int(b"BitsPerComponent")?) != (1, 1) { - return None; - } - let bytes = w.div_ceil(8).checked_mul(h)?; - // As for `clear`: the filters before the JBIG2 decoder may give up to three bytes for each one - // of the raster and a byte a row. - let allowance = bytes.checked_add(h)?.checked_mul(3)?.checked_add(4096)?; - let mut data = doc.decoded_image_within(s, allowance).ok()?; - if data.len() < bytes { - return None; - } - data.truncate(bytes); - let mut dict = s.dict.clone(); - dict.remove(b"Length"); - // The globals are named in the parameters; the copy is coded alone, as `clear`'s are. - dict.remove(b"DecodeParms"); - dict.remove(b"Alternates"); - Some(smallest(dict, &data, w, h, true, true)) -} diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index fad55c431f..4b68fc2e73 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -19,6 +19,7 @@ use std::collections::{HashMap, HashSet}; use pdfcraft_cos::{Dict, Document, ObjRef, Object, Stream}; pub mod codes; +mod globals; mod image; mod interp; mod overlay; @@ -70,10 +71,12 @@ pub enum RedactError { /// could not be shown complete. #[error("the document's objects nest too deeply to remove hidden information completely; the operation was refused")] TooDeep, - /// A JBIG2 image shares its `/JBIG2Globals` with a redacted one and can't be stored again - /// without them, so the shapes they hold, those of marks under the area included, would stay. + /// `/JBIG2Globals` used by something removed would stay: a JBIG2 image shares them and can't be + /// stored again without them, or something else refers to them. Either way the shapes they + /// hold, those of marks under the area included, would stay. Counts the images, or 1 for another + /// reference. #[error( - "{0} JBIG2 image(s) share their symbol data with a redacted image and can't be decoded to be stored without it, so that data, which can show what is under the marks, would stay in the file; nothing was redacted" + "{0} item(s) share JBIG2 symbol data with something being removed and can't be stored without it, so that data, which can show what was removed, would stay in the file; nothing was changed" )] SharedJbig2Globals(usize), #[error(transparent)] @@ -139,7 +142,7 @@ pub struct Report { pub images_removed: usize, pub images_cleared: usize, /// JBIG2 images, anywhere in the document, stored again (as a cleared copy is) without the - /// `/JBIG2Globals` they shared with a redacted image, so those globals leave the file. + /// `/JBIG2Globals` they shared with something this removed, so those globals leave the file. pub images_restored: usize, pub paths_removed: usize, pub paths_clipped: usize, @@ -405,6 +408,7 @@ fn apply_to(doc: &mut Document, pages: Option<&[usize]>) -> Result) -> Result) -> Result Result { - let mut globals = HashSet::new(); - let mut seen = HashSet::new(); - let mut stack: Vec = doomed.to_vec(); - while let Some(r) = stack.pop() { - if !seen.insert(r) || seen.len() > MAX_JBIG2_WALK { - continue; - } - let object = doc.get(r); - let Object::Stream(s) = &*object else { continue }; - if let Some(g) = doc.jbig2_globals_ref(s) { - globals.insert(g); - } - // A form's images, and its forms' (an image's /SMask is a stream of its own, found below). - let resources = s.dict.get(b"Resources").map(|r| doc.resolve(r)); - let xobjects = resources.as_deref().and_then(Object::as_dict).and_then(|d| d.get(b"XObject")).map(|x| doc.resolve(x)); - if let Some(x) = xobjects.as_deref().and_then(Object::as_dict) { - stack.extend(x.iter().filter_map(|(_, v)| v.as_ref())); - } - if let Some(m) = s.dict.get(b"SMask").and_then(Object::as_ref) { - stack.push(m); - } - } - if globals.is_empty() { - return Ok(0); - } - let sharing: Vec = doc - .scan_objects() - .filter(|(_, o)| matches!(&**o, Object::Stream(s) if doc.jbig2_globals_ref(s).is_some_and(|g| globals.contains(&g)))) - .map(|(r, _)| r) - .collect(); - let doomed: HashSet = doomed.iter().copied().collect(); - let (mut restored, mut stuck) = (0, 0); - for r in sharing { - let object = doc.get(r); - let Object::Stream(s) = &*object else { continue }; - match image::without_globals(doc, s) { - Some(new) => { - doc.set(r, Object::Stream(new)); - restored += usize::from(!doomed.contains(&r)); - } - None if doomed.contains(&r) => {} - None => stuck += 1, - } - } - if stuck > 0 { - return Err(RedactError::SharedJbig2Globals(stuck)); - } - Ok(restored) -} - /// What applying the marks collects across pages. struct Applying { report: Report, diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index 98703dfbf2..5052df340a 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -521,6 +521,7 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result Result, + symbols: Vec, + page: Vec, +} + +fn shared_scan() -> SharedScan { + const LETTERS: [[&str; 7]; 3] = [ + ["#####", "#....", "####.", "#....", "#....", "#....", "#####"], + [".###.", "#...#", "#....", "#....", "#....", "#...#", ".###."], + ["#...#", "#...#", "#####", "#...#", "#...#", "#...#", "#...#"], + ]; + let (w, h) = (400usize, 40usize); + let stride = w / 8; + let mut bits = vec![0xFFu8; stride * h]; + for (line, top) in [2usize, 12, 22, 32].into_iter().enumerate() { + for (k, x) in (3..w - 6).step_by(7).enumerate() { + for (dy, row) in LETTERS[(k + line) % 3].iter().enumerate() { + for (dx, _) in row.bytes().enumerate().filter(|(_, c)| *c == b'#') { + bits[(top + dy) * stride + (x + dx) / 8] &= !(0x80 >> ((x + dx) % 8)); + } + } + } + } + let coded = pdfcraft_filters::encode_jbig2(&bits, w as u32, h as u32).unwrap(); + // The end of the segment at `at`, and where its page association is (7.2.2 to 7.2.7). + let end = |at: usize| { + let page = at + 6 + usize::from(coded[at + 5] >> 5); + let length = u32::from_be_bytes(coded[page + 1..page + 5].try_into().unwrap()); + (page + 5 + length as usize, page) + }; + let (dictionary, _) = end(0); + assert_eq!(coded[dictionary + 4], 0, "the second segment is the symbol dictionary"); + let (rest, page) = end(dictionary); + let mut symbols = coded[dictionary..rest].to_vec(); + symbols[page - dictionary] = 0; + SharedScan { page: [&coded[..dictionary], &coded[rest..]].concat(), symbols, bits } +} + +/// An image of [`SharedScan`]'s size that uses the dictionary, object 9, by `parms`. +fn shared_image(parms: &str) -> String { + format!("/Type /XObject /Subtype /Image /Width 400 /Height 40 /BitsPerComponent 1 /ColorSpace /DeviceGray /Filter /JBIG2Decode {parms}") +} + +/// Two pages of a scan sharing object 9: its symbols, and `LEAK_GLOBALS`, for the shape of a mark found +/// only under the area. The first page draws image 7 unless told otherwise; the second draws image 8. +/// `extra` are objects 10 on. +struct SharedPages { + /// What the first page draws, its `/Resources` entries and anything else in its dictionary. + content: Vec, + resources: String, + page: String, + /// Image 7's dictionary and data; image 8's. + first: String, + first_data: Vec, + other: String, + other_data: Vec, + extra: Vec>, +} + +const DRAWS: &[u8] = b"q 200 0 0 20 0 100 cm /Im Do Q"; + +impl SharedPages { + fn new(scan: &SharedScan) -> Self { + let image = shared_image(SHARED); + Self { + content: DRAWS.to_vec(), + resources: "/XObject << /Im 7 0 R >>".into(), + page: String::new(), + first: image.clone(), + first_data: scan.page.clone(), + other: image, + other_data: scan.page.clone(), + extra: vec![], + } + } + + fn build(self, scan: &SharedScan) -> Document { + let Self { content, resources, page, first, first_data, other, other_data, extra } = self; + let mut objs: Vec> = vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>".to_vec(), + format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 5 0 R /Resources << {resources} >> {page} >>").into_bytes(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 6 0 R /Resources << /XObject << /Im 8 0 R >> >> >>".to_vec(), + stream("", content), + stream("", DRAWS), + stream(&first, first_data), + stream(&other, other_data), + stream("", [jbig2_globals(b"LEAK_GLOBALS"), scan.symbols.clone()].concat()), + ]; + objs.extend(extra); + pdf(objs) + } +} + +/// The samples of the image the page draws as `/Im`. +fn drawn_samples(doc: &Document, page: usize) -> Vec { + let p = pdfcraft_model::pages(doc).swap_remove(page); + let res = doc.resolve(p.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + let xo = doc.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap(); + match &*doc.get(xo.get(b"Im").unwrap().as_ref().unwrap()) { + Object::Stream(s) => doc.decoded_image(s).unwrap(), + other => panic!("not an image: {other:?}"), + } +} + +const SHARED: &str = "/DecodeParms << /JBIG2Globals 9 0 R >>"; +const CLEARS: [f64; 4] = [0.25, 113.75, 49.75, 120.0]; +const COVERS: [f64; 4] = [0.0, 100.0, 200.0, 120.0]; + +/// The ways image 7 can leave the document while taking the dictionary's only reason to stay with it. +fn ways_out(scan: &SharedScan) -> Vec<(&'static str, [f64; 4], SharedPages)> { + let base = || SharedPages::new(scan); + let in_form = |filter: &str, data: &[u8]| { + stream(&format!("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /XObject << /Im 7 0 R >> >> {filter}"), data) + }; + let plain = "/Type /XObject /Subtype /Image /Width 1 /Height 1 /BitsPerComponent 8 /ColorSpace /DeviceGray"; + let through_form = |filter: &str, data: &[u8]| SharedPages { + content: b"/Fm Do".to_vec(), + resources: "/XObject << /Fm 10 0 R >>".into(), + extra: vec![in_form(filter, data)], + ..base() + }; + let annotated = |subtype: &str| SharedPages { + content: Vec::new(), + resources: String::new(), + page: "/Annots [10 0 R]".into(), + extra: vec![ + format!("<< /Type /Annot /Subtype /{subtype} /T (Field) /FT /Tx /Rect [0 100 200 120] /AP << /N 11 0 R >> >>").into_bytes(), + in_form("", DRAWS), + ], + ..base() + }; + let glyph = SharedPages { + content: b"BT /T3 200 Tf 0 100 Td (a) Tj ET".to_vec(), + resources: "/Font << /T3 10 0 R >>".into(), + extra: vec![ + b"<< /Type /Font /Subtype /Type3 /FontBBox [0 0 1000 1000] /FontMatrix [0.001 0 0 0.001 0 0] /CharProcs << /a 11 0 R >> \ + /Encoding << /Differences [97 /a] >> /FirstChar 97 /LastChar 97 /Widths [1000] /Resources << /XObject << /Im 7 0 R >> >> >>" + .to_vec(), + stream("", b"1000 0 d0 q 1000 0 0 100 0 0 cm /Im Do Q"), + ], + ..base() + }; + vec![ + ("cleared", CLEARS, base()), + ("removed", COVERS, base()), + ("in a form", COVERS, through_form("", DRAWS)), + ("in a form that can't be read", COVERS, through_form("/Filter /FlateDecode", b"not Flate")), + ("a stamp's appearance", COVERS, annotated("Stamp")), + ("a widget's appearance", COVERS, annotated("Widget")), + ("a Type 3 glyph", COVERS, glyph), + ( + "a mask", + COVERS, + SharedPages { first: format!("{plain} /SMask 10 0 R"), extra: vec![stream(&shared_image(SHARED), scan.page.clone())], ..base() }, + ), + ( + "an alternate", + COVERS, + SharedPages { + first: format!("{plain} /Alternates [<< /Image 10 0 R >>]"), + extra: vec![stream(&shared_image(SHARED), scan.page.clone())], + ..base() + }, + ), + ( + "/Filter by reference", + COVERS, + SharedPages { + first: shared_image("/DecodeParms [null << /JBIG2Globals 9 0 R >>]").replace("/Filter /JBIG2Decode", "/Filter 10 0 R"), + extra: vec![b"[/ASCIIHexDecode /JBIG2Decode]".to_vec()], + ..base() + }, + ), + ( + "a reference to it", + COVERS, + SharedPages { first: shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"), extra: vec![b"9 0 R".to_vec()], ..base() }, + ), + ( + "parameters through a reference to them", + COVERS, + SharedPages { + first: shared_image("/DecodeParms 10 0 R"), + extra: vec![b"11 0 R".to_vec(), b"<< /JBIG2Globals 9 0 R >>".to_vec()], + ..base() + }, + ), + ( + "an entry through a reference to it", + COVERS, + SharedPages { + first: shared_image("/DecodeParms [10 0 R]"), + extra: vec![b"11 0 R".to_vec(), b"<< /JBIG2Globals 9 0 R >>".to_vec()], + ..base() + }, + ), + ( + "an array of them through a reference to it", + COVERS, + SharedPages { + first: shared_image("/DecodeParms 10 0 R"), + extra: vec![b"11 0 R".to_vec(), b"[<< /JBIG2Globals 9 0 R >>]".to_vec()], + ..base() + }, + ), + ("/DP", COVERS, SharedPages { first: shared_image("/DP << /JBIG2Globals 9 0 R >>"), ..base() }), + ("another generation", COVERS, SharedPages { first: shared_image("/DecodeParms << /JBIG2Globals 9 1 R >>"), ..base() }), + ] +} + +/// Scanners and `jbig2enc -s` share one symbol dictionary across a scan's pages, and it holds the +/// shape of every distinct mark, one found only under a redaction included: a signature, a stamp. +/// The dictionary stayed in the file while any other image used it. Now the image left on the second +/// page is stored again without it, whichever way the first one left the document. +#[test] +fn a_redacted_scans_shared_jbig2_dictionary_leaves_the_file() { + let scan = shared_scan(); + for (way, rect, pages) in ways_out(&scan) { + let mut doc = pages.build(&scan); + assert_eq!(drawn_samples(&doc, 1), scan.bits, "{way}: the second page reads through the dictionary"); + mark(&mut doc, 0, &[rect], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!(report.images_restored, 1, "{way}"); + assert_gone(&doc, b"LEAK_GLOBALS"); + assert_gone(&doc, b"JBIG2Globals"); + assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits, "{way}: its pixels are unchanged"); + } +} + +/// However the image that stays names the dictionary, it comes out without it: its parameters by +/// reference, behind another filter, or a copy of the dictionary, as a file put back together from +/// pages split apart has, each page with its own. It is also read when its size or filter is given +/// by reference, which `decoded_image` alone refuses. +#[test] +fn what_stays_is_stored_without_the_dictionary_however_it_names_it() { + let scan = shared_scan(); + let hex: Vec = scan.page.iter().map(|b| format!("{b:02x}")).collect::().bytes().chain(*b">").collect(); + let copy = stream("", [jbig2_globals(b"LEAK_GLOBALS"), scan.symbols.clone()].concat()); + let image = shared_image(SHARED); + for (other, data, extra) in [ + (shared_image("/DecodeParms 10 0 R"), scan.page.clone(), vec![b"<< /JBIG2Globals 9 0 R >>".to_vec()]), + ( + shared_image("/DecodeParms [null << /JBIG2Globals 9 0 R >>]").replace("/Filter /JBIG2Decode", "/Filter [/ASCIIHexDecode /JBIG2Decode]"), + hex.clone(), + vec![], + ), + (shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"), scan.page.clone(), vec![copy]), + (image.replace("/Width 400", "/Width 10 0 R"), scan.page.clone(), vec![b"400".to_vec()]), + (image.replace("/Height 40", "/Height 10 0 R"), scan.page.clone(), vec![b"40".to_vec()]), + (image.replace("/Filter /JBIG2Decode", "/Filter 10 0 R"), scan.page.clone(), vec![b"/JBIG2Decode".to_vec()]), + ( + shared_image("/DecodeParms [null << /JBIG2Globals 9 0 R >>]").replace("/Filter /JBIG2Decode", "/Filter [/ASCIIHexDecode 10 0 R]"), + hex.clone(), + vec![b"/JBIG2Decode".to_vec()], + ), + (image.replace("/ColorSpace /DeviceGray ", ""), scan.page.clone(), vec![]), + ( + image.replace("/ColorSpace /DeviceGray", "/ColorSpace [/DeviceN [/Black] /DeviceGray 10 0 R]"), + scan.page.clone(), + vec![b"<< /FunctionType 2 /Domain [0 1] /N 1 >>".to_vec()], + ), + // Parameters no filter reads, on a picture that needs no dictionary. + (shared_image("/DP << /JBIG2Globals 9 0 R >>"), pdfcraft_filters::encode_jbig2_generic(&scan.bits, 400, 40).unwrap(), vec![]), + ] { + let mut doc = SharedPages { other: other.clone(), other_data: data, extra, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!(report.images_restored, 1, "{other}"); + assert_gone(&doc, b"LEAK_GLOBALS"); + assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits, "{other}: its pixels are unchanged"); + } +} + +/// JBIG2 is drawn as one bit of one component whatever the dictionary says, so the Flate copy has to +/// say so too: a stream with no `/BitsPerComponent`, or 8, shows the same picture stored again. +#[test] +fn a_stored_jbig2_image_is_one_bit_whatever_its_dictionary_said() { + let scan = shared_scan(); + let hex: Vec = scan.page.iter().map(|b| format!("{b:02x}")).collect::().bytes().chain(*b">").collect(); + let chained = + shared_image("/DecodeParms [null << /JBIG2Globals 9 0 R >>]").replace("/Filter /JBIG2Decode", "/Filter [/ASCIIHexDecode /JBIG2Decode]"); + let lone = shared_image(SHARED).replace("/Filter /JBIG2Decode", "/Filter [/JBIG2Decode]"); + for (other, other_data, bits) in [ + (shared_image(SHARED), scan.page.clone(), ""), + (shared_image(SHARED), scan.page.clone(), "/BitsPerComponent 8"), + (chained, hex, ""), + (lone, scan.page.clone(), ""), + ] { + let other = other.replace("/BitsPerComponent 1", bits); + let mut doc = SharedPages { other, other_data, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + apply(&mut doc, None).unwrap(); + let saved = reopen(&doc); + assert_eq!(drawn_samples(&saved, 1), scan.bits, "{bits}"); + let p = pdfcraft_model::pages(&saved).swap_remove(1); + let res = saved.resolve(p.dict.get(b"Resources").unwrap()).as_dict().cloned().unwrap(); + let xo = saved.resolve(res.get(b"XObject").unwrap()).as_dict().cloned().unwrap(); + let image = saved.get(xo.get(b"Im").unwrap().as_ref().unwrap()); + let Object::Stream(s) = &*image else { panic!("not an image") }; + assert_eq!(s.dict.int(b"BitsPerComponent"), Some(1), "{bits}"); + } +} + +/// A mark that takes nothing using the dictionary leaves it, and what uses it, alone: here it covers +/// text in a form that also draws the scan, away from the scan. A stream nothing reaches isn't decoded +/// for nothing either: this one can't be, and it uses the dictionary. +#[test] +fn a_dictionary_stays_when_nothing_using_it_left() { + let scan = shared_scan(); + let text = b"BT /F1 10 Tf 10 250 Td (SECRET) Tj ET q 200 0 0 20 0 100 cm /Im Do Q"; + let pages = SharedPages { + content: b"/Fm Do".to_vec(), + resources: "/XObject << /Fm 10 0 R >>".into(), + extra: vec![ + stream( + "/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /Font << /F1 << /Type /Font /Subtype /Type1 /BaseFont /Helvetica >> >> /XObject << /Im 7 0 R >> >>", + text, + ), + stream(&shared_image(SHARED), b"not JBIG2"), + ], + ..SharedPages::new(&scan) + }; + let mut doc = pages.build(&scan); + mark(&mut doc, 0, &[[0.0, 240.0, 200.0, 270.0]], ""); + let report = apply(&mut doc, None).unwrap(); + assert_eq!((report.images_removed, report.images_cleared, report.images_restored), (0, 0, 0)); + assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits); +} + +/// A dictionary of other bytes is another dictionary: the scan using it keeps it and isn't stored again. +#[test] +fn another_jbig2_dictionary_is_left_alone() { + let scan = shared_scan(); + let own = stream("", [jbig2_globals(b"OWN_GLOBALS"), scan.symbols.clone()].concat()); + let other = shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"); + let mut doc = SharedPages { other, extra: vec![own], ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_restored, 0); + let saved = reopen(&doc); + let holds = |secret: &[u8]| { + saved.scan_objects().any(|(_, o)| matches!(&*o, Object::Stream(s) if s.decoded().is_ok_and(|d| d.windows(secret.len()).any(|w| w == secret)))) + }; + assert!(!holds(b"LEAK_GLOBALS") && holds(b"OWN_GLOBALS")); + assert_eq!(drawn_samples(&saved, 1), scan.bits); +} + +/// An image that can't be stored without the dictionary may still be drawn: the renderer draws a +/// JBIG2 page of another size than its image, which `decoded_image` refuses, and draws three components +/// of one bit as grey. Removing it would lose a page no mark reached, and keeping the dictionary would +/// keep what the mark covered, so the redaction refuses and the document stays as it was. So it does +/// for anything else that refers to the dictionary, a reference that holds a reference to it among them. +#[test] +fn a_shared_jbig2_dictionary_that_cant_leave_refuses_the_redaction() { + let scan = shared_scan(); + let image = shared_image(SHARED); + let elsewhere = "/Type /XObject /Subtype /Image /Width 1 /Height 1 /BitsPerComponent 1 /ColorSpace /DeviceGray /Note 9 0 R"; + let not_jbig2 = image.replace("/Filter /JBIG2Decode", "/Filter /FlateDecode"); + for (other, data, extra) in [ + (image.replace("/Height 40", "/Height 41"), scan.page.clone(), vec![]), + (image.replace("/DeviceGray", "/DeviceRGB"), scan.page.clone(), vec![]), + (shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"), scan.page.clone(), vec![b"9 0 R".to_vec()]), + (elsewhere.to_string(), vec![0], vec![]), + (not_jbig2, pdfcraft_filters::encode_flate(&scan.bits), vec![]), + // A copy with its filter by reference is known for a copy, and the decoder doesn't read it. + ( + shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"), + scan.page.clone(), + vec![ + stream("/Filter 11 0 R", pdfcraft_filters::encode_flate(&[jbig2_globals(b"LEAK_GLOBALS"), scan.symbols.clone()].concat())), + b"/FlateDecode".to_vec(), + ], + ), + ] { + let mut doc = SharedPages { other: other.clone(), other_data: data, extra, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + let before = doc.clone(); + assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(_))), "{other}"); + assert_eq!(drawn_samples(&doc, 0), drawn_samples(&before, 0), "{other}: nothing was redacted"); + } +} + +/// What is stored again is bounded as what is cleared is: an image of [`image::MAX_PIXELS`] is, and one +/// past it, which decodes, refuses the redaction. +#[test] +fn an_image_past_the_size_redaction_clears_refuses_it() { + let scan = shared_scan(); + let h = 8192u32; + for (w, stored) in [(8192u32, true), (8200, false)] { + let blank = vec![0xFFu8; (w as usize).div_ceil(8) * h as usize]; + let other = shared_image(SHARED).replace("/Width 400", &format!("/Width {w}")).replace("/Height 40", &format!("/Height {h}")); + let data = pdfcraft_filters::encode_jbig2_generic(&blank, w, h).unwrap(); + let mut doc = SharedPages { other, other_data: data, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + let run = apply(&mut doc, None); + assert_eq!(run.as_ref().is_ok_and(|r| r.images_restored == 1), stored, "{w}"); + assert_eq!(matches!(run, Err(RedactError::SharedJbig2Globals(1))), !stored, "{w}"); + } +} + +/// An alternate of an image stored again stays with it, stored without the dictionary in turn: a page +/// no mark reached keeps what it had. +#[test] +fn an_alternate_of_what_stays_stays() { + let scan = shared_scan(); + let other = format!("{} /Alternates [<< /Image 10 0 R >>]", shared_image(SHARED)); + let mut doc = SharedPages { other, extra: vec![stream(&shared_image(SHARED), scan.page.clone())], ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_restored, 2); + assert_gone(&doc, b"LEAK_GLOBALS"); + let saved = reopen(&doc); + let kept = saved.scan_objects().filter(|(_, o)| matches!(&**o, Object::Stream(s) if s.dict.contains(b"Alternates"))).count(); + assert_eq!(kept, 1); +} + +/// The refusal counts the images that can't be stored, or says 1 for something else that refers to +/// the dictionary. +#[test] +fn a_refused_redaction_counts_what_cant_be_stored() { + let scan = shared_scan(); + let wrong = shared_image(SHARED).replace("/Height 40", "/Height 41"); + let other = format!("{wrong} /SMask 10 0 R"); + let mut doc = SharedPages { other, extra: vec![stream(&wrong, scan.page.clone())], ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(2)))); +} + +/// Remove Hidden Information leaves the same way: comments go, with what their appearances drew. +#[test] +fn removing_hidden_information_takes_a_shared_jbig2_dictionary_too() { + let scan = shared_scan(); + let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /XObject << /Im 7 0 R >> >>", DRAWS); + let annotation = b"<< /Type /Annot /Subtype /Stamp /Rect [0 100 200 120] /AP << /N 11 0 R >> >>".to_vec(); + let mut doc = SharedPages { + content: Vec::new(), + resources: String::new(), + page: "/Annots [10 0 R]".into(), + extra: vec![annotation, form], + ..SharedPages::new(&scan) + } + .build(&scan); + crate::sanitize::remove_hidden(&mut doc, &[crate::sanitize::Hidden::Comments]).unwrap(); + assert_gone(&doc, b"LEAK_GLOBALS"); + assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits); +} + +/// [`assert_no_secret`] for a file that may hold JBIG2 streams, which `decoded` doesn't read: a stream +/// stored again can be coded alone. +fn assert_gone(doc: &Document, secret: &[u8]) { + for (r, object) in reopen(doc).scan_objects() { + let mut bytes = Vec::new(); + pdfcraft_cos::serialize(&object, &mut bytes); + assert!(!bytes.windows(secret.len()).any(|w| w == secret), "secret remains in object {r:?}"); + if let Object::Stream(s) = &*object + && let Ok(data) = s.decoded() + { + assert!(!data.windows(secret.len()).any(|w| w == secret), "secret remains in stream {r:?}"); + } + } +} + /// Apply Redactions gives the page its own pruned /Resources, but the dictionary it drew through /// before (inherited from the /Pages node, or shared with another page) still named the removed /// image and form, so a full-rewrite save kept them. diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index cabfc7823a..608c2ad275 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5829,8 +5829,8 @@ milestone = "M8" status = "shipped" commands = ["redact.apply"] tools = ["redact_apply"] -evidence = ["crates/redact/src/tests.rs::images_are_removed_or_have_their_pixels_cleared", "crates/redact/src/tests.rs::redaction_clears_pixels_intersected_without_covering_their_centres", "crates/redact/src/tests.rs::a_fax_image_has_its_covered_pixels_cleared_instead_of_being_removed", "crates/redact/src/tests.rs::group_3_on_bytes_is_removed_whole_and_other_fax_images_cleared", "crates/redact/src/tests.rs::an_image_whose_predictor_is_given_by_reference_has_the_covered_pixel_cleared", "crates/redact/src/tests.rs::a_jbig2_image_has_its_covered_pixels_cleared_instead_of_being_removed", "crates/redact/src/tests.rs::a_jbig2_image_that_cannot_be_cleared_for_certain_is_removed_whole", "crates/redact/src/tests.rs::a_redacted_jbig2_scan_keeps_neither_its_code_nor_its_globals", "crates/redact/src/tests.rs::a_cleared_black_and_white_scan_is_stored_as_group_4", "crates/redact/src/tests.rs::globals_a_redacted_jbig2_image_shares_with_another_page_leave_the_file"] -notes = "intersected pixel footprints are cleared conservatively in a copy, CCITT fax and JBIG2 images (scans stored as Group 4 or JBIG2, with their /JBIG2Globals) included; DCT/JPX images, Group 3 on bytes (which the bootstrap renderer may draw as other rows than the decoder reads), and a JBIG2 image whose page isn't its size, removed whole. The copy is Flate, or for a one-bit image of one component Group 4, or JBIG2 when it was JBIG2, whichever is smallest as stored, and refers to no globals; every other image using a redacted image's globals is stored again without them (Flate, Group 4 or JBIG2, whichever is smallest), so they leave the file (one that can't be decoded refuses the redaction)" +evidence = ["crates/redact/src/tests.rs::images_are_removed_or_have_their_pixels_cleared", "crates/redact/src/tests.rs::a_redacted_scans_shared_jbig2_dictionary_leaves_the_file", "crates/redact/src/tests.rs::redaction_clears_pixels_intersected_without_covering_their_centres", "crates/redact/src/tests.rs::a_fax_image_has_its_covered_pixels_cleared_instead_of_being_removed", "crates/redact/src/tests.rs::group_3_on_bytes_is_removed_whole_and_other_fax_images_cleared", "crates/redact/src/tests.rs::an_image_whose_predictor_is_given_by_reference_has_the_covered_pixel_cleared", "crates/redact/src/tests.rs::a_jbig2_image_has_its_covered_pixels_cleared_instead_of_being_removed", "crates/redact/src/tests.rs::a_jbig2_image_that_cannot_be_cleared_for_certain_is_removed_whole", "crates/redact/src/tests.rs::a_redacted_jbig2_scan_keeps_neither_its_code_nor_its_globals", "crates/redact/src/tests.rs::a_cleared_black_and_white_scan_is_stored_as_group_4", "crates/redact/src/tests.rs::globals_a_redacted_jbig2_image_shares_with_another_page_leave_the_file"] +notes = "intersected pixel footprints are cleared conservatively in a copy, CCITT fax and JBIG2 images (scans stored as Group 4 or JBIG2, with their /JBIG2Globals) included; DCT/JPX images, Group 3 on bytes (which the bootstrap renderer may draw as other rows than the decoder reads), and a JBIG2 image whose page isn't its size, removed whole. The copy is Flate, or for a one-bit image of one component Group 4, or JBIG2 when it was JBIG2, whichever is smallest as stored, and refers to no globals; whatever left the file takes its globals with it: what else names them, or an exact copy, is stored again without them (Flate, Group 4 or JBIG2, whichever is smallest), and apply refuses when one can't be. Not found: a dictionary holding the same shapes in other bytes. An image that hasn't left the file keeps its globals with its pixels: one another page draws, or one a pattern, soft mask or structure element refers to" [[feature]] id = "protect.redact-vectors" From 13422bf279bdd14ccfa009fa7be2c46cbb4adaac Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 01:15:53 +0100 Subject: [PATCH 02/27] M8: the shared JBIG2 refusal says no more than it knows apply works on a copy, so nothing changes when it refuses, but remove_hidden isn't transactional: a direct caller's document is part changed by then. The message said nothing was changed for both. The app's callers work on copies. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 4b68fc2e73..1aa8ecce0c 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -76,7 +76,7 @@ pub enum RedactError { /// hold, those of marks under the area included, would stay. Counts the images, or 1 for another /// reference. #[error( - "{0} item(s) share JBIG2 symbol data with something being removed and can't be stored without it, so that data, which can show what was removed, would stay in the file; nothing was changed" + "{0} item(s) share JBIG2 symbol data with something being removed and can't be stored without it, so that data, which can show what was removed, would stay in the file; this was refused" )] SharedJbig2Globals(usize), #[error(transparent)] From b3b47b651d6964e152bfc98469c862f1b734b857 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 08:26:31 +0100 Subject: [PATCH 03/27] M8: the shared JBIG2 refusal says what keeps the data When nothing that names the dictionary is stuck and something else refers to it (a private key, say), the refusal said 1 item(s) couldn't be stored without it. It now counts 0 and says something else refers to the data. remove_hidden's refusal gets a test: the comment has gone by then, and the text doesn't say nothing was changed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 6 ++--- crates/redact/src/lib.rs | 17 +++++++++---- crates/redact/src/tests.rs | 48 +++++++++++++++++++++++++++++++++--- 3 files changed, 60 insertions(+), 11 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 349b9e267a..cd0bc982d8 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -133,8 +133,8 @@ fn without_dictionary(doc: &Document, s: &Stream) -> Option { /// more. /// /// Errors: [`RedactError::SharedJbig2Globals`](crate::RedactError::SharedJbig2Globals) when a dictionary would -/// still be kept, as what names it can't be stored without it or something else refers to it. Such -/// an image may still be drawn, on a page no mark reached, so it isn't removed, and `doc` is part +/// still be kept, as what names it can't be stored without it (counted) or something else refers to +/// it (0 when that is all). Such an image may still be drawn, on a page no mark reached, so it isn't removed, and `doc` is part /// changed by then: a caller works on a copy it can drop. pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result { let now = kept(doc); @@ -176,7 +176,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result String { + let why = match stuck { + 0 => "something else in the file refers to JBIG2 symbol data shared with something being removed".to_string(), + n => format!("{n} item(s) share JBIG2 symbol data with something being removed and can't be stored without it"), + }; + format!("{why}, so that data, which can show what was removed, would stay in the file; this was refused") +} + /// A redaction mark on a page. #[derive(Clone, Debug, PartialEq)] pub struct Mark { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index a5ec57fc4e..04a4ed1f9b 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1660,8 +1660,8 @@ fn an_alternate_of_what_stays_stays() { assert_eq!(kept, 1); } -/// The refusal counts the images that can't be stored, or says 1 for something else that refers to -/// the dictionary. +/// The refusal counts the images that can't be stored, and says so; when only something else refers +/// to the dictionary, it counts none and says that instead, however many such references there are. #[test] fn a_refused_redaction_counts_what_cant_be_stored() { let scan = shared_scan(); @@ -1669,7 +1669,23 @@ fn a_refused_redaction_counts_what_cant_be_stored() { let other = format!("{wrong} /SMask 10 0 R"); let mut doc = SharedPages { other, extra: vec![stream(&wrong, scan.page.clone())], ..SharedPages::new(&scan) }.build(&scan); mark(&mut doc, 0, &[CLEARS], ""); - assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(2)))); + let refused = apply(&mut doc, None); + assert!(matches!(refused, Err(RedactError::SharedJbig2Globals(2)))); + assert!(refused.unwrap_err().to_string().starts_with("2 item(s) share JBIG2 symbol data with something being removed and can't be stored")); + let note = "/Type /XObject /Subtype /Image /Width 1 /Height 1 /BitsPerComponent 1 /ColorSpace /DeviceGray /Note 9 0 R"; + for (other, extra, stuck) in [ + (note.to_string(), vec![], 0), + (format!("{note} /SMask 10 0 R"), vec![stream(note, vec![0])], 0), + (format!("{note} /SMask 10 0 R"), vec![stream(&wrong, scan.page.clone())], 1), + ] { + let mut doc = SharedPages { other: other.clone(), other_data: vec![0], extra, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + let refused = apply(&mut doc, None); + assert!(matches!(refused, Err(RedactError::SharedJbig2Globals(n)) if n == stuck), "{other}"); + let text = refused.unwrap_err().to_string(); + assert_eq!(text.starts_with("something else in the file refers to JBIG2 symbol data"), stuck == 0, "{other}: {text}"); + assert_eq!(text.contains("can't be stored"), stuck > 0, "{other}: {text}"); + } } /// Remove Hidden Information leaves the same way: comments go, with what their appearances drew. @@ -1691,6 +1707,32 @@ fn removing_hidden_information_takes_a_shared_jbig2_dictionary_too() { assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits); } +/// Remove Hidden Information refuses as a redaction does when the dictionary can't leave, but it has +/// changed the document by then, so the refusal doesn't say nothing was changed. +#[test] +fn a_refused_removal_of_hidden_information_doesnt_say_nothing_changed() { + let scan = shared_scan(); + let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300] /Resources << /XObject << /Im 7 0 R >> >>", DRAWS); + let annotation = b"<< /Type /Annot /Subtype /Stamp /Rect [0 100 200 120] /AP << /N 11 0 R >> >>".to_vec(); + let mut doc = SharedPages { + content: Vec::new(), + resources: String::new(), + page: "/Annots [10 0 R]".into(), + other: shared_image(SHARED).replace("/Filter /JBIG2Decode", "/Filter /FlateDecode"), + other_data: pdfcraft_filters::encode_flate(&scan.bits), + extra: vec![annotation, form], + ..SharedPages::new(&scan) + } + .build(&scan); + let annotations = |doc: &Document| crate::annots_of(doc, &pdfcraft_model::pages(doc)[0].dict).len(); + assert_eq!(annotations(&doc), 1); + let refused = crate::sanitize::remove_hidden(&mut doc, &[crate::sanitize::Hidden::Comments]); + assert!(matches!(refused, Err(RedactError::SharedJbig2Globals(1)))); + assert_eq!(annotations(&doc), 0, "the comment went before the refusal"); + let text = refused.unwrap_err().to_string(); + assert!(text.ends_with("this was refused") && !text.contains("nothing was changed"), "{text}"); +} + /// [`assert_no_secret`] for a file that may hold JBIG2 streams, which `decoded` doesn't read: a stream /// stored again can be coded alone. fn assert_gone(doc: &Document, secret: &[u8]) { From 14e5eec34121a9fb9670ad345c94e12da97f181b Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 08:39:00 +0100 Subject: [PATCH 04/27] M8: a dictionary named beside a shared one stays, whatever the order A kept image that named the dictionary added every dictionary it names to what had to leave, in one pass. An image using only one of those, earlier in the file, had been passed over, so it kept that dictionary and the redaction refused, saying something else referred to the data. In the other order it was stored again. Now only the names that matched have to leave, so another dictionary stays with what uses it, in either order. The variant's doc says what is counted: something that names the data, JBIG2 or not. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 9 +++++--- crates/redact/src/lib.rs | 2 +- crates/redact/src/tests.rs | 42 ++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 4 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index cd0bc982d8..8bfb9733fc 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -157,11 +157,14 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result>| read.entry(*n).or_insert_with(|| content(doc, *n)).is_some_and(|c| contents.contains(&c)); - if names.iter().any(|n| dictionaries.contains(n) || copy(n, &mut read)) { - dictionaries.extend(names); + // Only what matched has to leave, a copy found here included. Another dictionary this also + // names was shared with nothing that left: what else uses it may keep it, whichever comes + // first in the file. + let matched: Vec = named(doc, s).into_iter().filter(|n| dictionaries.contains(n) || copy(n, &mut read)).collect(); + if !matched.is_empty() { + dictionaries.extend(matched); users.push(r); } } diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 477d4bcb37..d7108e77ae 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -71,7 +71,7 @@ pub enum RedactError { /// could not be shown complete. #[error("the document's objects nest too deeply to remove hidden information completely; the operation was refused")] TooDeep, - /// `/JBIG2Globals` used by something removed would stay: a JBIG2 image shares them and can't be + /// `/JBIG2Globals` used by something removed would stay: something that names them can't be /// stored again without them, or something else refers to them. Either way the shapes they /// hold, those of marks under the area included, would stay. Counts what can't be stored without /// them: 0 when only something else keeps them. diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 04a4ed1f9b..7512211326 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1688,6 +1688,48 @@ fn a_refused_redaction_counts_what_cant_be_stored() { } } +/// An image that names the dictionary and another one is stored again without both, but the other +/// was shared with nothing that left: an image that uses only it stays as it was, whichever of the +/// two comes first in the file, and so does that dictionary. +#[test] +fn another_dictionary_named_beside_it_stays_whatever_the_order() { + let scan = shared_scan(); + let other_dictionary = stream("", [jbig2_globals(b"OTHER_GLOBALS"), scan.symbols.clone()].concat()); + let only_other = shared_image("/DecodeParms << /JBIG2Globals 11 0 R >>"); + let chained = "/Type /XObject /Subtype /Image /Width 400 /Height 40 /BitsPerComponent 1 /ColorSpace /DeviceGray /Filter [/FlateDecode /JBIG2Decode] /DecodeParms [<< /JBIG2Globals 11 0 R >> << /JBIG2Globals 9 0 R >>]"; + let both_spellings = [ + (chained.to_string(), pdfcraft_filters::encode_flate(&scan.page)), + (format!("{} /DP << /JBIG2Globals 11 0 R >>", shared_image(SHARED)), scan.page.clone()), + ]; + let other_stays = |doc: &Document| { + let saved = reopen(doc); + saved.scan_objects().any(|(_, o)| matches!(&*o, Object::Stream(s) if s.raw.windows(13).any(|w| w == b"OTHER_GLOBALS"))) + }; + for (both, data) in both_spellings { + // Image 8 is drawn on the second page, and image 10 is its alternate. + let only_other_first = SharedPages { + other: format!("{only_other} /Alternates [<< /Image 10 0 R >>]"), + extra: vec![stream(&both, data.clone()), other_dictionary.clone()], + ..SharedPages::new(&scan) + }; + let both_first = SharedPages { + other: format!("{both} /Alternates [<< /Image 10 0 R >>]"), + other_data: data.clone(), + extra: vec![stream(&only_other, scan.page.clone()), other_dictionary.clone()], + ..SharedPages::new(&scan) + }; + for (pages, only_other_at) in [(only_other_first, 8), (both_first, 10)] { + let mut doc = pages.build(&scan); + let untouched = doc.get(pdfcraft_cos::ObjRef::new(only_other_at, 0)); + mark(&mut doc, 0, &[CLEARS], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_restored, 1, "{both}"); + assert_gone(&doc, b"LEAK_GLOBALS"); + assert!(other_stays(&doc), "{both}"); + assert_eq!(doc.get(pdfcraft_cos::ObjRef::new(only_other_at, 0)), untouched, "{both}"); + } + } +} + /// Remove Hidden Information leaves the same way: comments go, with what their appearances drew. #[test] fn removing_hidden_information_takes_a_shared_jbig2_dictionary_too() { From cb1a04073171f3b2c2ab82eba1ee7df6f515a026 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 09:55:48 +0100 Subject: [PATCH 05/27] M8: a shared JBIG2 dictionary is found in /FDecodeParms too /FDecodeParms are the parameters of data in another file, which a reader may draw instead of the stream's own. A dictionary that only a removed image named there stayed in the file with Ok. It is now read with /DecodeParms and /DP, and an image whose data is in another file counts as one that can't be stored without the dictionary: readers draw that file, not a copy stored here. images_restored counts what was stored again. A dictionary that names itself as its own globals (malformed) leaves with what used it but can't be stored, and was counted. The table's /Mask route gets a row in the routes test, as it was only measured. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/README.md | 4 ++-- crates/redact/src/globals.rs | 20 +++++++++++++----- crates/redact/src/tests.rs | 41 ++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 7 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index daaacc4442..95c0e07161 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -26,9 +26,9 @@ clear_marks(&mut doc, None)?; // remove marks without app no globals. Globals hold the shapes of every image that shares them (a scan's pages, often), those of marks under the area included, so whatever left the document, by any route, takes its globals with it on the full rewrite: anything else the save would keep that names them, or a - byte-for-byte copy of them, is stored again without them, as a copy is (Flate, Group 4 or JBIG2, + copy of them (the same bytes once decoded), is stored again without them, as a copy is (Flate, Group 4 or JBIG2, whichever is smallest; `Report::images_restored`). When one can't be (it isn't JBIG2, it's past - the size redaction clears, or it can't be decoded), or something else refers to them, `apply` + the size redaction clears, its data is in another file, or it can't be decoded), or something else refers to them, `apply` refuses and changes nothing (`RedactError::SharedJbig2Globals`). `remove_hidden` takes them out the same way. Another dictionary holding the same shapes in other bytes isn't found, nor is a copy nothing names, and an image that hasn't left keeps its globals with its pixels: one another page diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 8bfb9733fc..13bc851957 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -49,7 +49,8 @@ fn named(doc: &Document, s: &Stream) -> Vec { } } }; - for key in [b"DecodeParms".as_slice(), b"DP"] { + // `/FDecodeParms` too: those of data in another file, which a reader may draw instead. + for key in [b"DecodeParms".as_slice(), b"DP", b"FDecodeParms"] { let Some(parms) = s.dict.get(key).map(|p| followed(doc, p)) else { continue }; match &*parms { Object::Array(entries) => entries.iter().for_each(&mut read), @@ -94,8 +95,12 @@ fn content(doc: &Document, num: u32) -> Option { /// referred to. `None` for a stream that isn't JBIG2, whose decoder took no dictionary and whose /// samples another filter may give otherwise; for one past the size of an image redaction clears, /// or known to have more than one component, which JBIG2 is drawn as one of whatever the dictionary -/// says; and for one that can't be decoded. +/// says; for one whose data is in another file (`/F`), which readers draw instead of what is here; +/// and for one that can't be decoded. fn without_dictionary(doc: &Document, s: &Stream) -> Option { + if s.dict.contains(b"F") { + return None; + } let readable = readable(doc, s); let jbig2 = match readable.dict.get(b"Filter") { Some(Object::Array(a)) => a.iter().any(|f| f.as_name() == Some(b"JBIG2Decode".as_slice())), @@ -168,12 +173,15 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result doc.set(r, Object::Stream(stored)), + Some(stored) => { + doc.set(r, Object::Stream(stored)); + stored_again += 1; + } None => stuck += 1, } } @@ -181,5 +189,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result Vec<(&'static str, [f64; 4], SharedPages)> { COVERS, SharedPages { first: format!("{plain} /SMask 10 0 R"), extra: vec![stream(&shared_image(SHARED), scan.page.clone())], ..base() }, ), + ( + "a stencil mask", + COVERS, + SharedPages { + first: format!("{plain} /Mask 10 0 R"), + extra: vec![stream( + &shared_image(SHARED).replace("/BitsPerComponent 1 /ColorSpace /DeviceGray", "/ImageMask true"), + scan.page.clone(), + )], + ..base() + }, + ), + ( + "parameters for data in another file", + COVERS, + SharedPages { first: shared_image("/F (scan.jb2) /FFilter /JBIG2Decode /FDecodeParms << /JBIG2Globals 9 0 R >>"), ..base() }, + ), ( "an alternate", COVERS, @@ -1686,6 +1703,30 @@ fn a_refused_redaction_counts_what_cant_be_stored() { assert_eq!(text.starts_with("something else in the file refers to JBIG2 symbol data"), stuck == 0, "{other}: {text}"); assert_eq!(text.contains("can't be stored"), stuck > 0, "{other}: {text}"); } + // An image whose data is in another file can't be stored here, though what is here decodes: readers + // draw the other file, with the dictionary its own parameters name. + let elsewhere = format!("{} /F (page2.jb2) /FFilter /JBIG2Decode /FDecodeParms << /JBIG2Globals 9 0 R >>", shared_image(SHARED)); + let mut doc = SharedPages { other: elsewhere, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(1)))); +} + +/// A dictionary that names itself as its own globals (malformed) can't be stored without them, but +/// leaves with what used it, so the run goes on; it isn't counted as stored. +#[test] +fn a_dictionary_that_names_itself_isnt_counted_as_stored() { + let scan = shared_scan(); + let mut doc = SharedPages::new(&scan).build(&scan); + let dictionary = pdfcraft_cos::ObjRef::new(9, 0); + let Object::Stream(mut s) = (*doc.get(dictionary)).clone() else { panic!("object 9 is the dictionary") }; + let mut parms = pdfcraft_cos::Dict::new(); + parms.set(b"JBIG2Globals".to_vec(), Object::Ref(dictionary)); + s.dict.set(b"DecodeParms".to_vec(), Object::Dict(parms)); + doc.set(dictionary, Object::Stream(s)); + mark(&mut doc, 0, &[CLEARS], ""); + assert_eq!(apply(&mut doc, None).unwrap().images_restored, 1); + assert_gone(&doc, b"LEAK_GLOBALS"); + assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits); } /// An image that names the dictionary and another one is stored again without both, but the other From 1b39d9a00cfe8650594bf633609860c2a7dc8761 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 10:39:01 +0100 Subject: [PATCH 06/27] M8: /F null is none, and no copy keeps the old data's parameters A /F that is null, by reference to null or to no object, is as if absent, so an image with one is stored again, not refused as data in another file. The check is image::data_elsewhere. A copy stored again kept /FFilter and /FDecodeParms, which named now reads, so the dictionary stayed through the copy and the run refused, blaming something else. A cleared copy kept those and /DP. Both drop them, the cleared copy only when its data isn't in another file. A dictionary that names itself isn't counted as something that uses one when the run is refused, as it already wasn't when it goes on. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 8 +++++-- crates/redact/src/image.rs | 18 ++++++++++++--- crates/redact/src/tests.rs | 44 ++++++++++++++++++++++++++++-------- 3 files changed, 56 insertions(+), 14 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 13bc851957..05b7c3bf0d 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -98,7 +98,7 @@ fn content(doc: &Document, num: u32) -> Option { /// says; for one whose data is in another file (`/F`), which readers draw instead of what is here; /// and for one that can't be decoded. fn without_dictionary(doc: &Document, s: &Stream) -> Option { - if s.dict.contains(b"F") { + if crate::image::data_elsewhere(doc, s) { return None; } let readable = readable(doc, s); @@ -120,7 +120,9 @@ fn without_dictionary(doc: &Document, s: &Stream) -> Option { let allowance = w.div_ceil(8).checked_mul(h)?.checked_add(h)?.checked_mul(3)?.checked_add(4096)?; let data = doc.decoded_image_within(&readable, allowance).ok()?; let mut dict = readable.dict.clone(); - for key in [b"Length".as_slice(), b"DecodeParms", b"DP"] { + // `/FFilter` and `/FDecodeParms` too: with no data in another file they mean nothing, but + // `named` reads them. + for key in [b"Length".as_slice(), b"DecodeParms", b"DP", b"FFilter", b"FDecodeParms"] { dict.remove(key); } dict.set(b"BitsPerComponent".to_vec(), Object::Int(1)); @@ -182,6 +184,8 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result {} None => stuck += 1, } } diff --git a/crates/redact/src/image.rs b/crates/redact/src/image.rs index 39308839b9..9935055159 100644 --- a/crates/redact/src/image.rs +++ b/crates/redact/src/image.rs @@ -163,12 +163,24 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] return Some(None); } let mut dict = s.dict.clone(); - dict.remove(b"Length"); - dict.remove(b"DecodeParms"); - dict.remove(b"Alternates"); + for key in [b"Length".as_slice(), b"DecodeParms", b"DP", b"Alternates"] { + dict.remove(key); + } + if !data_elsewhere(doc, s) { + // Parameters for data in another file mean nothing without `/F`, but may name a dictionary. + dict.remove(b"FFilter"); + dict.remove(b"FDecodeParms"); + } Some(Some(smallest(dict, &data, w, h, ncomp == 1 && bpc == 1, is_jbig2))) } +/// Whether `s`'s data is in another file, which a reader may draw instead of what is here: a `/F` +/// that isn't null (a null entry is as if absent, and a reference to no object is null, ISO +/// 32000-2 §7.3.7 and §7.3.10). +pub(crate) fn data_elsewhere(doc: &Document, s: &Stream) -> bool { + s.dict.get(b"F").is_some_and(|f| !matches!(&*doc.resolve(f), Object::Null)) +} + /// The cleared samples `data` of a `w` × `h` image as a stream with `dict`: Flate, or, for one bit /// of one component (`bilevel`, a scan as a rule, which Flate stores several times larger), CCITT /// Group 4, or JBIG2 when the image was JBIG2 (`jbig2`; JBIG2 is PDF 1.4, which such a file already diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index e40fcaabbe..4e9a9f1fd7 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1469,6 +1469,13 @@ fn ways_out(scan: &SharedScan) -> Vec<(&'static str, [f64; 4], SharedPages)> { }, ), ("/DP", COVERS, SharedPages { first: shared_image("/DP << /JBIG2Globals 9 0 R >>"), ..base() }), + // Cleared, the copy keeps no parameters of the old data, so none names the dictionary. + ("/DP, cleared", CLEARS, SharedPages { first: format!("{} /DP << /JBIG2Globals 9 0 R >>", shared_image(SHARED)), ..base() }), + ( + "parameters for data in another file, with none, cleared", + CLEARS, + SharedPages { first: format!("{} /FFilter /JBIG2Decode /FDecodeParms << /JBIG2Globals 9 0 R >>", shared_image(SHARED)), ..base() }, + ), ("another generation", COVERS, SharedPages { first: shared_image("/DecodeParms << /JBIG2Globals 9 1 R >>"), ..base() }), ] } @@ -1526,6 +1533,17 @@ fn what_stays_is_stored_without_the_dictionary_however_it_names_it() { ), // Parameters no filter reads, on a picture that needs no dictionary. (shared_image("/DP << /JBIG2Globals 9 0 R >>"), pdfcraft_filters::encode_jbig2_generic(&scan.bits, 400, 40).unwrap(), vec![]), + // Those for data in another file, with none: beside the parameters it reads, and alone. + (format!("{image} /FFilter /JBIG2Decode /FDecodeParms << /JBIG2Globals 9 0 R >>"), scan.page.clone(), vec![]), + ( + shared_image("/FFilter /JBIG2Decode /FDecodeParms << /JBIG2Globals 9 0 R >>"), + pdfcraft_filters::encode_jbig2_generic(&scan.bits, 400, 40).unwrap(), + vec![], + ), + // A `/F` that is null, directly, by reference, or as a reference to no object, is none. + (format!("{image} /F null"), scan.page.clone(), vec![]), + (format!("{image} /F 10 0 R"), scan.page.clone(), vec![b"null".to_vec()]), + (format!("{image} /F 99 0 R"), scan.page.clone(), vec![]), ] { let mut doc = SharedPages { other: other.clone(), other_data: data, extra, ..SharedPages::new(&scan) }.build(&scan); mark(&mut doc, 0, &[CLEARS], ""); @@ -1712,21 +1730,29 @@ fn a_refused_redaction_counts_what_cant_be_stored() { } /// A dictionary that names itself as its own globals (malformed) can't be stored without them, but -/// leaves with what used it, so the run goes on; it isn't counted as stored. +/// leaves with what used it, so the run goes on; it isn't counted as stored, nor, when the run is +/// refused, as something that uses the dictionary. #[test] fn a_dictionary_that_names_itself_isnt_counted_as_stored() { let scan = shared_scan(); - let mut doc = SharedPages::new(&scan).build(&scan); - let dictionary = pdfcraft_cos::ObjRef::new(9, 0); - let Object::Stream(mut s) = (*doc.get(dictionary)).clone() else { panic!("object 9 is the dictionary") }; - let mut parms = pdfcraft_cos::Dict::new(); - parms.set(b"JBIG2Globals".to_vec(), Object::Ref(dictionary)); - s.dict.set(b"DecodeParms".to_vec(), Object::Dict(parms)); - doc.set(dictionary, Object::Stream(s)); - mark(&mut doc, 0, &[CLEARS], ""); + let names_itself = |pages: SharedPages| { + let mut doc = pages.build(&scan); + let dictionary = pdfcraft_cos::ObjRef::new(9, 0); + let Object::Stream(mut s) = (*doc.get(dictionary)).clone() else { panic!("object 9 is the dictionary") }; + let mut parms = pdfcraft_cos::Dict::new(); + parms.set(b"JBIG2Globals".to_vec(), Object::Ref(dictionary)); + s.dict.set(b"DecodeParms".to_vec(), Object::Dict(parms)); + doc.set(dictionary, Object::Stream(s)); + mark(&mut doc, 0, &[CLEARS], ""); + doc + }; + let mut doc = names_itself(SharedPages::new(&scan)); assert_eq!(apply(&mut doc, None).unwrap().images_restored, 1); assert_gone(&doc, b"LEAK_GLOBALS"); assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits); + // The second page's image has its data in another file, so it can't be stored: one item. + let mut doc = names_itself(SharedPages { other: format!("{} /F (p.jb2)", shared_image(SHARED)), ..SharedPages::new(&scan) }); + assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(1)))); } /// An image that names the dictionary and another one is stored again without both, but the other From f0ff1f7fe6f0cd2c802a0020c93e9bfe33c3712a Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 11:56:56 +0100 Subject: [PATCH 07/27] M8: values given through a chain of references are read as their object /F, /Width, /Height, /Filter and its entries, /ColorSpace and an ICC profile were resolved one reference deep, while /DecodeParms and /JBIG2Globals follow a chain (followed). So /F through a reference to a reference to null counted as data in another file and refused the run, and a /ColorSpace chained to /DeviceRGB wasn't known to have three components, so a JBIG2 image naming it was stored as one bit, where main refused it. All now go through followed, which a ring ends. image::components serves clear too, so an image whose colour space is given that way is now cleared, where it was removed whole; clear's /Decode for a stencil mask, read one reference deep, follows the chain as well, so a chained [1 0] isn't missed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 8 ++++---- crates/redact/src/image.rs | 17 +++++++++-------- crates/redact/src/tests.rs | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 05b7c3bf0d..c11388b64a 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -21,7 +21,7 @@ pub(crate) fn kept(doc: &Document) -> HashSet { } /// `o`, or what references lead to from it. A ring of them ends as a reference, which is no dictionary. -fn followed(doc: &Document, o: &Object) -> Arc { +pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { let mut at = doc.resolve(o); for _ in 0..MAX_HOPS { let next = match &*at { @@ -60,10 +60,10 @@ fn named(doc: &Document, s: &Stream) -> Vec { found } -/// `v`, or what it refers to, inside an array too. +/// `v`, or what references lead to from it, inside an array too. fn direct(doc: &Document, v: &Object) -> Object { - match &*doc.resolve(v) { - Object::Array(a) => Object::Array(a.iter().map(|e| (*doc.resolve(e)).clone()).collect()), + match &*followed(doc, v) { + Object::Array(a) => Object::Array(a.iter().map(|e| (*followed(doc, e)).clone()).collect()), other => other.clone(), } } diff --git a/crates/redact/src/image.rs b/crates/redact/src/image.rs index 9935055159..f429729929 100644 --- a/crates/redact/src/image.rs +++ b/crates/redact/src/image.rs @@ -13,8 +13,9 @@ use pdfcraft_cos::{Dict, Document, Object, Stream}; /// Most pixels an image may have to be cleared in place (larger ones are removed). pub(crate) const MAX_PIXELS: u64 = 64 << 20; +/// The components of image `s`'s colour space, through any references (`None` when unknown). pub(crate) fn components(doc: &Document, s: &Stream) -> Option { - let cs = doc.resolve(s.dict.get(b"ColorSpace")?); + let cs = crate::globals::followed(doc, s.dict.get(b"ColorSpace")?); let cs = &*cs; let name = match cs { Object::Name(n) => n.as_slice(), @@ -27,7 +28,7 @@ pub(crate) fn components(doc: &Document, s: &Stream) -> Option { b"DeviceCMYK" | b"CMYK" => 4, b"ICCBased" => { // /N lives in the ICC profile stream, and shall be 1, 3 or 4 (ISO 32000-2 §8.6.5.5). - match &*doc.resolve(cs.as_array()?.get(1)?) { + match &*crate::globals::followed(doc, cs.as_array()?.get(1)?) { Object::Stream(icc) => usize::try_from(icc.dict.int(b"N")?).ok().filter(|n| matches!(n, 1 | 3 | 4))?, _ => return None, } @@ -119,9 +120,9 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] data.truncate(bytes); // Image masks: a sample of 0 paints unless /Decode is [1 0]; cleared samples must not paint. let clear_bit = if mask { - // The array, and its first number, given by reference too. - let decode = s.dict.get(b"Decode").map(|d| doc.resolve(d)); - let first = decode.as_deref().and_then(Object::as_array).and_then(|d| d.first()).map(|v| doc.resolve(v)); + // The array, and its first number, given by reference too, through any references. + let decode = s.dict.get(b"Decode").map(|d| crate::globals::followed(doc, d)); + let first = decode.as_deref().and_then(Object::as_array).and_then(|d| d.first()).map(|v| crate::globals::followed(doc, v)); let inverted = first.as_deref().and_then(Object::as_f64) == Some(1.0); !inverted } else { @@ -175,10 +176,10 @@ fn decode_and_clear(doc: &Document, s: &Stream, ctm: &Matrix, rects: &[[f64; 4]] } /// Whether `s`'s data is in another file, which a reader may draw instead of what is here: a `/F` -/// that isn't null (a null entry is as if absent, and a reference to no object is null, ISO -/// 32000-2 §7.3.7 and §7.3.10). +/// that isn't null, through any references (a null entry is as if absent, and a reference to no +/// object is null, ISO 32000-2 §7.3.7 and §7.3.10). A ring of references counts as a file. pub(crate) fn data_elsewhere(doc: &Document, s: &Stream) -> bool { - s.dict.get(b"F").is_some_and(|f| !matches!(&*doc.resolve(f), Object::Null)) + s.dict.get(b"F").is_some_and(|f| !matches!(&*crate::globals::followed(doc, f), Object::Null)) } /// The cleared samples `data` of a `w` × `h` image as a stream with `dict`: Flate, or, for one bit diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 4e9a9f1fd7..171138c3a9 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -620,6 +620,25 @@ fn a_fax_image_has_its_covered_pixels_cleared_instead_of_being_removed() { vec![b"1".to_vec()], [0x00, 0xFF, 0xFF, 0xFF], ), + // Through a reference to a reference: the array, its first number, and a colour space. + ( + format!("{image} /ImageMask true /Decode 8 0 R /Filter /CCITTFaxDecode /DecodeParms {parms}"), + g4.clone(), + vec![b"9 0 R".to_vec(), b"[1 0]".to_vec()], + [0x00, 0xFF, 0xFF, 0xFF], + ), + ( + format!("{image} /ImageMask true /Decode [8 0 R 0] /Filter /CCITTFaxDecode /DecodeParms {parms}"), + g4.clone(), + vec![b"9 0 R".to_vec(), b"1".to_vec()], + [0x00, 0xFF, 0xFF, 0xFF], + ), + ( + format!("{image} /ColorSpace 8 0 R /Filter /CCITTFaxDecode /DecodeParms {parms}"), + g4.clone(), + vec![b"9 0 R".to_vec(), b"/DeviceGray".to_vec()], + [0x00, 0xFF, 0xFF, 0xFF], + ), ] { let mut objs = vec![stream(&dict, data)]; objs.extend(extra); @@ -1544,6 +1563,17 @@ fn what_stays_is_stored_without_the_dictionary_however_it_names_it() { (format!("{image} /F null"), scan.page.clone(), vec![]), (format!("{image} /F 10 0 R"), scan.page.clone(), vec![b"null".to_vec()]), (format!("{image} /F 99 0 R"), scan.page.clone(), vec![]), + // Through a reference to a reference: /F to null, the size, the filter, the colour space. + (format!("{image} /F 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"null".to_vec()]), + (image.replace("/Width 400", "/Width 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"400".to_vec()]), + (image.replace("/Height 40", "/Height 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"40".to_vec()]), + (image.replace("/Filter /JBIG2Decode", "/Filter 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"/JBIG2Decode".to_vec()]), + ( + shared_image("/DecodeParms [null << /JBIG2Globals 9 0 R >>]").replace("/Filter /JBIG2Decode", "/Filter [/ASCIIHexDecode 10 0 R]"), + hex.clone(), + vec![b"11 0 R".to_vec(), b"/JBIG2Decode".to_vec()], + ), + (image.replace("/ColorSpace /DeviceGray", "/ColorSpace 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"/DeviceGray".to_vec()]), ] { let mut doc = SharedPages { other: other.clone(), other_data: data, extra, ..SharedPages::new(&scan) }.build(&scan); mark(&mut doc, 0, &[CLEARS], ""); @@ -1641,6 +1671,12 @@ fn a_shared_jbig2_dictionary_that_cant_leave_refuses_the_redaction() { for (other, data, extra) in [ (image.replace("/Height 40", "/Height 41"), scan.page.clone(), vec![]), (image.replace("/DeviceGray", "/DeviceRGB"), scan.page.clone(), vec![]), + (image.replace("/ColorSpace /DeviceGray", "/ColorSpace 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"/DeviceRGB".to_vec()]), + ( + format!("{image} /ColorSpace [/ICCBased 10 0 R]").replace("/ColorSpace /DeviceGray ", ""), + scan.page.clone(), + vec![b"11 0 R".to_vec(), stream("/N 3", b"")], + ), (shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"), scan.page.clone(), vec![b"9 0 R".to_vec()]), (elsewhere.to_string(), vec![0], vec![]), (not_jbig2, pdfcraft_filters::encode_flate(&scan.bits), vec![]), From 2eddb1bc987c3b105a6f3f612b5e77adce809332 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 12:18:35 +0100 Subject: [PATCH 08/27] M8: a copy is found whatever way its parameters are given content() decoded a dictionary through with_direct_parms, which reads /DecodeParms and its entries one reference deep, while named() follows the chain. So a copy of the dictionary whose parameters came through a reference to a reference decoded to other bytes, wasn't known for a copy, and stayed in the file with Ok. readable() now gives /DecodeParms, each entry and each value through followed, /JBIG2Globals aside, so a copy decodes as a reader that follows references decodes it. With the copy known, the image naming it has to be stored again, and PdfCraft decodes that image's symbols with the copy's parameters one reference deep, so it can't: the run is refused, where it went on with the copy in the file. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 25 +++++++++++++++++++++++- crates/redact/src/tests.rs | 38 ++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index c11388b64a..b095059a7e 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -68,7 +68,27 @@ fn direct(doc: &Document, v: &Object) -> Object { } } -/// `s` with what `decoded_image` and `decoded_within` read as given made direct: its size and filters. +/// `/DecodeParms` as a reader takes them, through any references: the array or dictionary, each +/// entry, and each value made direct, but `/JBIG2Globals`, which stays the reference it is. +fn direct_parms(doc: &Document, v: &Object) -> Object { + let entry = |e: &Object| match &*followed(doc, e) { + Object::Dict(d) => { + let mut made = d.clone(); + for (key, value) in d.iter().filter(|(key, _)| key.as_slice() != b"JBIG2Globals") { + made.set(key.to_vec(), (*followed(doc, value)).clone()); + } + Object::Dict(made) + } + other => other.clone(), + }; + match &*followed(doc, v) { + Object::Array(a) => Object::Array(a.iter().map(entry).collect()), + other => entry(other), + } +} + +/// `s` with what `decoded_image` and `decoded_within` read as given made direct: its size, filters +/// and their parameters. fn readable(doc: &Document, s: &Stream) -> Stream { let mut readable = s.clone(); for key in [b"Width".as_slice(), b"Height", b"Filter"] { @@ -76,6 +96,9 @@ fn readable(doc: &Document, s: &Stream) -> Stream { readable.dict.set(key.to_vec(), direct(doc, v)); } } + if let Some(v) = s.dict.get(b"DecodeParms") { + readable.dict.set(b"DecodeParms".to_vec(), direct_parms(doc, v)); + } readable } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 171138c3a9..48d00da332 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1791,6 +1791,44 @@ fn a_dictionary_that_names_itself_isnt_counted_as_stored() { assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(1)))); } +/// A copy of the dictionary stored with a predictor is found whatever way its parameters are given, +/// through a chain of references among them, as a reader that follows them decodes it. Through a +/// chain, PdfCraft can't decode the image with the copy's symbols to store it again, so the run is +/// refused; it never goes on with the copy left in the file. +#[test] +fn a_copy_is_found_however_its_parameters_are_given() { + let scan = shared_scan(); + let globals = [jbig2_globals(b"LEAK_GLOBALS"), scan.symbols.clone()].concat(); + // PNG rows of one byte, each with no prediction: a byte for the row's tag, then the byte. + let predicted: Vec = globals.iter().flat_map(|b| [0u8, *b]).collect(); + let flate = pdfcraft_filters::encode_flate(&predicted); + let parms = b"<< /Predictor 12 /Columns 1 >>".to_vec(); + for (spelling, extra, stored) in [ + ("/DecodeParms << /Predictor 12 /Columns 1 >>", vec![], true), + ("/DecodeParms 11 0 R", vec![parms.clone()], true), + ("/DecodeParms 11 0 R", vec![b"12 0 R".to_vec(), parms.clone()], false), + ("/DecodeParms [11 0 R]", vec![b"12 0 R".to_vec(), parms.clone()], false), + ("/DecodeParms << /Predictor 11 0 R /Columns 1 >>", vec![b"12 0 R".to_vec(), b"12".to_vec()], false), + ] { + let mut objects = vec![stream(&format!("/Filter /FlateDecode {spelling}"), flate.clone())]; + objects.extend(extra.clone()); + let other = shared_image("/DecodeParms << /JBIG2Globals 10 0 R >>"); + let mut doc = SharedPages { other, extra: objects, ..SharedPages::new(&scan) }.build(&scan); + mark(&mut doc, 0, &[CLEARS], ""); + let at = format!("{spelling} {}", extra.len()); + match apply(&mut doc, None) { + Ok(report) => { + assert!(stored, "{at}: went on"); + assert_eq!(report.images_restored, 1, "{at}"); + let kept: std::collections::HashSet = pdfcraft_cos::reachable(&doc).into_iter().map(|r| r.num).collect(); + assert!(!kept.contains(&10) && !kept.contains(&9), "{at}: the copy or the dictionary stays"); + assert_eq!(drawn_samples(&reopen(&doc), 1), scan.bits, "{at}"); + } + Err(e) => assert!(!stored && matches!(e, RedactError::SharedJbig2Globals(1)), "{at}: {e:?}"), + } + } +} + /// An image that names the dictionary and another one is stored again without both, but the other /// was shared with nothing that left: an image that uses only it stays as it was, whichever of the /// two comes first in the file, and so does that dictionary. From 4b57bd1eb6a635008169440593156d810c930e6d Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 13:03:25 +0100 Subject: [PATCH 09/27] M8: what is made direct to read a stream is small, however often it is named direct_parms copied every value it followed, and direct() every /Filter entry, so each reference to one large array copied it again: about 5.5 MB a reference, 11 GB from a file of about 225 KB. Now, as Document::with_direct_parms does, a size is a number, /Filter a name or an array of names, and direct_parms() keeps of each entry the numbers and flags decoders read (DECODE_PARMS_KEYS, now public in cos) and /JBIG2Globals as given, for as many entries as there are filters. Anything else is null. A kept image whose own /DecodeParms come through a chain is stored again with them, which 2eddb1bc9 made so: what_stays tests it now. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/cos/src/document.rs | 1 + crates/cos/src/document/objects.rs | 2 +- crates/cos/src/lib.rs | 2 +- crates/redact/src/globals.rs | 62 ++++++++++++++++++++++-------- crates/redact/src/tests.rs | 22 +++++++++++ 5 files changed, 70 insertions(+), 19 deletions(-) diff --git a/crates/cos/src/document.rs b/crates/cos/src/document.rs index 082d1910d1..0508c4024c 100644 --- a/crates/cos/src/document.rs +++ b/crates/cos/src/document.rs @@ -19,6 +19,7 @@ mod reader; mod state; mod xref; +pub use objects::DECODE_PARMS_KEYS; #[cfg(test)] pub(crate) use reader::find; #[cfg(test)] diff --git a/crates/cos/src/document/objects.rs b/crates/cos/src/document/objects.rs index 39a4d9a6da..8f2e7a973f 100644 --- a/crates/cos/src/document/objects.rs +++ b/crates/cos/src/document/objects.rs @@ -781,7 +781,7 @@ impl Document { /// The `/DecodeParms` entries the decoders read (ISO 32000-2 Tables 8, 11 and 13: the Flate and /// LZW predictors, CCITT, DCT's colour transform): numbers and flags a file may give by /// reference, which [`Dict::int`] doesn't follow. -const DECODE_PARMS_KEYS: [&[u8]; 13] = [ +pub const DECODE_PARMS_KEYS: [&[u8]; 13] = [ b"Predictor", b"Colors", b"BitsPerComponent", diff --git a/crates/cos/src/lib.rs b/crates/cos/src/lib.rs index 76dc0b6fe7..9d45d8f982 100644 --- a/crates/cos/src/lib.rs +++ b/crates/cos/src/lib.rs @@ -24,7 +24,7 @@ mod writer; pub use budget::{DecodeBudget, MAX_PAGE_DECODED, MAX_PAGE_FORMS}; pub use bytes::Bytes; -pub use document::{Document, ReadState, Revision, SourceIdentity, XrefEntry}; +pub use document::{DECODE_PARMS_KEYS, Document, ReadState, Revision, SourceIdentity, XrefEntry}; pub use object::{Dict, MAX_DECODED, MAX_JBIG2_GLOBALS, Name, ObjRef, Object, PdfString, Stream}; pub use parser::{Lexer, is_whitespace, parse_indirect}; pub use pdfcraft_crypt::{ diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index b095059a7e..476b3d955b 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -9,7 +9,7 @@ use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; use std::sync::Arc; -use pdfcraft_cos::{Document, MAX_JBIG2_GLOBALS, Object, Stream, reachable}; +use pdfcraft_cos::{DECODE_PARMS_KEYS, Dict, Document, MAX_JBIG2_GLOBALS, Object, Stream, reachable}; /// The references followed from `/JBIG2Globals` towards a stream, so that a ring of them ends. const MAX_HOPS: usize = 32; @@ -60,44 +60,72 @@ fn named(doc: &Document, s: &Stream) -> Vec { found } -/// `v`, or what references lead to from it, inside an array too. -fn direct(doc: &Document, v: &Object) -> Object { +/// A `/Filter` through any references, as a decoder reads it: a name, or an array of names, each +/// entry through references too. Anything else is null, so nothing large is copied however often +/// the file refers to it. +fn direct_filter(doc: &Document, v: &Object) -> Object { + let name = |o: &Object| if matches!(o, Object::Name(_)) { o.clone() } else { Object::Null }; match &*followed(doc, v) { - Object::Array(a) => Object::Array(a.iter().map(|e| (*followed(doc, e)).clone()).collect()), - other => other.clone(), + Object::Array(a) => Object::Array(a.iter().map(|e| name(&followed(doc, e))).collect()), + other => name(other), } } -/// `/DecodeParms` as a reader takes them, through any references: the array or dictionary, each -/// entry, and each value made direct, but `/JBIG2Globals`, which stays the reference it is. -fn direct_parms(doc: &Document, v: &Object) -> Object { +/// A size through any references: a number, or null. +fn direct_number(doc: &Document, v: &Object) -> Object { + match &*followed(doc, v) { + number @ (Object::Int(_) | Object::Real(_)) => number.clone(), + _ => Object::Null, + } +} + +/// `/DecodeParms` as a reader takes them, through any references, for `filters` filters: of each +/// entry, the numbers and flags the decoders read ([`DECODE_PARMS_KEYS`]), and `/JBIG2Globals` as +/// given. An entry that isn't a dictionary is null. As `Document::with_direct_parms`, nothing large +/// is copied however often the file refers to it. +fn direct_parms(doc: &Document, v: &Object, filters: usize) -> Object { let entry = |e: &Object| match &*followed(doc, e) { Object::Dict(d) => { - let mut made = d.clone(); - for (key, value) in d.iter().filter(|(key, _)| key.as_slice() != b"JBIG2Globals") { - made.set(key.to_vec(), (*followed(doc, value)).clone()); + let mut made = Dict::new(); + for key in DECODE_PARMS_KEYS { + let Some(value) = d.get(key) else { continue }; + let value = followed(doc, value); + if matches!(*value, Object::Int(_) | Object::Real(_) | Object::Bool(_)) { + made.set(key.to_vec(), (*value).clone()); + } + } + if let Some(globals) = d.get(b"JBIG2Globals") { + made.set(b"JBIG2Globals".to_vec(), globals.clone()); } Object::Dict(made) } - other => other.clone(), + _ => Object::Null, }; match &*followed(doc, v) { - Object::Array(a) => Object::Array(a.iter().map(entry).collect()), + Object::Array(a) => Object::Array(a.iter().take(filters).map(entry).collect()), other => entry(other), } } /// `s` with what `decoded_image` and `decoded_within` read as given made direct: its size, filters /// and their parameters. -fn readable(doc: &Document, s: &Stream) -> Stream { +pub(crate) fn readable(doc: &Document, s: &Stream) -> Stream { let mut readable = s.clone(); - for key in [b"Width".as_slice(), b"Height", b"Filter"] { + for key in [b"Width".as_slice(), b"Height"] { if let Some(v) = s.dict.get(key) { - readable.dict.set(key.to_vec(), direct(doc, v)); + readable.dict.set(key.to_vec(), direct_number(doc, v)); } } + if let Some(v) = s.dict.get(b"Filter") { + readable.dict.set(b"Filter".to_vec(), direct_filter(doc, v)); + } if let Some(v) = s.dict.get(b"DecodeParms") { - readable.dict.set(b"DecodeParms".to_vec(), direct_parms(doc, v)); + let filters = match readable.dict.get(b"Filter") { + Some(Object::Array(a)) => a.len(), + Some(_) => 1, + None => 0, + }; + readable.dict.set(b"DecodeParms".to_vec(), direct_parms(doc, v, filters)); } readable } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 48d00da332..eb1d1d96dd 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1565,6 +1565,7 @@ fn what_stays_is_stored_without_the_dictionary_however_it_names_it() { (format!("{image} /F 99 0 R"), scan.page.clone(), vec![]), // Through a reference to a reference: /F to null, the size, the filter, the colour space. (format!("{image} /F 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"null".to_vec()]), + (shared_image("/DecodeParms 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"<< /JBIG2Globals 9 0 R >>".to_vec()]), (image.replace("/Width 400", "/Width 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"400".to_vec()]), (image.replace("/Height 40", "/Height 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"40".to_vec()]), (image.replace("/Filter /JBIG2Decode", "/Filter 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"/JBIG2Decode".to_vec()]), @@ -1791,6 +1792,27 @@ fn a_dictionary_that_names_itself_isnt_counted_as_stored() { assert!(matches!(apply(&mut doc, None), Err(RedactError::SharedJbig2Globals(1)))); } +/// What a stream's size, filters and parameters are made direct to is small, however often they +/// refer to a large object: only numbers, names and the parameters decoders read are copied. +#[test] +fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { + let big = format!("[{}]", "1 ".repeat(100_000)); + let keys: String = (0..50).map(|k| format!(" /K{k} 2 0 R")).collect(); + // Three filters, and parameters for 2,001: the entries past the filters are dropped. + let (filters, entries) = (" 2 0 R".repeat(2), " 2 0 R".repeat(2000)); + let dict = + format!("/Width 2 0 R /Height 2 0 R /Filter [/FlateDecode{filters}] /DecodeParms [<< /Predictor 3 0 R /Columns 2 0 R{keys} >>{entries}]"); + let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), big.into_bytes(), b"4 0 R".to_vec(), b"12".to_vec(), stream(&dict, b"")]); + let Object::Stream(s) = &*doc.get(ObjRef::new(5, 0)) else { panic!("object 5 is the stream") }; + let made = crate::globals::readable(&doc, s); + let mut bytes = Vec::new(); + pdfcraft_cos::serialize(&Object::Dict(made.dict.clone()), &mut bytes); + assert!(bytes.len() < 4096, "{} bytes made direct", bytes.len()); + let parms = made.dict.get(b"DecodeParms").and_then(Object::as_array).unwrap(); + assert_eq!(parms.first().and_then(Object::as_dict).and_then(|d| d.get(b"Predictor")), Some(&Object::Int(12))); + assert_eq!(parms.len(), 3, "an entry for each filter"); +} + /// A copy of the dictionary stored with a predictor is found whatever way its parameters are given, /// through a chain of references among them, as a reader that follows them decodes it. Through a /// chain, PdfCraft can't decode the image with the copy's symbols to store it again, so the run is From 4acc7e040f112a62d76fa771d858b0bcaf477de2 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 13:25:31 +0100 Subject: [PATCH 10/27] M8: what streams share is read once a run, and nothing large is copied for each Three ways a small file could still make a redaction run long or large: - direct_parms kept /JBIG2Globals as given, so a direct array there was copied for each entry (427 MiB from 32 KB). It is kept only when it is a reference, as Document::with_direct_parms keeps it; a direct value is no stream, so no decoder reads it. - The entries copied were bounded only by /Filter's length, and the work repeated for every stream that uses a dictionary: time grew with the square of the file (117 s from 249 KB). A /Filter longer than any decoder takes (MAX_CHAIN, and a /Crypt) is read as no filter, so its parameters aren't read either; no decoder reads such a stream anyway. - named() read every entry of every stream's parameters, through up to 33 references each, with nothing shared between streams. Names keeps what each value the file refers to names, for the whole run, so streams sharing an array or an entry cost it once; unshare adds a shared list, and finds what it matches, once too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 130 ++++++++++++++++++++++++++--------- crates/redact/src/tests.rs | 47 ++++++++++++- 2 files changed, 143 insertions(+), 34 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 476b3d955b..735bbc3667 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -7,6 +7,7 @@ use std::collections::hash_map::DefaultHasher; use std::collections::{HashMap, HashSet}; use std::hash::{Hash, Hasher}; +use std::rc::Rc; use std::sync::Arc; use pdfcraft_cos::{DECODE_PARMS_KEYS, Dict, Document, MAX_JBIG2_GLOBALS, Object, Stream, reachable}; @@ -33,39 +34,75 @@ pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { at } -/// Every object `s` names as `/JBIG2Globals`, and each reference on the way to one: in any entry of -/// its parameters, whichever filter that entry belongs to and however the filters are given. A +/// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the +/// file refers to, by its object number and whether it is read as a list of entries. +pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); + +/// Every object a stream names as `/JBIG2Globals`, and each reference on the way to one: in any entry +/// of its parameters, whichever filter that entry belongs to and however the filters are given. A /// reader may use less of this, but leaving a dictionary behind is the failure that matters here. -fn named(doc: &Document, s: &Stream) -> Vec { - let mut found = Vec::new(); - let mut read = |entry: &Object| { - let held = followed(doc, entry); - let Some(mut at) = held.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return }; - for _ in 0..=MAX_HOPS { - found.push(at.num); - match &*doc.get(at) { - Object::Ref(next) => at = *next, - _ => return, - } +/// +/// What a value the file refers to names is read once a run, so streams that share one, by +/// reference or through an entry, cost it once however many there are. +#[derive(Default)] +pub(crate) struct Names { + memo: HashMap<(u32, bool), Rc<[u32]>>, + /// How many values were read rather than remembered. + pub(crate) read: usize, +} + +impl Names { + /// What `s` names, one list for each key that gives it parameters. + pub(crate) fn of(&mut self, doc: &Document, s: &Stream) -> Vec { + // `/FDecodeParms` too: those of data in another file, which a reader may draw instead. + [b"DecodeParms".as_slice(), b"DP", b"FDecodeParms"].into_iter().filter_map(|key| s.dict.get(key)).map(|v| self.value(doc, v, true)).collect() + } + + /// What `v` names: one entry, or (`list`) an array of them or one. + fn value(&mut self, doc: &Document, v: &Object, list: bool) -> Listed { + let key = v.as_ref().map(|r| (r.num, list)); + if let Some(hit) = key.and_then(|k| self.memo.get(&k)) { + return (hit.clone(), key); } - }; - // `/FDecodeParms` too: those of data in another file, which a reader may draw instead. - for key in [b"DecodeParms".as_slice(), b"DP", b"FDecodeParms"] { - let Some(parms) = s.dict.get(key).map(|p| followed(doc, p)) else { continue }; - match &*parms { - Object::Array(entries) => entries.iter().for_each(&mut read), - other => read(other), + self.read += 1; + let names: Rc<[u32]> = match (&*followed(doc, v), list) { + (Object::Array(entries), true) => { + let mut all: Vec = entries.iter().flat_map(|e| self.value(doc, e, false).0.to_vec()).collect(); + all.sort_unstable(); + all.dedup(); + all.into() + } + (entry, _) => entry_names(doc, entry).into(), + }; + if let Some(k) = key { + self.memo.insert(k, names.clone()); + } + (names, key) + } +} + +/// What one parameters entry names as `/JBIG2Globals`, and each reference on the way to it. +fn entry_names(doc: &Document, entry: &Object) -> Vec { + let mut found = Vec::new(); + let Some(mut at) = entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return found }; + for _ in 0..=MAX_HOPS { + found.push(at.num); + match &*doc.get(at) { + Object::Ref(next) => at = *next, + _ => break, } } found } /// A `/Filter` through any references, as a decoder reads it: a name, or an array of names, each -/// entry through references too. Anything else is null, so nothing large is copied however often -/// the file refers to it. +/// entry through references too. Anything else is null, a chain longer than any decoder takes +/// (`MAX_CHAIN`, and a `/Crypt`) included, so nothing large is copied however often the file refers +/// to it, and a stream that names it is read as one with no filter. fn direct_filter(doc: &Document, v: &Object) -> Object { let name = |o: &Object| if matches!(o, Object::Name(_)) { o.clone() } else { Object::Null }; match &*followed(doc, v) { + Object::Array(a) if a.len() > pdfcraft_filters::MAX_CHAIN + 1 => Object::Null, Object::Array(a) => Object::Array(a.iter().map(|e| name(&followed(doc, e))).collect()), other => name(other), } @@ -80,8 +117,8 @@ fn direct_number(doc: &Document, v: &Object) -> Object { } /// `/DecodeParms` as a reader takes them, through any references, for `filters` filters: of each -/// entry, the numbers and flags the decoders read ([`DECODE_PARMS_KEYS`]), and `/JBIG2Globals` as -/// given. An entry that isn't a dictionary is null. As `Document::with_direct_parms`, nothing large +/// entry, the numbers and flags the decoders read ([`DECODE_PARMS_KEYS`]), and `/JBIG2Globals` when it +/// is a reference. An entry that isn't a dictionary is null. As `Document::with_direct_parms`, nothing large /// is copied however often the file refers to it. fn direct_parms(doc: &Document, v: &Object, filters: usize) -> Object { let entry = |e: &Object| match &*followed(doc, e) { @@ -94,7 +131,9 @@ fn direct_parms(doc: &Document, v: &Object, filters: usize) -> Object { made.set(key.to_vec(), (*value).clone()); } } - if let Some(globals) = d.get(b"JBIG2Globals") { + // As a reference only, as `Document::with_direct_parms` keeps it: a direct value is no + // stream, so no decoder reads it, and it may be large. + if let Some(globals @ Object::Ref(_)) = d.get(b"JBIG2Globals") { made.set(b"JBIG2Globals".to_vec(), globals.clone()); } Object::Dict(made) @@ -122,8 +161,8 @@ pub(crate) fn readable(doc: &Document, s: &Stream) -> Stream { if let Some(v) = s.dict.get(b"DecodeParms") { let filters = match readable.dict.get(b"Filter") { Some(Object::Array(a)) => a.len(), - Some(_) => 1, - None => 0, + Some(Object::Name(_)) => 1, + _ => 0, }; readable.dict.set(b"DecodeParms".to_vec(), direct_parms(doc, v, filters)); } @@ -196,12 +235,19 @@ fn without_dictionary(doc: &Document, s: &Stream) -> Option { /// changed by then: a caller works on a copy it can drop. pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result { let now = kept(doc); + let mut names = Names::default(); let mut dictionaries: HashSet = HashSet::new(); + // A list the file refers to is added once, however many streams share it. + let mut added: HashSet<(u32, bool)> = HashSet::new(); for &num in before.difference(&now) { if let Ok(object) = doc.try_get(num) && let Object::Stream(s) = &*object { - dictionaries.extend(named(doc, s)); + for (list, key) in names.of(doc, s) { + if key.is_none_or(|k| added.insert(k)) { + dictionaries.extend(list.iter()); + } + } } } if dictionaries.is_empty() { @@ -209,6 +255,11 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result = dictionaries.iter().filter_map(|n| content(doc, *n)).collect(); let mut read: HashMap> = HashMap::new(); + // Whether a name matches doesn't depend on order: it is a dictionary that left, or a copy of one + // (what is added below is only ever such a copy). So what a shared list matches is found once. + let left = dictionaries.clone(); + let mut matches: HashMap<(u32, bool), Rc<[u32]>> = HashMap::new(); + added.clear(); let mut users = Vec::new(); for (r, object) in doc.scan_objects() { let Object::Stream(s) = &*object else { continue }; @@ -220,9 +271,26 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result = named(doc, s).into_iter().filter(|n| dictionaries.contains(n) || copy(n, &mut read)).collect(); - if !matched.is_empty() { - dictionaries.extend(matched); + let mut uses = false; + for (list, key) in names.of(doc, s) { + let matched = match key.and_then(|k| matches.get(&k)) { + Some(hit) => hit.clone(), + None => { + let found: Rc<[u32]> = list.iter().copied().filter(|n| left.contains(n) || copy(n, &mut read)).collect(); + if let Some(k) = key { + matches.insert(k, found.clone()); + } + found + } + }; + if !matched.is_empty() { + uses = true; + if key.is_none_or(|k| added.insert(k)) { + dictionaries.extend(matched.iter()); + } + } + } + if uses { users.push(r); } } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index eb1d1d96dd..dd1d9b4b43 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1798,11 +1798,25 @@ fn a_dictionary_that_names_itself_isnt_counted_as_stored() { fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { let big = format!("[{}]", "1 ".repeat(100_000)); let keys: String = (0..50).map(|k| format!(" /K{k} 2 0 R")).collect(); - // Three filters, and parameters for 2,001: the entries past the filters are dropped. - let (filters, entries) = (" 2 0 R".repeat(2), " 2 0 R".repeat(2000)); + // Three filters, and parameters for 2,001: the entries past the filters are dropped. Two name + // object 6, whose `/JBIG2Globals` is the large array given directly, which no decoder reads. + let (filters, entries) = (" 2 0 R".repeat(2), format!(" 6 0 R 6 0 R{}", " 2 0 R".repeat(1998))); let dict = format!("/Width 2 0 R /Height 2 0 R /Filter [/FlateDecode{filters}] /DecodeParms [<< /Predictor 3 0 R /Columns 2 0 R{keys} >>{entries}]"); - let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), big.into_bytes(), b"4 0 R".to_vec(), b"12".to_vec(), stream(&dict, b"")]); + let globals = format!("<< /JBIG2Globals {big} >>"); + // A chain of more filters than any decoder takes is no filter, and its parameters aren't read: + // ten here, against eight and a `/Crypt`. + let long = format!("/Filter [{}] /DecodeParms 2 0 R", "/FlateDecode ".repeat(10)); + let doc = pdf(vec![ + b"<< /Type /Catalog >>".to_vec(), + big.into_bytes(), + b"4 0 R".to_vec(), + b"12".to_vec(), + stream(&dict, b""), + globals.into_bytes(), + stream(&long, b""), + stream(&format!("/Filter [{}]", "/FlateDecode ".repeat(9)), b""), + ]); let Object::Stream(s) = &*doc.get(ObjRef::new(5, 0)) else { panic!("object 5 is the stream") }; let made = crate::globals::readable(&doc, s); let mut bytes = Vec::new(); @@ -1811,6 +1825,33 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { let parms = made.dict.get(b"DecodeParms").and_then(Object::as_array).unwrap(); assert_eq!(parms.first().and_then(Object::as_dict).and_then(|d| d.get(b"Predictor")), Some(&Object::Int(12))); assert_eq!(parms.len(), 3, "an entry for each filter"); + assert!(parms[1].as_dict().is_some_and(|d| !d.contains(b"JBIG2Globals")), "{:?}", parms[1]); + let Object::Stream(s) = &*doc.get(ObjRef::new(7, 0)) else { panic!("object 7 is the stream") }; + let made = crate::globals::readable(&doc, s); + assert_eq!(made.dict.get(b"Filter"), Some(&Object::Null)); + assert_eq!(made.dict.get(b"DecodeParms").and_then(Object::as_array).map(Vec::len), Some(0)); + // Eight filters and a `/Crypt` are as many as a decoder takes: those are read. + let Object::Stream(s) = &*doc.get(ObjRef::new(8, 0)) else { panic!("object 8 is the stream") }; + let made = crate::globals::readable(&doc, s); + assert_eq!(made.dict.get(b"Filter").and_then(Object::as_array).map(Vec::len), Some(9)); +} + +/// What streams share by reference is read once a run, however many share it: a parameters array +/// of 5,000 entries named by 100 streams costs the array and its one entry, once. +#[test] +fn a_shared_parameters_array_is_read_once_however_many_streams_share_it() { + let entries = " 4 0 R".repeat(5000); + let mut objects = + vec![b"<< /Type /Catalog >>".to_vec(), format!("[{entries}]").into_bytes(), stream("", b"symbols"), b"<< /JBIG2Globals 3 0 R >>".to_vec()]; + objects.extend((0..100).map(|_| stream("/Filter /JBIG2Decode /DecodeParms 2 0 R", b""))); + let doc = pdf(objects); + let mut names = crate::globals::Names::default(); + for n in 5..105 { + let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; + let lists = names.of(&doc, s); + assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::>(), [vec![3]]); + } + assert_eq!(names.read, 2); } /// A copy of the dictionary stored with a predictor is found whatever way its parameters are given, From f5aa20f7a9e815583951ff2980446fab2c516dd6 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 14:03:42 +0100 Subject: [PATCH 11/27] M8: a shared parameters value is remembered by the object it ends at Names kept what a value names by its first reference, so streams naming one array each through an object of their own that refers to it missed the memo every time: quadratic time, and now memory too (8000 such streams with 8000 entries, 1.4 MB: 30 s, 504 MiB). It is keyed by the object the references end at, so another reference, or a chain, finds it (the same file: 0.29 s, 7 MiB, measured by the review). matches and added use the same key. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 21 ++++++++++++++++----- crates/redact/src/tests.rs | 17 +++++++++++++---- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 735bbc3667..c8b91f248a 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -23,19 +23,28 @@ pub(crate) fn kept(doc: &Document) -> HashSet { /// `o`, or what references lead to from it. A ring of them ends as a reference, which is no dictionary. pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { + followed_to(doc, o).0 +} + +/// [`followed`], and the last reference on the way: the one that names what it ends at, whichever +/// reference or chain of them led there. `None` when `o` was given directly. +fn followed_to(doc: &Document, o: &Object) -> (Arc, Option) { + let mut last = o.as_ref(); let mut at = doc.resolve(o); for _ in 0..MAX_HOPS { let next = match &*at { Object::Ref(next) => *next, _ => break, }; + last = Some(next); at = doc.get(next); } - at + (at, last) } /// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the -/// file refers to, by its object number and whether it is read as a list of entries. +/// file refers to, by the number of the object the references end at and whether it is read as a +/// list of entries. pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); /// Every object a stream names as `/JBIG2Globals`, and each reference on the way to one: in any entry @@ -43,7 +52,7 @@ pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); /// reader may use less of this, but leaving a dictionary behind is the failure that matters here. /// /// What a value the file refers to names is read once a run, so streams that share one, by -/// reference or through an entry, cost it once however many there are. +/// reference, through a chain of them or through an entry, cost it once however many there are. #[derive(Default)] pub(crate) struct Names { memo: HashMap<(u32, bool), Rc<[u32]>>, @@ -60,12 +69,14 @@ impl Names { /// What `v` names: one entry, or (`list`) an array of them or one. fn value(&mut self, doc: &Document, v: &Object, list: bool) -> Listed { - let key = v.as_ref().map(|r| (r.num, list)); + // By the object the references end at: another reference to it, or a chain, finds it too. + let (held, last) = followed_to(doc, v); + let key = last.map(|r| (r.num, list)); if let Some(hit) = key.and_then(|k| self.memo.get(&k)) { return (hit.clone(), key); } self.read += 1; - let names: Rc<[u32]> = match (&*followed(doc, v), list) { + let names: Rc<[u32]> = match (&*held, list) { (Object::Array(entries), true) => { let mut all: Vec = entries.iter().flat_map(|e| self.value(doc, e, false).0.to_vec()).collect(); all.sort_unstable(); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index dd1d9b4b43..e7e7854021 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1836,17 +1836,26 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { assert_eq!(made.dict.get(b"Filter").and_then(Object::as_array).map(Vec::len), Some(9)); } -/// What streams share by reference is read once a run, however many share it: a parameters array -/// of 5,000 entries named by 100 streams costs the array and its one entry, once. +/// What streams share by reference is read once a run, however many share it and however they +/// refer to it: a parameters array of 5,000 entries named by 100 streams, directly, each through an +/// object of its own that refers to it, or through a chain of two, costs the array and its one +/// entry, once. #[test] fn a_shared_parameters_array_is_read_once_however_many_streams_share_it() { let entries = " 4 0 R".repeat(5000); let mut objects = vec![b"<< /Type /Catalog >>".to_vec(), format!("[{entries}]").into_bytes(), stream("", b"symbols"), b"<< /JBIG2Globals 3 0 R >>".to_vec()]; - objects.extend((0..100).map(|_| stream("/Filter /JBIG2Decode /DecodeParms 2 0 R", b""))); + // Objects 5 to 54 refer to the array, and 55 to 5. + objects.extend((0..50).map(|_| b"2 0 R".to_vec())); + objects.push(b"5 0 R".to_vec()); + // Streams 56 to 155: fifty name the array directly, forty-nine through objects 5 to 53, one + // through the chain from 55. + objects.extend((0..50).map(|_| stream("/Filter /JBIG2Decode /DecodeParms 2 0 R", b""))); + objects.extend((5..54).map(|n| stream(&format!("/Filter /JBIG2Decode /DecodeParms {n} 0 R"), b""))); + objects.push(stream("/Filter /JBIG2Decode /DecodeParms 55 0 R", b"")); let doc = pdf(objects); let mut names = crate::globals::Names::default(); - for n in 5..105 { + for n in 56..156 { let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; let lists = names.of(&doc, s); assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::>(), [vec![3]]); From 85bcfea183c12447717489cb6deb55a15cb83a96 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 14:26:30 +0100 Subject: [PATCH 12/27] M8: a remembered parameters value is found before it is fetched f5aa20f7a keyed what Names remembers by the object references end at, but followed them all before looking: every hit fetched the object again. With two large objects that push each other out of cos's caches, each fetch decoded 17 MiB, and a 66 KiB file took 26 s where it took 0.6 s. Each reference is now looked up before it is fetched, and what it leads to is remembered under every reference on the way, with the key of the last: a hit fetches nothing, an alias or a chain only the references before what is remembered. The test counts fetches. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 57 +++++++++++++++++++++++------------- crates/redact/src/tests.rs | 2 ++ 2 files changed, 38 insertions(+), 21 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index c8b91f248a..7a2336abce 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -23,29 +23,24 @@ pub(crate) fn kept(doc: &Document) -> HashSet { /// `o`, or what references lead to from it. A ring of them ends as a reference, which is no dictionary. pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { - followed_to(doc, o).0 -} - -/// [`followed`], and the last reference on the way: the one that names what it ends at, whichever -/// reference or chain of them led there. `None` when `o` was given directly. -fn followed_to(doc: &Document, o: &Object) -> (Arc, Option) { - let mut last = o.as_ref(); let mut at = doc.resolve(o); for _ in 0..MAX_HOPS { let next = match &*at { Object::Ref(next) => *next, _ => break, }; - last = Some(next); at = doc.get(next); } - (at, last) + at } /// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the /// file refers to, by the number of the object the references end at and whether it is read as a /// list of entries. -pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); +pub(crate) type Listed = (Rc<[u32]>, Option); + +/// A value the file refers to: the number of an object, and whether it is read as a list of entries. +pub(crate) type Key = (u32, bool); /// Every object a stream names as `/JBIG2Globals`, and each reference on the way to one: in any entry /// of its parameters, whichever filter that entry belongs to and however the filters are given. A @@ -55,9 +50,13 @@ pub(crate) type Listed = (Rc<[u32]>, Option<(u32, bool)>); /// reference, through a chain of them or through an entry, cost it once however many there are. #[derive(Default)] pub(crate) struct Names { - memo: HashMap<(u32, bool), Rc<[u32]>>, + /// What a reference leads to names, under every reference on the way, with the key of the one + /// that names what it ends at: aliases and chains find it, and are counted as that one. + memo: HashMap, Key)>, /// How many values were read rather than remembered. pub(crate) read: usize, + /// How many objects were fetched on the way to a value. + pub(crate) fetched: usize, } impl Names { @@ -69,11 +68,22 @@ impl Names { /// What `v` names: one entry, or (`list`) an array of them or one. fn value(&mut self, doc: &Document, v: &Object, list: bool) -> Listed { - // By the object the references end at: another reference to it, or a chain, finds it too. - let (held, last) = followed_to(doc, v); - let key = last.map(|r| (r.num, list)); - if let Some(hit) = key.and_then(|k| self.memo.get(&k)) { - return (hit.clone(), key); + // Through the references as `followed` goes, each looked up before it is fetched, so one + // remembered costs nothing more, and an alias or a chain only the references before it. + let mut keys: Vec = Vec::new(); + let mut held = Arc::new(v.clone()); + for _ in 0..=MAX_HOPS { + let Some(r) = Object::as_ref(&held) else { break }; + let key = (r.num, list); + if let Some((hit, at)) = self.memo.get(&key).cloned() { + for k in keys { + self.memo.insert(k, (hit.clone(), at)); + } + return (hit, Some(at)); + } + keys.push(key); + self.fetched += 1; + held = doc.get(r); } self.read += 1; let names: Rc<[u32]> = match (&*held, list) { @@ -85,10 +95,15 @@ impl Names { } (entry, _) => entry_names(doc, entry).into(), }; - if let Some(k) = key { - self.memo.insert(k, names.clone()); + // Remembered when the references end at an object, under the last of them; a ring, or a + // chain past the limit, is read again where it is met. + let at = keys.last().copied().filter(|_| !matches!(*held, Object::Ref(_))); + if let Some(at) = at { + for k in keys { + self.memo.insert(k, (names.clone(), at)); + } } - (names, key) + (names, at) } } @@ -249,7 +264,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result = HashSet::new(); // A list the file refers to is added once, however many streams share it. - let mut added: HashSet<(u32, bool)> = HashSet::new(); + let mut added: HashSet = HashSet::new(); for &num in before.difference(&now) { if let Ok(object) = doc.try_get(num) && let Object::Stream(s) = &*object @@ -269,7 +284,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result> = HashMap::new(); + let mut matches: HashMap> = HashMap::new(); added.clear(); let mut users = Vec::new(); for (r, object) in doc.scan_objects() { diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index e7e7854021..28fc4888cb 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1861,6 +1861,8 @@ fn a_shared_parameters_array_is_read_once_however_many_streams_share_it() { assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::>(), [vec![3]]); } assert_eq!(names.read, 2); + // Each object once: the array, its entry, each of the 49 objects that refer to it, and 55. + assert_eq!(names.fetched, 52); } /// A copy of the dictionary stored with a predictor is found whatever way its parameters are given, From fee25bfe4d124599301a074a209dff67e6bbea01 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 14:43:36 +0100 Subject: [PATCH 13/27] M8: finding what shares a dictionary spends from one budget a run Reviews kept finding ways a small file could make this run long or large: readable() fetched the same objects again for each image that names a dictionary (~3.9 s each when two large objects push each other out of cos's caches), and the memo of names grew with distinct arrays times their entries (~3.7 GB from 2 MiB). Nothing bounded the run as a whole. Everything unshare reads now goes through one Run: what a reference leads to is fetched at most once a run for each way it is read (a size, a filter, parameters, an entry, a flag, /F, a colour space, an ICC profile), looked up before it is fetched and kept only as what is read of it. Objects fetched, names remembered, and what is decoded (a unit for each 4 KiB) count against RUN_BUDGET (2^22), and past it the run is refused with SharedJbig2Budget. A scan reads a few objects a page, so a book of thousands of pages sharing one dictionary stays well within it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 460 +++++++++++++++++++++++++---------- crates/redact/src/lib.rs | 7 + crates/redact/src/tests.rs | 59 ++++- 3 files changed, 388 insertions(+), 138 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 7a2336abce..06f7de8f75 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -34,6 +34,13 @@ pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { at } +/// The most one run may spend finding what shares a dictionary with what left: objects fetched, +/// names remembered, and decoded data, a unit and one for each whole 4 KiB. Past it the run is refused +/// ([`RedactError::SharedJbig2Budget`](crate::RedactError::SharedJbig2Budget)). A scan reads a few +/// objects a page, and its pages stored again a unit for each 4 KiB of their samples, so a book of +/// thousands of pages sharing one dictionary stays well within it. +pub(crate) const RUN_BUDGET: usize = 1 << 22; + /// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the /// file refers to, by the number of the object the references end at and whether it is read as a /// list of entries. @@ -42,169 +49,320 @@ pub(crate) type Listed = (Rc<[u32]>, Option); /// A value the file refers to: the number of an object, and whether it is read as a list of entries. pub(crate) type Key = (u32, bool); -/// Every object a stream names as `/JBIG2Globals`, and each reference on the way to one: in any entry -/// of its parameters, whichever filter that entry belongs to and however the filters are given. A -/// reader may use less of this, but leaving a dictionary behind is the failure that matters here. -/// -/// What a value the file refers to names is read once a run, so streams that share one, by -/// reference, through a chain of them or through an entry, cost it once however many there are. -#[derive(Default)] -pub(crate) struct Names { +/// How a value is read, and so what is kept of it: what a reference leads to is remembered for each. +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +enum How { + /// A size: a number, or null. + Number, + /// One filter: a name, or null. + Name, + /// `/Filter`: a name, or an array of names, or null for a chain longer than any decoder takes. + Filter, + /// `/DecodeParms` for so many filters. + Parms(usize), + /// One parameters entry: the numbers and flags decoders read, and `/JBIG2Globals` by reference. + Entry, + /// A number or a flag. + Scalar, + /// `/F`: whether it names anything (true) or is null. + Present, + /// `/ColorSpace`: whether it is known to have more than one component. + Components, + /// An ICC profile: its `/N`, or null. + Profile, +} + +/// What one redaction or Remove Hidden Information run reads, to find what shares a dictionary with +/// what left, against [`RUN_BUDGET`]. Each object is fetched at most once a run for each way it is +/// read, and kept only as what is read of it: a value many streams share, by reference, through a +/// chain of them or through an entry, costs it once however many there are. +pub(crate) struct Run { + budget: usize, + spent: usize, + /// What a reference leads to, as it is read, under every reference on the way. + reduced: HashMap<(u32, How), Rc>, /// What a reference leads to names, under every reference on the way, with the key of the one /// that names what it ends at: aliases and chains find it, and are counted as that one. - memo: HashMap, Key)>, - /// How many values were read rather than remembered. + names: HashMap, Key)>, + /// How many values were read for their names rather than remembered. pub(crate) read: usize, - /// How many objects were fetched on the way to a value. + /// How many objects were fetched. pub(crate) fetched: usize, } -impl Names { - /// What `s` names, one list for each key that gives it parameters. - pub(crate) fn of(&mut self, doc: &Document, s: &Stream) -> Vec { +impl Run { + pub(crate) fn new(budget: usize) -> Self { + Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), read: 0, fetched: 0 } + } + + /// Counts `units` against the budget. + fn spend(&mut self, units: usize) -> Result<(), crate::RedactError> { + self.spent = self.spent.saturating_add(units); + if self.spent > self.budget { Err(crate::RedactError::SharedJbig2Budget) } else { Ok(()) } + } + + /// Fetches the object `r` names, counted. + fn fetch(&mut self, doc: &Document, r: pdfcraft_cos::ObjRef) -> Result, crate::RedactError> { + self.spend(1)?; + self.fetched += 1; + Ok(doc.get(r)) + } + + /// What `v` leads to through references, as `followed` goes, kept as `read` makes it. Each + /// reference is looked up before it is fetched, and what it leads to is remembered under every + /// one on the way, so one remembered costs nothing more, and an alias or a chain only the + /// references before it. A ring, or a chain past the limit, is read as the reference it ends at. + fn follow( + &mut self, + doc: &Document, + v: &Object, + how: How, + read: impl FnOnce(&mut Self, &Document, &Object) -> Result, + ) -> Result, crate::RedactError> { + let mut keys: Vec<(u32, How)> = Vec::new(); + let mut fetched: Option> = None; + for _ in 0..=MAX_HOPS { + let Some(r) = fetched.as_deref().unwrap_or(v).as_ref() else { break }; + if let Some(hit) = self.reduced.get(&(r.num, how)).cloned() { + for k in keys { + self.reduced.insert(k, hit.clone()); + } + return Ok(hit); + } + keys.push((r.num, how)); + fetched = Some(self.fetch(doc, r)?); + } + let made = Rc::new(read(self, doc, fetched.as_deref().unwrap_or(v))?); + for k in keys { + self.reduced.insert(k, made.clone()); + } + Ok(made) + } + + /// What `s` names as `/JBIG2Globals`, and each reference on the way to one: in any entry of its + /// parameters, whichever filter that entry belongs to and however the filters are given, one + /// list for each key that gives it parameters. A reader may use less of this, but leaving a + /// dictionary behind is the failure that matters here. + pub(crate) fn names_of(&mut self, doc: &Document, s: &Stream) -> Result, crate::RedactError> { // `/FDecodeParms` too: those of data in another file, which a reader may draw instead. - [b"DecodeParms".as_slice(), b"DP", b"FDecodeParms"].into_iter().filter_map(|key| s.dict.get(key)).map(|v| self.value(doc, v, true)).collect() + [b"DecodeParms".as_slice(), b"DP", b"FDecodeParms"] + .into_iter() + .filter_map(|key| s.dict.get(key)) + .map(|v| self.names_in(doc, v, true)) + .collect() } /// What `v` names: one entry, or (`list`) an array of them or one. - fn value(&mut self, doc: &Document, v: &Object, list: bool) -> Listed { - // Through the references as `followed` goes, each looked up before it is fetched, so one - // remembered costs nothing more, and an alias or a chain only the references before it. + fn names_in(&mut self, doc: &Document, v: &Object, list: bool) -> Result { let mut keys: Vec = Vec::new(); - let mut held = Arc::new(v.clone()); + let mut fetched: Option> = None; for _ in 0..=MAX_HOPS { - let Some(r) = Object::as_ref(&held) else { break }; + let Some(r) = fetched.as_deref().unwrap_or(v).as_ref() else { break }; let key = (r.num, list); - if let Some((hit, at)) = self.memo.get(&key).cloned() { + if let Some((hit, at)) = self.names.get(&key).cloned() { for k in keys { - self.memo.insert(k, (hit.clone(), at)); + self.names.insert(k, (hit.clone(), at)); } - return (hit, Some(at)); + return Ok((hit, Some(at))); } keys.push(key); - self.fetched += 1; - held = doc.get(r); + fetched = Some(self.fetch(doc, r)?); } self.read += 1; - let names: Rc<[u32]> = match (&*held, list) { + let held = fetched.as_deref().unwrap_or(v); + let names: Rc<[u32]> = match (held, list) { (Object::Array(entries), true) => { - let mut all: Vec = entries.iter().flat_map(|e| self.value(doc, e, false).0.to_vec()).collect(); + let mut all: Vec = Vec::new(); + for e in entries { + all.extend(self.names_in(doc, e, false)?.0.iter()); + } all.sort_unstable(); all.dedup(); all.into() } - (entry, _) => entry_names(doc, entry).into(), + (entry, _) => self.entry_names(doc, entry)?.into(), }; - // Remembered when the references end at an object, under the last of them; a ring, or a - // chain past the limit, is read again where it is met. - let at = keys.last().copied().filter(|_| !matches!(*held, Object::Ref(_))); + // Remembered when the references end at an object, under the last of them, and counted + // against the budget as held; a ring, or a chain past the limit, is read again where met. + let at = keys.last().copied().filter(|_| !matches!(held, Object::Ref(_))); if let Some(at) = at { + self.spend(names.len())?; for k in keys { - self.memo.insert(k, (names.clone(), at)); + self.names.insert(k, (names.clone(), at)); } } - (names, at) + Ok((names, at)) } -} -/// What one parameters entry names as `/JBIG2Globals`, and each reference on the way to it. -fn entry_names(doc: &Document, entry: &Object) -> Vec { - let mut found = Vec::new(); - let Some(mut at) = entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return found }; - for _ in 0..=MAX_HOPS { - found.push(at.num); - match &*doc.get(at) { - Object::Ref(next) => at = *next, - _ => break, + /// What one parameters entry names as `/JBIG2Globals`, and each reference on the way to it. + fn entry_names(&mut self, doc: &Document, entry: &Object) -> Result, crate::RedactError> { + let mut found = Vec::new(); + let Some(mut at) = entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return Ok(found) }; + for _ in 0..=MAX_HOPS { + found.push(at.num); + match &*self.fetch(doc, at)? { + Object::Ref(next) => at = *next, + _ => break, + } } + Ok(found) } - found -} -/// A `/Filter` through any references, as a decoder reads it: a name, or an array of names, each -/// entry through references too. Anything else is null, a chain longer than any decoder takes -/// (`MAX_CHAIN`, and a `/Crypt`) included, so nothing large is copied however often the file refers -/// to it, and a stream that names it is read as one with no filter. -fn direct_filter(doc: &Document, v: &Object) -> Object { - let name = |o: &Object| if matches!(o, Object::Name(_)) { o.clone() } else { Object::Null }; - match &*followed(doc, v) { - Object::Array(a) if a.len() > pdfcraft_filters::MAX_CHAIN + 1 => Object::Null, - Object::Array(a) => Object::Array(a.iter().map(|e| name(&followed(doc, e))).collect()), - other => name(other), + /// A size through any references: a number, or null. + fn number(&mut self, doc: &Document, v: &Object) -> Result { + let read = |_: &mut Self, _: &Document, o: &Object| Ok(if matches!(o, Object::Int(_) | Object::Real(_)) { o.clone() } else { Object::Null }); + Ok((*self.follow(doc, v, How::Number, read)?).clone()) } -} -/// A size through any references: a number, or null. -fn direct_number(doc: &Document, v: &Object) -> Object { - match &*followed(doc, v) { - number @ (Object::Int(_) | Object::Real(_)) => number.clone(), - _ => Object::Null, + /// A `/Filter` through any references, as a decoder reads it: a name, or an array of names, each + /// entry through references too. Anything else is null, a chain longer than any decoder takes + /// (`MAX_CHAIN`, and a `/Crypt`) included, so a stream that names it is read as one with no + /// filter. + fn filter(&mut self, doc: &Document, v: &Object) -> Result { + fn name(_: &mut Run, _: &Document, o: &Object) -> Result { + Ok(if matches!(o, Object::Name(_)) { o.clone() } else { Object::Null }) + } + let read = |run: &mut Self, doc: &Document, o: &Object| match o { + Object::Array(a) if a.len() > pdfcraft_filters::MAX_CHAIN + 1 => Ok(Object::Null), + Object::Array(a) => { + let mut names = Vec::with_capacity(a.len()); + for e in a { + names.push((*run.follow(doc, e, How::Name, name)?).clone()); + } + Ok(Object::Array(names)) + } + other => name(run, doc, other), + }; + Ok((*self.follow(doc, v, How::Filter, read)?).clone()) } -} -/// `/DecodeParms` as a reader takes them, through any references, for `filters` filters: of each -/// entry, the numbers and flags the decoders read ([`DECODE_PARMS_KEYS`]), and `/JBIG2Globals` when it -/// is a reference. An entry that isn't a dictionary is null. As `Document::with_direct_parms`, nothing large -/// is copied however often the file refers to it. -fn direct_parms(doc: &Document, v: &Object, filters: usize) -> Object { - let entry = |e: &Object| match &*followed(doc, e) { - Object::Dict(d) => { + /// `/DecodeParms` as a reader takes them, through any references, for `filters` filters: of each + /// entry, the numbers and flags the decoders read ([`DECODE_PARMS_KEYS`]), and `/JBIG2Globals` when + /// it is a reference (a direct value is no stream, so no decoder reads it, and it may be large). + /// An entry that isn't a dictionary is null, as `Document::with_direct_parms` reads them. + fn parms(&mut self, doc: &Document, v: &Object, filters: usize) -> Result { + fn scalar(_: &mut Run, _: &Document, o: &Object) -> Result { + Ok(if matches!(o, Object::Int(_) | Object::Real(_) | Object::Bool(_)) { o.clone() } else { Object::Null }) + } + fn entry(run: &mut Run, doc: &Document, o: &Object) -> Result { + let Object::Dict(d) = o else { return Ok(Object::Null) }; let mut made = Dict::new(); for key in DECODE_PARMS_KEYS { let Some(value) = d.get(key) else { continue }; - let value = followed(doc, value); - if matches!(*value, Object::Int(_) | Object::Real(_) | Object::Bool(_)) { + let value = run.follow(doc, value, How::Scalar, scalar)?; + if !matches!(*value, Object::Null) { made.set(key.to_vec(), (*value).clone()); } } - // As a reference only, as `Document::with_direct_parms` keeps it: a direct value is no - // stream, so no decoder reads it, and it may be large. if let Some(globals @ Object::Ref(_)) = d.get(b"JBIG2Globals") { made.set(b"JBIG2Globals".to_vec(), globals.clone()); } - Object::Dict(made) + Ok(Object::Dict(made)) } - _ => Object::Null, - }; - match &*followed(doc, v) { - Object::Array(a) => Object::Array(a.iter().take(filters).map(entry).collect()), - other => entry(other), - } -} - -/// `s` with what `decoded_image` and `decoded_within` read as given made direct: its size, filters -/// and their parameters. -pub(crate) fn readable(doc: &Document, s: &Stream) -> Stream { - let mut readable = s.clone(); - for key in [b"Width".as_slice(), b"Height"] { - if let Some(v) = s.dict.get(key) { - readable.dict.set(key.to_vec(), direct_number(doc, v)); - } - } - if let Some(v) = s.dict.get(b"Filter") { - readable.dict.set(b"Filter".to_vec(), direct_filter(doc, v)); - } - if let Some(v) = s.dict.get(b"DecodeParms") { - let filters = match readable.dict.get(b"Filter") { - Some(Object::Array(a)) => a.len(), - Some(Object::Name(_)) => 1, - _ => 0, + let read = move |run: &mut Self, doc: &Document, o: &Object| match o { + Object::Array(a) => { + let mut entries = Vec::new(); + for e in a.iter().take(filters) { + entries.push((*run.follow(doc, e, How::Entry, entry)?).clone()); + } + Ok(Object::Array(entries)) + } + other => entry(run, doc, other), }; - readable.dict.set(b"DecodeParms".to_vec(), direct_parms(doc, v, filters)); + Ok((*self.follow(doc, v, How::Parms(filters), read)?).clone()) + } + + /// Whether `s`'s data is in another file, which a reader may draw instead of what is here: a `/F` + /// that isn't null, through any references (a null entry is as if absent, and a reference to no + /// object is null, ISO 32000-2 §7.3.7 and §7.3.10). A ring of references counts as a file. + fn elsewhere(&mut self, doc: &Document, s: &Stream) -> Result { + let Some(f) = s.dict.get(b"F") else { return Ok(false) }; + let read = |_: &mut Self, _: &Document, o: &Object| Ok(Object::Bool(!matches!(o, Object::Null))); + Ok(matches!(*self.follow(doc, f, How::Present, read)?, Object::Bool(true))) + } + + /// Whether `s`'s colour space is known to have more than one component, through any references: + /// a JBIG2 image is drawn as one whatever its dictionary says, so such an image read as one bit + /// would show otherwise. An unknown colour space isn't known to have more. + fn several_components(&mut self, doc: &Document, s: &Stream) -> Result { + let Some(cs) = s.dict.get(b"ColorSpace") else { return Ok(false) }; + fn profile(_: &mut Run, _: &Document, o: &Object) -> Result { + // /N lives in the ICC profile stream, and shall be 1, 3 or 4 (ISO 32000-2 §8.6.5.5). + let n = match o { + Object::Stream(icc) => icc.dict.int(b"N").filter(|n| matches!(n, 1 | 3 | 4)), + _ => None, + }; + Ok(n.map_or(Object::Null, Object::Int)) + } + let read = |run: &mut Self, doc: &Document, o: &Object| { + let name = match o { + Object::Name(n) => n.as_slice(), + Object::Array(a) => match a.first().and_then(Object::as_name) { + Some(n) => n, + None => return Ok(Object::Bool(false)), + }, + _ => return Ok(Object::Bool(false)), + }; + Ok(Object::Bool(match name { + b"DeviceRGB" | b"RGB" | b"CalRGB" | b"Lab" | b"DeviceCMYK" | b"CMYK" => true, + b"ICCBased" => match o.as_array().and_then(|a| a.get(1)) { + Some(entry) => run.follow(doc, entry, How::Profile, profile)?.as_int().is_some_and(|n| n > 1), + None => false, + }, + _ => false, + })) + }; + Ok(matches!(*self.follow(doc, cs, How::Components, read)?, Object::Bool(true))) + } + + /// `s` with what `decoded_image` and `decoded_within` read as given made direct: its size, + /// filters and their parameters, each kept small, however often the file refers to a large object. + pub(crate) fn readable(&mut self, doc: &Document, s: &Stream) -> Result { + let mut readable = s.clone(); + for key in [b"Width".as_slice(), b"Height"] { + if let Some(v) = s.dict.get(key) { + let number = self.number(doc, v)?; + readable.dict.set(key.to_vec(), number); + } + } + if let Some(v) = s.dict.get(b"Filter") { + let filter = self.filter(doc, v)?; + readable.dict.set(b"Filter".to_vec(), filter); + } + if let Some(v) = s.dict.get(b"DecodeParms") { + let filters = match readable.dict.get(b"Filter") { + Some(Object::Array(a)) => a.len(), + Some(Object::Name(_)) => 1, + _ => 0, + }; + let parms = self.parms(doc, v, filters)?; + readable.dict.set(b"DecodeParms".to_vec(), parms); + } + Ok(readable) + } + + /// Counts what was decoded: a unit, and one more for each whole 4 KiB. + pub(crate) fn decoded(&mut self, bytes: usize) -> Result<(), crate::RedactError> { + self.spend(bytes / 4096 + 1) } - readable } /// What dictionary `num` holds, to know a copy of it by: a file put together from pages split apart /// has one per page, each byte for byte the others. -fn content(doc: &Document, num: u32) -> Option { - let object = doc.try_get(num).ok()?; - let Object::Stream(s) = &*object else { return None }; - let readable = readable(doc, s); - let data = doc.with_direct_parms(&readable).decoded_within(MAX_JBIG2_GLOBALS).unwrap_or_else(|_| s.raw.to_vec()); +fn content(run: &mut Run, doc: &Document, num: u32) -> Result, crate::RedactError> { + let Ok(object) = doc.try_get(num) else { return Ok(None) }; + let Object::Stream(s) = &*object else { return Ok(None) }; + let readable = run.readable(doc, s)?; + let data = doc.with_direct_parms(&readable).decoded_within(MAX_JBIG2_GLOBALS); let mut hash = DefaultHasher::new(); - data.hash(&mut hash); - Some(hash.finish()) + match &data { + Ok(data) => { + run.decoded(data.len())?; + data.hash(&mut hash); + } + Err(_) => s.raw[..].hash(&mut hash), + } + Ok(Some(hash.finish())) } /// JBIG2 stream `s` stored alone, as a cleared copy is: what it shows unchanged and no dictionary @@ -213,36 +371,65 @@ fn content(doc: &Document, num: u32) -> Option { /// or known to have more than one component, which JBIG2 is drawn as one of whatever the dictionary /// says; for one whose data is in another file (`/F`), which readers draw instead of what is here; /// and for one that can't be decoded. -fn without_dictionary(doc: &Document, s: &Stream) -> Option { - if crate::image::data_elsewhere(doc, s) { - return None; +fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result, crate::RedactError> { + if run.elsewhere(doc, s)? { + return Ok(None); } - let readable = readable(doc, s); + let readable = run.readable(doc, s)?; let jbig2 = match readable.dict.get(b"Filter") { Some(Object::Array(a)) => a.iter().any(|f| f.as_name() == Some(b"JBIG2Decode".as_slice())), Some(f) => f.as_name() == Some(b"JBIG2Decode".as_slice()), None => false, }; - if !jbig2 || (s.dict.contains(b"ColorSpace") && crate::image::components(doc, s).is_some_and(|n| n > 1)) { - return None; + if !jbig2 || run.several_components(doc, s)? { + return Ok(None); } let side = |key: &[u8]| readable.dict.int(key).and_then(|v| usize::try_from(v).ok()); - let (w, h) = (side(b"Width")?, side(b"Height")?); + let (Some(w), Some(h)) = (side(b"Width"), side(b"Height")) else { return Ok(None) }; if (w as u64).saturating_mul(h as u64) > crate::image::MAX_PIXELS { - return None; + return Ok(None); } // As for `clear`: the filters before the JBIG2 decoder may give up to three bytes for each one // of the raster and a byte a row. - let allowance = w.div_ceil(8).checked_mul(h)?.checked_add(h)?.checked_mul(3)?.checked_add(4096)?; - let data = doc.decoded_image_within(&readable, allowance).ok()?; + let Some(allowance) = + w.div_ceil(8).checked_mul(h).and_then(|n| n.checked_add(h)).and_then(|n| n.checked_mul(3)).and_then(|n| n.checked_add(4096)) + else { + return Ok(None); + }; + let Ok(data) = doc.decoded_image_within(&readable, allowance) else { return Ok(None) }; + run.decoded(data.len())?; let mut dict = readable.dict.clone(); // `/FFilter` and `/FDecodeParms` too: with no data in another file they mean nothing, but - // `named` reads them. + // `names_of` reads them. for key in [b"Length".as_slice(), b"DecodeParms", b"DP", b"FFilter", b"FDecodeParms"] { dict.remove(key); } dict.set(b"BitsPerComponent".to_vec(), Object::Int(1)); - Some(crate::image::smallest(dict, &data, w, h, true, true)) + Ok(Some(crate::image::smallest(dict, &data, w, h, true, true))) +} + +/// Whether dictionary `n` is one that left (`left`), or a copy of one (`contents`); what each +/// object holds is found once a run (`read`). +fn matches_left( + run: &mut Run, + doc: &Document, + n: u32, + left: &HashSet, + contents: &HashSet, + read: &mut HashMap>, +) -> Result { + if left.contains(&n) { + return Ok(true); + } + let held = match read.get(&n) { + Some(held) => *held, + None => { + let held = content(run, doc, n)?; + read.insert(n, held); + held + } + }; + Ok(held.is_some_and(|c| contents.contains(&c))) } /// Takes the `/JBIG2Globals` dictionaries of what left the document since `before` ([`kept`]) out of @@ -255,13 +442,16 @@ fn without_dictionary(doc: &Document, s: &Stream) -> Option { /// An image another page still draws hasn't left: it keeps its pixels, so its dictionary keeps nothing /// more. /// +/// What it reads and decodes is counted against [`RUN_BUDGET`] for the whole run. +/// /// Errors: [`RedactError::SharedJbig2Globals`](crate::RedactError::SharedJbig2Globals) when a dictionary would /// still be kept, as what names it can't be stored without it (counted) or something else refers to -/// it (0 when that is all). Such an image may still be drawn, on a page no mark reached, so it isn't removed, and `doc` is part -/// changed by then: a caller works on a copy it can drop. +/// it (0 when that is all); [`RedactError::SharedJbig2Budget`](crate::RedactError::SharedJbig2Budget) when +/// finding that would spend more than the budget. Such an image may still be drawn, on a page no mark +/// reached, so it isn't removed, and `doc` is part changed by then: a caller works on a copy it can drop. pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result { let now = kept(doc); - let mut names = Names::default(); + let mut run = Run::new(RUN_BUDGET); let mut dictionaries: HashSet = HashSet::new(); // A list the file refers to is added once, however many streams share it. let mut added: HashSet = HashSet::new(); @@ -269,7 +459,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result) -> Result = dictionaries.iter().filter_map(|n| content(doc, *n)).collect(); + let mut contents: HashSet = HashSet::new(); + for &n in &dictionaries { + if let Some(c) = content(&mut run, doc, n)? { + contents.insert(c); + } + } let mut read: HashMap> = HashMap::new(); // Whether a name matches doesn't depend on order: it is a dictionary that left, or a copy of one // (what is added below is only ever such a copy). So what a shared list matches is found once. @@ -292,18 +487,23 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result>| read.entry(*n).or_insert_with(|| content(doc, *n)).is_some_and(|c| contents.contains(&c)); // Only what matched has to leave, a copy found here included. Another dictionary this also // names was shared with nothing that left: what else uses it may keep it, whichever comes // first in the file. let mut uses = false; - for (list, key) in names.of(doc, s) { + for (list, key) in run.names_of(doc, s)? { let matched = match key.and_then(|k| matches.get(&k)) { Some(hit) => hit.clone(), None => { - let found: Rc<[u32]> = list.iter().copied().filter(|n| left.contains(n) || copy(n, &mut read)).collect(); + let mut found = Vec::new(); + for &n in list.iter() { + if matches_left(&mut run, doc, n, &left, &contents, &mut read)? { + found.push(n); + } + } + let found: Rc<[u32]> = found.into(); if let Some(k) = key { + run.spend(found.len())?; matches.insert(k, found.clone()); } found @@ -324,7 +524,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet) -> Result { doc.set(r, Object::Stream(stored)); stored_again += 1; diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index d7108e77ae..99e596b102 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -77,6 +77,13 @@ pub enum RedactError { /// them: 0 when only something else keeps them. #[error("{}", shared_jbig2_text(*.0))] SharedJbig2Globals(usize), + /// Finding what shares JBIG2 symbol data with what is being removed would read more of the file + /// than one run allows (`globals::RUN_BUDGET`): its images and parameters refer to the same + /// objects in more ways than are checked, or hold more than is decoded. + #[error( + "finding what shares JBIG2 symbol data with something being removed would read more of this file than one run allows, so it can't be shown that the data leaves with it; this was refused" + )] + SharedJbig2Budget, #[error(transparent)] Edit(#[from] pdfcraft_edit::EditError), #[error(transparent)] diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 28fc4888cb..c9881f2880 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1575,6 +1575,8 @@ fn what_stays_is_stored_without_the_dictionary_however_it_names_it() { vec![b"11 0 R".to_vec(), b"/JBIG2Decode".to_vec()], ), (image.replace("/ColorSpace /DeviceGray", "/ColorSpace 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"/DeviceGray".to_vec()]), + // An ICC profile of one component is one. + (format!("{} /ColorSpace [/ICCBased 10 0 R]", image.replace("/ColorSpace /DeviceGray ", "")), scan.page.clone(), vec![stream("/N 1", b"")]), ] { let mut doc = SharedPages { other: other.clone(), other_data: data, extra, ..SharedPages::new(&scan) }.build(&scan); mark(&mut doc, 0, &[CLEARS], ""); @@ -1672,6 +1674,7 @@ fn a_shared_jbig2_dictionary_that_cant_leave_refuses_the_redaction() { for (other, data, extra) in [ (image.replace("/Height 40", "/Height 41"), scan.page.clone(), vec![]), (image.replace("/DeviceGray", "/DeviceRGB"), scan.page.clone(), vec![]), + (image.replace("/DeviceGray", "/DeviceCMYK"), scan.page.clone(), vec![]), (image.replace("/ColorSpace /DeviceGray", "/ColorSpace 10 0 R"), scan.page.clone(), vec![b"11 0 R".to_vec(), b"/DeviceRGB".to_vec()]), ( format!("{image} /ColorSpace [/ICCBased 10 0 R]").replace("/ColorSpace /DeviceGray ", ""), @@ -1818,7 +1821,7 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { stream(&format!("/Filter [{}]", "/FlateDecode ".repeat(9)), b""), ]); let Object::Stream(s) = &*doc.get(ObjRef::new(5, 0)) else { panic!("object 5 is the stream") }; - let made = crate::globals::readable(&doc, s); + let made = crate::globals::Run::new(crate::globals::RUN_BUDGET).readable(&doc, s).unwrap(); let mut bytes = Vec::new(); pdfcraft_cos::serialize(&Object::Dict(made.dict.clone()), &mut bytes); assert!(bytes.len() < 4096, "{} bytes made direct", bytes.len()); @@ -1827,12 +1830,12 @@ fn what_is_made_direct_is_small_however_often_a_large_object_is_named() { assert_eq!(parms.len(), 3, "an entry for each filter"); assert!(parms[1].as_dict().is_some_and(|d| !d.contains(b"JBIG2Globals")), "{:?}", parms[1]); let Object::Stream(s) = &*doc.get(ObjRef::new(7, 0)) else { panic!("object 7 is the stream") }; - let made = crate::globals::readable(&doc, s); + let made = crate::globals::Run::new(crate::globals::RUN_BUDGET).readable(&doc, s).unwrap(); assert_eq!(made.dict.get(b"Filter"), Some(&Object::Null)); assert_eq!(made.dict.get(b"DecodeParms").and_then(Object::as_array).map(Vec::len), Some(0)); // Eight filters and a `/Crypt` are as many as a decoder takes: those are read. let Object::Stream(s) = &*doc.get(ObjRef::new(8, 0)) else { panic!("object 8 is the stream") }; - let made = crate::globals::readable(&doc, s); + let made = crate::globals::Run::new(crate::globals::RUN_BUDGET).readable(&doc, s).unwrap(); assert_eq!(made.dict.get(b"Filter").and_then(Object::as_array).map(Vec::len), Some(9)); } @@ -1854,15 +1857,55 @@ fn a_shared_parameters_array_is_read_once_however_many_streams_share_it() { objects.extend((5..54).map(|n| stream(&format!("/Filter /JBIG2Decode /DecodeParms {n} 0 R"), b""))); objects.push(stream("/Filter /JBIG2Decode /DecodeParms 55 0 R", b"")); let doc = pdf(objects); - let mut names = crate::globals::Names::default(); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); for n in 56..156 { let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; - let lists = names.of(&doc, s); + let lists = run.names_of(&doc, s).unwrap(); assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::>(), [vec![3]]); } - assert_eq!(names.read, 2); - // Each object once: the array, its entry, each of the 49 objects that refer to it, and 55. - assert_eq!(names.fetched, 52); + assert_eq!(run.read, 2); + // Each object once: the array, its entry, the dictionary it names, each of the 49 objects that + // refer to the array, and 55. + assert_eq!(run.fetched, 53); + // A run whose budget that is past is refused. + let mut run = crate::globals::Run::new(20); + let refused = (56..156).try_for_each(|n| { + let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; + run.names_of(&doc, s).map(|_| ()) + }); + assert_eq!(refused, Err(RedactError::SharedJbig2Budget)); +} + +/// A run's budget is spent to the unit: a run may spend all of it, and is refused past it. What is +/// decoded spends a unit, and one more for each whole 4 KiB. +#[test] +fn a_runs_budget_is_spent_to_the_unit() { + let mut run = crate::globals::Run::new(10); + assert_eq!(run.decoded(4095), Ok(())); + assert_eq!(run.decoded(8 * 4096), Ok(())); + assert_eq!(run.decoded(0), Err(RedactError::SharedJbig2Budget)); +} + +/// What a stream's size, filters and parameters lead to is fetched once a run, however many streams +/// share them by reference: fifty images naming the same objects fetch each once for each way it is +/// read. +#[test] +fn what_streams_share_is_fetched_once_a_run() { + let mut objects = + vec![b"<< /Type /Catalog >>".to_vec(), b"/FlateDecode".to_vec(), b"<< /Predictor 4 0 R /Columns 4 0 R >>".to_vec(), b"1".to_vec()]; + objects.extend( + (0..50).map(|_| stream("/Width 4 0 R /Height 4 0 R /Filter [2 0 R 2 0 R] /DecodeParms [3 0 R 3 0 R] /ColorSpace 2 0 R /F 4 0 R", b"")), + ); + let doc = pdf(objects); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); + for n in 5..55 { + let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; + let made = run.readable(&doc, s).unwrap(); + assert_eq!(made.dict.get(b"Width"), Some(&Object::Int(1))); + assert_eq!(made.dict.get(b"Filter").and_then(Object::as_array).map(Vec::len), Some(2)); + } + // Object 4 as a size and as a parameter, 2 as a filter, 3 as an entry. + assert_eq!(run.fetched, 4); } /// A copy of the dictionary stored with a predictor is found whatever way its parameters are given, From ecf02896527e676bad98526dadce92e4bddabc2a Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 15:46:21 +0100 Subject: [PATCH 14/27] M8: the run budget counts what it missed A review of fee25bfe4 found three costs the budget didn't count: - A /JBIG2Globals reference in parameters given directly was fetched again for every entry of every stream (2,000 entries, 53 KB: 40 s, two thousand units of four million). What a reference leads to is now remembered for the run under it, and a chain met part way takes what is known from there. - A /Filter name was kept whatever it was, so a 32 MiB name referred to nine times was held nine times and copied for each image (725 MiB from 2.8 KB). Only a name a decoder knows is kept; any other is no filter, and a name longer than any decoder's isn't looked at. - A decode that failed spent nothing, so a dictionary one byte past its limit escaped the budget. A failure now costs as much as it was allowed to decode: a dictionary MAX_JBIG2_GLOBALS, an image its allowance. The README says what the budget counts, and that past it the redaction is refused. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/README.md | 6 ++-- crates/redact/src/globals.rs | 61 +++++++++++++++++++++++++++------- crates/redact/src/tests.rs | 63 ++++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 14 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 95c0e07161..83f5410c97 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -32,8 +32,10 @@ clear_marks(&mut doc, None)?; // remove marks without app refuses and changes nothing (`RedactError::SharedJbig2Globals`). `remove_hidden` takes them out the same way. Another dictionary holding the same shapes in other bytes isn't found, nor is a copy nothing names, and an image that hasn't left keeps its globals with its pixels: one another page - still draws, or one something that doesn't draw it still refers to. Each image is decoded once, - with no total across the document yet. Codecs redaction doesn't decode (DCT, JPX) are + still draws, or one something that doesn't draw it still refers to. Finding what to store again + reads each object at most once for each way it is read, and what it fetches, remembers and + decodes (a decode that fails counting as much as it was allowed) counts against one budget for + the run; past it the redaction is refused (`RedactError::SharedJbig2Budget`). Codecs redaction doesn't decode (DCT, JPX) are removed whole (fail-closed), and so is a JBIG2 image whose page isn't its size (the renderer draws the whole page into the image, so the rows cleared wouldn't be the ones shown) or of more than one bit or component, and Group 3 on bytes (`/K` 0 or more, `/EncodedByteAlign`): PdfCraft's diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 06f7de8f75..63658e5afa 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -78,12 +78,15 @@ enum How { /// chain of them or through an entry, costs it once however many there are. pub(crate) struct Run { budget: usize, - spent: usize, + /// What was spent of the budget. + pub(crate) spent: usize, /// What a reference leads to, as it is read, under every reference on the way. reduced: HashMap<(u32, How), Rc>, /// What a reference leads to names, under every reference on the way, with the key of the one /// that names what it ends at: aliases and chains find it, and are counted as that one. names: HashMap, Key)>, + /// What a `/JBIG2Globals` reference leads to: it and each reference after it, by its number. + chains: HashMap>, /// How many values were read for their names rather than remembered. pub(crate) read: usize, /// How many objects were fetched. @@ -92,7 +95,7 @@ pub(crate) struct Run { impl Run { pub(crate) fn new(budget: usize) -> Self { - Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), read: 0, fetched: 0 } + Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), chains: HashMap::new(), read: 0, fetched: 0 } } /// Counts `units` against the budget. @@ -196,15 +199,36 @@ impl Run { /// What one parameters entry names as `/JBIG2Globals`, and each reference on the way to it. fn entry_names(&mut self, doc: &Document, entry: &Object) -> Result, crate::RedactError> { + match entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) { + Some(start) => Ok(self.chain(doc, start)?.to_vec()), + None => Ok(Vec::new()), + } + } + + /// `start` and each reference after it, up to the limit, remembered under `start` for the run, + /// so entries naming it, directly or not, fetch it once however many there are; a chain met + /// part way takes what is known from there. + fn chain(&mut self, doc: &Document, start: pdfcraft_cos::ObjRef) -> Result, crate::RedactError> { + if let Some(hit) = self.chains.get(&start.num) { + return Ok(hit.clone()); + } let mut found = Vec::new(); - let Some(mut at) = entry.as_dict().and_then(|d| d.get(b"JBIG2Globals")).and_then(Object::as_ref) else { return Ok(found) }; + let mut at = start; for _ in 0..=MAX_HOPS { + if let Some(rest) = self.chains.get(&at.num) { + found.extend(rest.iter()); + break; + } found.push(at.num); match &*self.fetch(doc, at)? { Object::Ref(next) => at = *next, _ => break, } } + found.truncate(MAX_HOPS + 1); + let found: Rc<[u32]> = found.into(); + self.spend(found.len())?; + self.chains.insert(start.num, found.clone()); Ok(found) } @@ -214,13 +238,18 @@ impl Run { Ok((*self.follow(doc, v, How::Number, read)?).clone()) } - /// A `/Filter` through any references, as a decoder reads it: a name, or an array of names, each - /// entry through references too. Anything else is null, a chain longer than any decoder takes - /// (`MAX_CHAIN`, and a `/Crypt`) included, so a stream that names it is read as one with no - /// filter. + /// A `/Filter` through any references, as a decoder reads it: a name a decoder knows, or an array + /// of them, each entry through references too. Anything else is null, a chain longer than any + /// decoder takes (`MAX_CHAIN`, and a `/Crypt`) included, so a stream that names it is read as one + /// with no filter. fn filter(&mut self, doc: &Document, v: &Object) -> Result { + // A name a decoder knows; any other is no filter, and isn't copied, however long. fn name(_: &mut Run, _: &Document, o: &Object) -> Result { - Ok(if matches!(o, Object::Name(_)) { o.clone() } else { Object::Null }) + let known = |n: &[u8]| n.len() <= 16 && !matches!(pdfcraft_filters::Filter::from_name(n), pdfcraft_filters::Filter::Unknown(_)); + Ok(match o { + Object::Name(n) if known(n) => o.clone(), + _ => Object::Null, + }) } let read = |run: &mut Self, doc: &Document, o: &Object| match o { Object::Array(a) if a.len() > pdfcraft_filters::MAX_CHAIN + 1 => Ok(Object::Null), @@ -349,7 +378,7 @@ impl Run { /// What dictionary `num` holds, to know a copy of it by: a file put together from pages split apart /// has one per page, each byte for byte the others. -fn content(run: &mut Run, doc: &Document, num: u32) -> Result, crate::RedactError> { +pub(crate) fn content(run: &mut Run, doc: &Document, num: u32) -> Result, crate::RedactError> { let Ok(object) = doc.try_get(num) else { return Ok(None) }; let Object::Stream(s) = &*object else { return Ok(None) }; let readable = run.readable(doc, s)?; @@ -360,7 +389,11 @@ fn content(run: &mut Run, doc: &Document, num: u32) -> Result, crate run.decoded(data.len())?; data.hash(&mut hash); } - Err(_) => s.raw[..].hash(&mut hash), + // What it took to find out it can't be read: as much as it was allowed to decode. + Err(_) => { + run.decoded(MAX_JBIG2_GLOBALS)?; + s.raw[..].hash(&mut hash); + } } Ok(Some(hash.finish())) } @@ -371,7 +404,7 @@ fn content(run: &mut Run, doc: &Document, num: u32) -> Result, crate /// or known to have more than one component, which JBIG2 is drawn as one of whatever the dictionary /// says; for one whose data is in another file (`/F`), which readers draw instead of what is here; /// and for one that can't be decoded. -fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result, crate::RedactError> { +pub(crate) fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result, crate::RedactError> { if run.elsewhere(doc, s)? { return Ok(None); } @@ -396,7 +429,11 @@ fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> Result>".to_vec(), stream("", b"symbols")]; + objects.extend((0..50).map(|_| stream(&format!("/Filter /JBIG2Decode /DecodeParms [{entries}]"), b""))); + let doc = pdf(objects); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); + for n in 3..53 { + let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; + let lists = run.names_of(&doc, s).unwrap(); + assert_eq!(lists.iter().map(|(list, _)| list.to_vec()).collect::>(), [vec![2]]); + } + assert_eq!((run.fetched, run.spent), (1, 2), "one fetch, and the one number held"); +} + +/// A `/JBIG2Globals` reference leading through a chain names each reference on the way, up to the +/// limit of 33, whether the chain is met first part way or from its start. +#[test] +fn a_chain_of_references_is_named_to_the_limit() { + // Objects 2 to 41 each refer to the next, and 42 is the dictionary; 43 names the chain from 20, + // and 44 from its start. + let mut objects = vec![b"<< /Type /Catalog >>".to_vec()]; + objects.extend((3..=42).map(|n| format!("{n} 0 R").into_bytes())); + objects.push(stream("", b"symbols")); + objects.push(stream("/Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 20 0 R >>", b"")); + objects.push(stream("/Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 2 0 R >>", b"")); + let doc = pdf(objects); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); + let named = |run: &mut crate::globals::Run, n: u32| { + let Object::Stream(s) = &*doc.get(ObjRef::new(n, 0)) else { panic!("object {n} is a stream") }; + run.names_of(&doc, s).unwrap().iter().flat_map(|(list, _)| list.to_vec()).collect::>() + }; + assert_eq!(named(&mut run, 43), (20..=42).collect::>()); + assert_eq!(named(&mut run, 44), (2..=34).collect::>()); +} + +/// What can't be decoded is charged as much as it was allowed to decode, so going past a limit +/// costs as much as reaching it: a dictionary `MAX_JBIG2_GLOBALS` (here one behind a filter that +/// isn't decoded here), an image its allowance. +#[test] +fn a_decode_that_fails_is_charged_what_it_was_allowed() { + let image = "/Type /XObject /Subtype /Image /Width 8 /Height 8 /BitsPerComponent 1 /Filter /JBIG2Decode"; + let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), stream("/Filter /DCTDecode", b"not a JPEG"), stream(image, b"not JBIG2")]); + let units = pdfcraft_cos::MAX_JBIG2_GLOBALS / 4096 + 1; + let run = |budget| crate::globals::Run::new(budget); + assert!(crate::globals::content(&mut run(units), &doc, 2).is_ok()); + assert_eq!(crate::globals::content(&mut run(units - 1), &doc, 2), Err(RedactError::SharedJbig2Budget)); + // The image's allowance: (one byte a row of 8, and a byte a row) × 3 a byte, and 4096: 4144 + // bytes, two units. + let Object::Stream(s) = &*doc.get(ObjRef::new(3, 0)) else { panic!("object 3 is the image") }; + assert!(matches!(crate::globals::without_dictionary(&mut run(2), &doc, s), Ok(None))); + assert!(matches!(crate::globals::without_dictionary(&mut run(1), &doc, s), Err(RedactError::SharedJbig2Budget))); } /// What streams share by reference is read once a run, however many share it and however they From 251d761d6b21870c7dce0b8488e4f5076fa7c7b6 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 16:19:25 +0100 Subject: [PATCH 15/27] M8: an image is charged the dictionaries it names A review of ecf028965 measured that decoding an image undoes the filters of each dictionary it names again when cos doesn't hold their bytes (20 images naming a 32 MiB dictionary: 1.2 s at 2 units each). content() now remembers what each dictionary decoded to, or its limit when it couldn't, and an image is charged that before it is decoded. Names gathered from many entries are held as a set as they are read, so a million entries naming one chain hold it once. The claim that each object is fetched once a run for each way it is read says its exception: a /JBIG2Globals chain named part way is followed again from there. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/README.md | 7 ++++--- crates/redact/src/globals.rs | 38 ++++++++++++++++++++++++++---------- crates/redact/src/tests.rs | 15 ++++++++++++++ 3 files changed, 47 insertions(+), 13 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 83f5410c97..9b06de4056 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -33,9 +33,10 @@ clear_marks(&mut doc, None)?; // remove marks without app the same way. Another dictionary holding the same shapes in other bytes isn't found, nor is a copy nothing names, and an image that hasn't left keeps its globals with its pixels: one another page still draws, or one something that doesn't draw it still refers to. Finding what to store again - reads each object at most once for each way it is read, and what it fetches, remembers and - decodes (a decode that fails counting as much as it was allowed) counts against one budget for - the run; past it the redaction is refused (`RedactError::SharedJbig2Budget`). Codecs redaction doesn't decode (DCT, JPX) are + reads each object at most once for each way it is read (a `/JBIG2Globals` chain named part way, + or a ring of references or a chain of more than 32, may be read again), and what it fetches, remembers and decodes (a decode that + fails counting as much as it was allowed, and an image the dictionaries it names) counts against + one budget for the run; past it the redaction is refused (`RedactError::SharedJbig2Budget`). Codecs redaction doesn't decode (DCT, JPX) are removed whole (fail-closed), and so is a JBIG2 image whose page isn't its size (the renderer draws the whole page into the image, so the rows cleared wouldn't be the ones shown) or of more than one bit or component, and Group 3 on bytes (`/K` 0 or more, `/EncodedByteAlign`): PdfCraft's diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index 63658e5afa..42e34339f5 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -37,8 +37,9 @@ pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { /// The most one run may spend finding what shares a dictionary with what left: objects fetched, /// names remembered, and decoded data, a unit and one for each whole 4 KiB. Past it the run is refused /// ([`RedactError::SharedJbig2Budget`](crate::RedactError::SharedJbig2Budget)). A scan reads a few -/// objects a page, and its pages stored again a unit for each 4 KiB of their samples, so a book of -/// thousands of pages sharing one dictionary stays well within it. +/// objects a page, and its pages stored again a unit for each 4 KiB of their samples and of the +/// dictionary they name, so a book of thousands of pages sharing a dictionary of a MiB or two stays +/// within it. pub(crate) const RUN_BUDGET: usize = 1 << 22; /// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the @@ -75,7 +76,10 @@ enum How { /// What one redaction or Remove Hidden Information run reads, to find what shares a dictionary with /// what left, against [`RUN_BUDGET`]. Each object is fetched at most once a run for each way it is /// read, and kept only as what is read of it: a value many streams share, by reference, through a -/// chain of them or through an entry, costs it once however many there are. +/// chain of them or through an entry, costs it once however many there are. The exceptions are a +/// `/JBIG2Globals` chain named part way, which is followed again from there, and a ring of +/// references or a chain past the limit, read again (up to 33 fetches) wherever it is met; every +/// fetch counts. pub(crate) struct Run { budget: usize, /// What was spent of the budget. @@ -87,6 +91,9 @@ pub(crate) struct Run { names: HashMap, Key)>, /// What a `/JBIG2Globals` reference leads to: it and each reference after it, by its number. chains: HashMap>, + /// What each dictionary was found to decode to, or its limit when it couldn't be: an image + /// naming it may undo its filters again, which costs as much. + sizes: HashMap, /// How many values were read for their names rather than remembered. pub(crate) read: usize, /// How many objects were fetched. @@ -95,7 +102,7 @@ pub(crate) struct Run { impl Run { pub(crate) fn new(budget: usize) -> Self { - Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), chains: HashMap::new(), read: 0, fetched: 0 } + Run { budget, spent: 0, reduced: HashMap::new(), names: HashMap::new(), chains: HashMap::new(), sizes: HashMap::new(), read: 0, fetched: 0 } } /// Counts `units` against the budget. @@ -175,13 +182,12 @@ impl Run { let held = fetched.as_deref().unwrap_or(v); let names: Rc<[u32]> = match (held, list) { (Object::Array(entries), true) => { - let mut all: Vec = Vec::new(); + // A set as it is read, so many entries naming the same holds them once. + let mut all = std::collections::BTreeSet::new(); for e in entries { all.extend(self.names_in(doc, e, false)?.0.iter()); } - all.sort_unstable(); - all.dedup(); - all.into() + all.into_iter().collect() } (entry, _) => self.entry_names(doc, entry)?.into(), }; @@ -206,8 +212,8 @@ impl Run { } /// `start` and each reference after it, up to the limit, remembered under `start` for the run, - /// so entries naming it, directly or not, fetch it once however many there are; a chain met - /// part way takes what is known from there. + /// so entries naming it, directly or not, fetch it once however many there are; a chain that + /// reaches the start of one already remembered takes what is known from there. fn chain(&mut self, doc: &Document, start: pdfcraft_cos::ObjRef) -> Result, crate::RedactError> { if let Some(hit) = self.chains.get(&start.num) { return Ok(hit.clone()); @@ -387,11 +393,13 @@ pub(crate) fn content(run: &mut Run, doc: &Document, num: u32) -> Result { run.decoded(data.len())?; + run.sizes.insert(num, data.len()); data.hash(&mut hash); } // What it took to find out it can't be read: as much as it was allowed to decode. Err(_) => { run.decoded(MAX_JBIG2_GLOBALS)?; + run.sizes.insert(num, MAX_JBIG2_GLOBALS); s.raw[..].hash(&mut hash); } } @@ -429,6 +437,16 @@ pub(crate) fn without_dictionary(run: &mut Run, doc: &Document, s: &Stream) -> R else { return Ok(None); }; + // Decoding it undoes the filters of each dictionary it names again when cos doesn't hold their + // bytes: charged as much as each was found to decode to. + let mut named = std::collections::BTreeSet::new(); + for (list, _) in run.names_of(doc, s)? { + named.extend(list.iter().copied()); + } + let undone = named.iter().filter_map(|n| run.sizes.get(n)).fold(0usize, |sum, size| sum.saturating_add(*size)); + if undone > 0 { + run.decoded(undone)?; + } let Ok(data) = doc.decoded_image_within(&readable, allowance) else { // What it took to find out it can't be read: as much as it was allowed to decode. run.decoded(allowance)?; diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index dbc3141a05..c8cea005e2 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1884,6 +1884,21 @@ fn a_chain_of_references_is_named_to_the_limit() { assert_eq!(named(&mut run, 44), (2..=34).collect::>()); } +/// An image is charged the decoded size of each dictionary it names, as decoding it may undo that +/// dictionary's filters again. +#[test] +fn an_image_is_charged_the_dictionaries_it_names() { + let image = "/Type /XObject /Subtype /Image /Width 8 /Height 8 /BitsPerComponent 1 /Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 2 0 R >>"; + let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), stream("", vec![0u8; 40_000]), stream(image, b"not JBIG2")]); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); + crate::globals::content(&mut run, &doc, 2).unwrap(); + assert_eq!(run.spent, 10, "40,000 bytes: a unit and nine whole 4 KiB"); + let Object::Stream(s) = &*doc.get(ObjRef::new(3, 0)) else { panic!("object 3 is the image") }; + assert!(matches!(crate::globals::without_dictionary(&mut run, &doc, s), Ok(None))); + // The chain to 2 (a fetch and the number held), the dictionary's 10, the image's allowance 2. + assert_eq!(run.spent, 24); +} + /// What can't be decoded is charged as much as it was allowed to decode, so going past a limit /// costs as much as reaching it: a dictionary `MAX_JBIG2_GLOBALS` (here one behind a filter that /// isn't decoded here), an image its allowance. From f802198f4adc107ebfb7765fd75653154bb3f3fb Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 18:10:06 +0100 Subject: [PATCH 16/27] M8: tests for what mutants found unchecked in clearing Mutation testing of the PR against main left three mutants of image.rs alive: the ICCBased arm of components deleted (an ICC image is then removed whole), and data_elsewhere made false or inverted (a cleared copy then drops, or keeps, /FFilter and /FDecodeParms whatever /F says). an_icc_based_image_is_cleared_in_place_through_any_references clears an image of one component and of three, its profile direct and through a reference to a reference, and checks the samples. a_cleared_copy_keeps_parameters_for_data_ in_another_file_only_with_one checks a /F string, direct or by reference, keeps them, and a null /F, a reference to null or to no object, or none, drops them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 9e6037ed7d..657ac1e5a8 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -4180,6 +4180,43 @@ fn untouched_replacement_descriptions_survive_redaction_and_hidden_text_cleanup( } } +/// An image drawn through an ICC profile has as many components as the profile's `/N` says, and +/// is cleared in place like any other, the profile given through a chain of references too. +#[test] +fn an_icc_based_image_is_cleared_in_place_through_any_references() { + for (n, chained) in [(1, false), (3, false), (1, true), (3, true)] { + let img = stream("/Type /XObject /Subtype /Image /Width 2 /Height 1 /ColorSpace [/ICCBased 8 0 R] /BitsPerComponent 8", vec![255u8; 2 * n]); + let icc = stream(&format!("/N {n}"), b""); + let objects = if chained { vec![img, b"9 0 R".to_vec(), icc] } else { vec![img, icc] }; + let mut doc = one_page(b"q 20 0 0 10 100 100 cm /Im Do Q", "/XObject << /Im 7 0 R >>", objects); + // The left pixel. + mark(&mut doc, 0, &[[95.0, 95.0, 109.0, 115.0]], ""); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.images_removed, r.images_cleared), (0, 1), "/N {n}, chained {chained}: {r:?}"); + let samples = [vec![0u8; n], vec![255u8; n]].concat(); + assert_eq!(named_image(&doc, 0, b"PCRedacted1"), Some(samples), "/N {n}, chained {chained}"); + } +} + +/// A cleared copy keeps the parameters for data in another file only while it has one: `/FFilter` +/// and `/FDecodeParms` mean nothing without a `/F` that isn't null, and may name a JBIG2 +/// dictionary that would then stay. +#[test] +fn a_cleared_copy_keeps_parameters_for_data_in_another_file_only_with_one() { + for (f, kept) in [("/F (scan.jb2)", true), ("/F 9 0 R", true), ("/F null", false), ("/F 8 0 R", false), ("/F 99 0 R", false), ("", false)] { + let dict = format!( + "/Type /XObject /Subtype /Image /Width 2 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8 {f} /FFilter /JBIG2Decode /FDecodeParms << /JBIG2Globals 10 0 R >>" + ); + let objects = vec![stream(&dict, [255u8, 255]), b"null".to_vec(), b"(scan.jb2)".to_vec()]; + let doc = one_page(b"q 20 0 0 10 100 100 cm /Im Do Q", "/XObject << /Im 7 0 R >>", objects); + let Object::Stream(s) = &*doc.get(ObjRef::new(7, 0)) else { panic!("object 7 is the image") }; + let left = [[0.0, 0.0, 0.4, 1.0]]; + let copy = crate::image::clear(&doc, s, &pdfcraft_content::Matrix::IDENTITY, &left, &mut pdfcraft_cos::ImageBudget::with_work(u64::MAX)); + let Ok(Some(copy)) = copy else { panic!("{f}: cleared in a copy") }; + assert_eq!((copy.dict.contains(b"FFilter"), copy.dict.contains(b"FDecodeParms")), (kept, kept), "{f}"); + } +} + #[test] fn a_hostile_icc_component_count_is_refused_not_multiplied_into_a_crash() { // /N comes from the file. 2^60 × 8 bpc × width 2 overflows the row size; a negative /N wrapped From b1f2216115e2b1c91177de17b6080a90fcc93986 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 18:27:00 +0100 Subject: [PATCH 17/27] M8: a dictionary pays the image allowance for every stage of its filters content(), which reads each JBIG2 dictionary to know copies of it, charged the run budget for what its filters made at the end, a unit for each 4 KiB. That says nothing of the work: three Flates can inflate a 204-byte stream to 32 MiB, a 1-bit TIFF predictor run over all of it, and ASCIIHex skip it as white space to nothing. The Resources review of f802198f4 measured 1.33 s for one unit, apply returning Ok after 26.6 s with 20 of them in 11 KB, about 50 to 70 minutes a MB, never refused; remove_hidden and Optimize's discard too. It now decodes through the run's ImageBudget (decode_filters), which pays for what every stage reads and makes, as the images stored again already are. Past it the run is refused with ImagesPastAllowance, which remove_hidden gives as SharedJbig2Budget. The run budget still counts the decoded size. a_dictionary_pays_for_every_stage_of_its_filters: two Flates of a MiB of zero bytes under ASCIIHex spend one unit of the run and more than a MiB of the allowance; with a MiB of allowance content() and unshare are refused. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/src/globals.rs | 29 ++++++++++++++++++--------- crates/redact/src/tests.rs | 39 +++++++++++++++++++++++++++++++++--- 2 files changed, 56 insertions(+), 12 deletions(-) diff --git a/crates/redact/src/globals.rs b/crates/redact/src/globals.rs index ba12a8646b..6a71336f51 100644 --- a/crates/redact/src/globals.rs +++ b/crates/redact/src/globals.rs @@ -39,7 +39,8 @@ pub(crate) fn followed(doc: &Document, o: &Object) -> Arc { /// ([`RedactError::SharedJbig2Budget`](crate::RedactError::SharedJbig2Budget)). A scan reads a few /// objects a page, and its pages stored again a unit for each 4 KiB of their samples and of the /// dictionary they name, so a book of thousands of pages sharing a dictionary of a MiB or two stays -/// within it. +/// within it. The work of decoding, every filter stage's and the JBIG2 decoder's, is paid besides +/// from the run's image allowance (`ImageBudget`), which a unit of decoded data says nothing of. pub(crate) const RUN_BUDGET: usize = 1 << 22; /// What a parameters value names as `/JBIG2Globals`, and the key it is remembered by: a value the @@ -384,11 +385,20 @@ impl Run { /// What dictionary `num` holds, to know a copy of it by: a file put together from pages split apart /// has one per page, each byte for byte the others. -pub(crate) fn content(run: &mut Run, doc: &Document, num: u32) -> Result, crate::RedactError> { +/// +/// Decoded within `images`, the run's image allowance, which pays for what every filter stage reads +/// and makes: what the last one makes says nothing of the work (a predictor over what three Flates +/// inflate, which hex then skips as white space, ends at nothing). Past it the run is refused +/// ([`RedactError::ImagesPastAllowance`](crate::RedactError::ImagesPastAllowance)). +pub(crate) fn content(run: &mut Run, doc: &Document, num: u32, images: &mut ImageBudget) -> Result, crate::RedactError> { let Ok(object) = doc.try_get(num) else { return Ok(None) }; let Object::Stream(s) = &*object else { return Ok(None) }; let readable = run.readable(doc, s)?; - let data = doc.with_direct_parms(&readable).decoded_within(MAX_JBIG2_GLOBALS); + let refused = images.refused(); + let data = images.decode_filters(doc, &readable, MAX_JBIG2_GLOBALS); + if data.is_err() && images.refused() > refused { + return Err(crate::RedactError::ImagesPastAllowance); + } let mut hash = DefaultHasher::new(); match &data { Ok(data) => { @@ -480,6 +490,7 @@ fn matches_left( left: &HashSet, contents: &HashSet, read: &mut HashMap>, + images: &mut ImageBudget, ) -> Result { if left.contains(&n) { return Ok(true); @@ -487,7 +498,7 @@ fn matches_left( let held = match read.get(&n) { Some(held) => *held, None => { - let held = content(run, doc, n)?; + let held = content(run, doc, n, images)?; read.insert(n, held); held } @@ -505,14 +516,14 @@ fn matches_left( /// An image another page still draws hasn't left: it keeps its pixels, so its dictionary keeps nothing /// more. /// -/// What it reads and decodes is counted against [`RUN_BUDGET`] for the whole run, and each image -/// is decoded within `images`, the run's image allowance. +/// What it reads and decodes is counted against [`RUN_BUDGET`] for the whole run, and each +/// dictionary and image is decoded within `images`, the run's image allowance. /// /// Errors: [`RedactError::SharedJbig2Globals`](crate::RedactError::SharedJbig2Globals) when a dictionary would /// still be kept, as what names it can't be stored without it (counted) or something else refers to /// it (0 when that is all); [`RedactError::SharedJbig2Budget`](crate::RedactError::SharedJbig2Budget) when /// finding that would spend more than the budget; [`RedactError::ImagesPastAllowance`](crate::RedactError::ImagesPastAllowance) -/// when decoding the images would spend more than `images` has left. Such an image may still be drawn, on a page no mark +/// when decoding the dictionaries and images would spend more than `images` has left. Such an image may still be drawn, on a page no mark /// reached, so it isn't removed, and `doc` is part changed by then: a caller works on a copy it can drop. pub(crate) fn unshare(doc: &mut Document, before: &HashSet, images: &mut ImageBudget) -> Result { let now = kept(doc); @@ -536,7 +547,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet, images: &mut Im } let mut contents: HashSet = HashSet::new(); for &n in &dictionaries { - if let Some(c) = content(&mut run, doc, n)? { + if let Some(c) = content(&mut run, doc, n, images)? { contents.insert(c); } } @@ -562,7 +573,7 @@ pub(crate) fn unshare(doc: &mut Document, before: &HashSet, images: &mut Im None => { let mut found = Vec::new(); for &n in list.iter() { - if matches_left(&mut run, doc, n, &left, &contents, &mut read)? { + if matches_left(&mut run, doc, n, &left, &contents, &mut read, images)? { found.push(n); } } diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index 657ac1e5a8..fadb70adb7 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -1981,7 +1981,7 @@ fn an_image_is_charged_the_dictionaries_it_names() { let image = "/Type /XObject /Subtype /Image /Width 8 /Height 8 /BitsPerComponent 1 /Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 2 0 R >>"; let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), stream("", vec![0u8; 40_000]), stream(image, b"not JBIG2")]); let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); - crate::globals::content(&mut run, &doc, 2).unwrap(); + crate::globals::content(&mut run, &doc, 2, &mut pdfcraft_cos::ImageBudget::with_work(u64::MAX)).unwrap(); assert_eq!(run.spent, 10, "40,000 bytes: a unit and nine whole 4 KiB"); let Object::Stream(s) = &*doc.get(ObjRef::new(3, 0)) else { panic!("object 3 is the image") }; assert!(matches!(crate::globals::without_dictionary(&mut run, &doc, s, &mut pdfcraft_cos::ImageBudget::with_work(u64::MAX)), Ok(None))); @@ -1989,6 +1989,38 @@ fn an_image_is_charged_the_dictionaries_it_names() { assert_eq!(run.spent, 24); } +/// A dictionary pays the run's image allowance for what every stage of its filters reads and makes, +/// not for what the last one makes: two Flates inflate a MiB of zero bytes, which hex skips as white +/// space, so it decodes to nothing. Past the allowance the run is refused, through `unshare` too. +#[test] +fn a_dictionary_pays_for_every_stage_of_its_filters() { + let hollow = pdfcraft_filters::encode_flate(&pdfcraft_filters::encode_flate(&vec![0u8; 1 << 20])); + let dict = || stream("/Filter [/FlateDecode /FlateDecode /ASCIIHexDecode]", hollow.clone()); + let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), dict()]); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); + let mut images = pdfcraft_cos::ImageBudget::with_work(u64::MAX); + assert!(crate::globals::content(&mut run, &doc, 2, &mut images).unwrap().is_some()); + assert_eq!(run.spent, 1, "it decodes to nothing: a unit"); + let paid = u64::MAX - images.work_left(); + assert!(paid > 1 << 20, "the MiB the second Flate made is paid for: {paid} steps"); + let mut run = crate::globals::Run::new(crate::globals::RUN_BUDGET); + let refused = crate::globals::content(&mut run, &doc, 2, &mut pdfcraft_cos::ImageBudget::with_work(1 << 20)); + assert_eq!(refused, Err(RedactError::ImagesPastAllowance)); + + // An image naming it leaves its page: finding copies of the dictionary reads it. + let image = "/Type /XObject /Subtype /Image /Width 8 /Height 8 /BitsPerComponent 1 /Filter /JBIG2Decode /DecodeParms << /JBIG2Globals 8 0 R >>"; + let mut doc = one_page(b"q 8 0 0 8 0 0 cm /Im Do Q", "/XObject << /Im 7 0 R >>", vec![stream(image, b""), dict()]); + let before = crate::globals::kept(&doc); + let page = pdfcraft_model::pages(&doc)[0].obj; + doc.update_dict(page, |d| { + d.remove(b"Resources"); + }) + .unwrap(); + let refused = crate::globals::unshare(&mut doc.clone(), &before, &mut pdfcraft_cos::ImageBudget::with_work(1 << 20)); + assert_eq!(refused, Err(RedactError::ImagesPastAllowance)); + assert_eq!(crate::globals::unshare(&mut doc, &before, &mut pdfcraft_cos::ImageBudget::with_work(u64::MAX)), Ok(0)); +} + /// What can't be decoded is charged as much as it was allowed to decode, so going past a limit /// costs as much as reaching it: a dictionary `MAX_JBIG2_GLOBALS` (here one behind a filter that /// isn't decoded here), an image its allowance. @@ -1998,8 +2030,9 @@ fn a_decode_that_fails_is_charged_what_it_was_allowed() { let doc = pdf(vec![b"<< /Type /Catalog >>".to_vec(), stream("/Filter /DCTDecode", b"not a JPEG"), stream(image, b"not JBIG2")]); let units = pdfcraft_cos::MAX_JBIG2_GLOBALS / 4096 + 1; let run = |budget| crate::globals::Run::new(budget); - assert!(crate::globals::content(&mut run(units), &doc, 2).is_ok()); - assert_eq!(crate::globals::content(&mut run(units - 1), &doc, 2), Err(RedactError::SharedJbig2Budget)); + let images = || pdfcraft_cos::ImageBudget::with_work(u64::MAX); + assert!(crate::globals::content(&mut run(units), &doc, 2, &mut images()).is_ok()); + assert_eq!(crate::globals::content(&mut run(units - 1), &doc, 2, &mut images()), Err(RedactError::SharedJbig2Budget)); // The image's allowance: (one byte a row of 8, and a byte a row) × 3 a byte, and 4096: 4144 // bytes, two units. let Object::Stream(s) = &*doc.get(ObjRef::new(3, 0)) else { panic!("object 3 is the image") }; From 5bef9b0e7176001c6816ad1da4e36fa4b18ff6d8 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 18:32:18 +0100 Subject: [PATCH 18/27] M8: the redact README says dictionaries are decoded within the allowance too Since b1f221611 the dictionaries read to know copies of them are decoded through the run's ImageBudget, every filter stage paid, as the images stored again are. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 4dc1c2ad2c..1f1d11f0e3 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -37,8 +37,9 @@ clear_marks(&mut doc, None)?; // remove marks without app or a ring of references or a chain of more than 32, may be read again), and what it fetches, remembers and decodes (a decode that fails counting as much as it was allowed, and an image the dictionaries it names) counts against one budget for the run; past it the redaction is refused (`RedactError::SharedJbig2Budget`). The - images stored again are decoded within the run's image allowance, the JBIG2 decoder's own work - included, after what clearing spent: past it `apply` is refused as one clearing too many images is + images stored again, and the dictionaries read to know copies of them, are decoded within the + run's image allowance, every filter stage and the JBIG2 decoder's own work included, after what + clearing spent: past it `apply` is refused as one clearing too many images is (`RedactError::ImagesPastAllowance`), and `remove_hidden`, which chose no pages, as one past the run's budget. Codecs redaction doesn't decode (DCT, JPX) are removed whole (fail-closed), and so is a JBIG2 image whose page isn't its size (the renderer From 4826ca6ebca2b07ba4cb348cfd93c632be3830a7 Mon Sep 17 00:00:00 2001 From: Grzegorz Martens Date: Mon, 2 Nov 2026 19:10:17 +0100 Subject: [PATCH 19/27] M8: an image stored again past the allowance has a test of its own again Since b1f221611 the dictionary is read within the allowance too, so the test that gave unshare no allowance was refused reading the dictionary, before the image was decoded: turning the image's refusal into "can't be read" left every test green (Claims review of 5bef9b0e7). The test now also gives exactly what reading the dictionary costs, so the image is what is refused. The same review: the README lists the other images that can't be stored again (more than one component, no size) and names ImageBudget::decode, not Document::decoded_image_within, which #5569 made private; SharedJbig2Budget's doc and remove_hidden's comment name the dictionaries read to know copies too. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AmsdeUCdTaD72jkDw2pJKY --- crates/redact/README.md | 7 ++++--- crates/redact/src/lib.rs | 5 +++-- crates/redact/src/sanitize.rs | 3 ++- crates/redact/src/tests.rs | 15 ++++++++++----- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 1f1d11f0e3..698e23c131 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -19,7 +19,7 @@ clear_marks(&mut doc, None)?; // remove marks without app inside; - **images**: fully covered → removed; partly covered → a copy with the covered pixels cleared (8/16-bit and 1/2/4-bit images, image masks; Flate/LZW/RL/A85/AHx, and CCITT fax or JBIG2 as - the last filter, decoded through `Document::decoded_image_within`, a JBIG2 image with its + the last filter, decoded through `ImageBudget::decode`, a JBIG2 image with its `/JBIG2Globals`; `/DecodeParms` given by reference, numbers inside included, count). The copy is Flate, or for a one-bit image of one component (a scan as a rule) CCITT Group 4, or JBIG2 when it was JBIG2, whichever is smallest as stored, coded from the cleared samples; it refers to @@ -27,8 +27,9 @@ clear_marks(&mut doc, None)?; // remove marks without app those of marks under the area included, so whatever left the document, by any route, takes its globals with it on the full rewrite: anything else the save would keep that names them, or a copy of them (the same bytes once decoded), is stored again without them, as a copy is (Flate, Group 4 or JBIG2, - whichever is smallest; `Report::images_restored`). When one can't be (it isn't JBIG2, it's past - the size redaction clears, its data is in another file, or it can't be decoded), or something else refers to them, `apply` + whichever is smallest; `Report::images_restored`). When one can't be (it isn't JBIG2, its colour + space is known to have more than one component, it has no size or one past what redaction + clears, its data is in another file, or it can't be decoded), or something else refers to them, `apply` refuses and changes nothing (`RedactError::SharedJbig2Globals`). `remove_hidden` takes them out the same way. Another dictionary holding the same shapes in other bytes isn't found, nor is a copy nothing names, and an image that hasn't left keeps its globals with its pixels: one another page diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index 7d91ef963d..49f46e51e1 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -84,8 +84,9 @@ pub enum RedactError { /// Finding what shares JBIG2 symbol data with what is being removed would read more of the file /// than one run allows (`globals::RUN_BUDGET`): its images and parameters refer to the same /// objects in more ways than are checked, or hold more than is decoded. Removing hidden - /// information says so too when storing those images again would decode more than its image - /// allowance, which no choice of pages makes smaller. + /// information says so too when the dictionaries it reads to know copies, or the images it + /// stores again, would decode more than its image allowance, which no choice of pages makes + /// smaller. #[error( "finding and storing again what shares JBIG2 symbol data with something being removed would read more of this file than one run allows, so it can't be shown that the data leaves with it; this was refused" )] diff --git a/crates/redact/src/sanitize.rs b/crates/redact/src/sanitize.rs index aef6c71e26..b560184b1b 100644 --- a/crates/redact/src/sanitize.rs +++ b/crates/redact/src/sanitize.rs @@ -689,7 +689,8 @@ pub fn remove_hidden(doc: &mut Document, which: &[Hidden]) -> Result