From c251b3a0a725c460774e85b717ec878b52a1e286 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 15:28:23 +0000 Subject: [PATCH 1/2] fix(redact): read inherited forms once for all names in resource cleanup unlink_doomed and unlink_from_forms asked, name by name, whether the remaining content still draws an XObject, and each question parsed the content and decoded every form without /Resources of its own again. A holder with many aliases of one removed image, drawn by a large inheriting form, multiplied that decoding by the number of aliases. names_drawn now works out once per content which names it draws, directly or through such forms (each read once), and whether any of it could not be read; every alias is answered from that set. A name not in the set while something could not be read still fails closed. --- crates/redact/src/interp/prune.rs | 79 +++++++++++-------- .../redact/src/tests/surviving_references.rs | 26 ++++++ 2 files changed, 71 insertions(+), 34 deletions(-) diff --git a/crates/redact/src/interp/prune.rs b/crates/redact/src/interp/prune.rs index 6ca5ea2803..d8742a73c5 100644 --- a/crates/redact/src/interp/prune.rs +++ b/crates/redact/src/interp/prune.rs @@ -20,33 +20,45 @@ pub(crate) fn prune(xo: &mut Dict, replaced: &[Vec], content: &[Vec]) { } } -/// Whether `content` draws the XObject `name` of the resource dictionary `xo`: directly, or -/// through a form that has no /Resources of its own (such a form draws through its caller's). -/// `None` when that cannot be told (a form that will not decode, or nesting past the limit). -fn content_draws(doc: &Document, content: &[Vec], xo: &Dict, name: &[u8], depth: usize, seen: &mut HashSet) -> Option { - let joined = Pieces::join(content); - let ops = joined.parse().ops; - for op in ops.iter().filter(|op| op.is("Do")) { - let Some(n) = op.last_name() else { continue }; - if n == name { - return Some(true); - } - let Some(r) = xo.get(n).and_then(Object::as_ref) else { continue }; - let obj = doc.get(r); - let Object::Stream(s) = &*obj else { continue }; - if s.dict.name(b"Subtype") != Some(b"Form") || s.dict.contains(b"Resources") || !seen.insert(r) { - continue; - } - if depth >= MAX_DEPTH { - return None; - } - let Ok(data) = s.decoded() else { return None }; - match content_draws(doc, std::slice::from_ref(&data), xo, name, depth + 1, seen) { - Some(false) => {} - other => return other, +/// The XObject names of the resource dictionary `xo` that `content` draws: directly, or through +/// a form that has no /Resources of its own (such a form draws through its caller's). Each such +/// form is read once, however many names are asked about. The flag says whether some of what +/// `content` draws could not be read (a form that will not decode, or nesting past the limit), so +/// a name missing from the set may still be drawn. +fn names_drawn(doc: &Document, content: &[Vec], xo: &Dict) -> (HashSet>, bool) { + let mut drawn = HashSet::new(); + let mut seen = HashSet::new(); + let mut pending = vec![(Pieces::join(content).parse().ops, 0)]; + while let Some((ops, depth)) = pending.pop() { + for n in ops.iter().filter(|op| op.is("Do")).filter_map(Op::last_name) { + if !drawn.insert(n.to_vec()) { + continue; + } + let Some(r) = xo.get(n).and_then(Object::as_ref) else { continue }; + let obj = doc.get(r); + let Object::Stream(s) = &*obj else { continue }; + if s.dict.name(b"Subtype") != Some(b"Form") || s.dict.contains(b"Resources") || !seen.insert(r) { + continue; + } + match s.decoded() { + Ok(data) if depth < MAX_DEPTH => pending.push((pdfcraft_content::parse(&data).ops, depth + 1)), + _ => return (drawn, true), + } } } - Some(false) + (drawn, false) +} + +/// Whether any of `drawn` (from [`names_drawn`]) draws `name`: `None` when none is known to and +/// one could not be read in full. +fn any_draws(drawn: &[(HashSet>, bool)], name: &[u8]) -> Option { + if drawn.iter().any(|(names, _)| names.contains(name)) { + Some(true) + } else if drawn.iter().any(|(_, unknown)| *unknown) { + None + } else { + Some(false) + } } /// Unlink the XObjects a rewrite dropped or replaced (`doomed`) from every page-tree resource @@ -112,15 +124,10 @@ pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_prope contents.push(page_streams(doc, &page, index.get(user).copied().unwrap_or(0))?.1); } let mut pruned = false; + // What each page draws through this dictionary, worked out once for all the names. + let drawn: Vec<_> = if names.is_empty() { Vec::new() } else { contents.iter().map(|content| names_drawn(doc, content, &xo)).collect() }; for n in names { - let mut drawn = Some(false); - for content in &contents { - drawn = content_draws(doc, content, &xo, &n, 0, &mut HashSet::new()); - if drawn != Some(false) { - break; - } - } - match drawn { + match any_draws(&drawn, &n) { Some(true) => {} Some(false) => { xo.remove(&n); @@ -230,8 +237,12 @@ fn unlink_from_forms(doc: &mut Document, holders: &[ObjRef], doomed: &HashSet], xo: &mut Dict, doomed_names: &[Vec]) -> Result { let mut pruned = false; + if doomed_names.is_empty() { + return Ok(false); + } + let drawn = [names_drawn(doc, content, xo)]; for name in doomed_names { - match content_draws(doc, content, xo, name, 0, &mut HashSet::new()) { + match any_draws(&drawn, name) { Some(true) => {} Some(false) => pruned |= xo.remove(name).is_some(), None => return Err(CosError::Syntax { offset: 0, detail: "redaction could not fully inspect shared form resources".into() }), diff --git a/crates/redact/src/tests/surviving_references.rs b/crates/redact/src/tests/surviving_references.rs index 477f03f9a5..bfcebd171e 100644 --- a/crates/redact/src/tests/surviving_references.rs +++ b/crates/redact/src/tests/surviving_references.rs @@ -298,3 +298,29 @@ fn type3_font_used_in_a_form_xobject_has_its_redacted_glyph_removed() { assert_eq!(report.glyphs, 1); assert!(!full_save_holds(&doc, b"SECRET_FORM_GLYPH_PAYLOAD"), "redacted glyph program survived"); } + +/// Resource cleanup read a form without /Resources of its own again for every name that could +/// still reach the removed image. Here 3 000 names of one redacted image sit in the inherited +/// /Pages resources, and the other page draws a large such form: it is read once, and every name +/// leaves with the image. +#[test] +fn many_names_of_a_redacted_image_are_unlinked_reading_inherited_forms_once() { + const ALIASES: usize = 3_000; + let aliases: String = (0..ALIASES).map(|i| format!("/A{i} 6 0 R ")).collect(); + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 /Resources 5 0 R >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 8 0 R >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 9 0 R >>".to_vec(), + format!("<< /XObject << /Im 6 0 R /Fm 7 0 R {aliases}>> >>").into_bytes(), + stream("/Type /XObject /Subtype /Image /Width 11 /Height 1 /ColorSpace /DeviceGray /BitsPerComponent 8", b"IMAGESECRET"), + stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"200 200 m 210 210 l S ".repeat(20_000)), + stream("", b"q 100 0 0 100 0 0 cm /Im Do Q"), + stream("", b"/Fm Do"), + ]); + mark(&mut doc, 0, &[[0.0, 0.0, 120.0, 120.0]], ""); + let started = std::time::Instant::now(); + assert_eq!(apply(&mut doc, None).unwrap().images_removed, 1); + assert!(started.elapsed() < std::time::Duration::from_secs(60), "{:?}", started.elapsed()); + assert!(!full_save_holds(&doc, b"IMAGESECRET"), "an alias kept the removed image reachable"); +} From da5a66427635fcb4f856d189fdb8f99fb66b1ab2 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 8 Nov 2026 11:14:23 -0500 Subject: [PATCH 2/2] review: keep reading queued forms after one that will not decode --- crates/redact/src/interp/prune.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/crates/redact/src/interp/prune.rs b/crates/redact/src/interp/prune.rs index d8742a73c5..b8a35c5576 100644 --- a/crates/redact/src/interp/prune.rs +++ b/crates/redact/src/interp/prune.rs @@ -29,6 +29,9 @@ fn names_drawn(doc: &Document, content: &[Vec], xo: &Dict) -> (HashSet], xo: &Dict) -> (HashSet pending.push((pdfcraft_content::parse(&data).ops, depth + 1)), - _ => return (drawn, true), + _ => unknown = true, } } } - (drawn, false) + (drawn, unknown) } /// Whether any of `drawn` (from [`names_drawn`]) draws `name`: `None` when none is known to and