Merge #5969: fix(redact): read inherited forms once for all names in resource cleanup

# Conflicts:
#	crates/redact/src/tests/surviving_references.rs

crates/redact/src/interp/prune.rs

@@ -20,33 +20,48 @@ pub(crate) fn prune(xo: &mut Dict, replaced: &[Vec<u8>], content: &[Vec<u8>]) {
     }
 }
 
-/// 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<u8>], xo: &Dict, name: &[u8], depth: usize, seen: &mut HashSet<ObjRef>) -> Option<bool> {
-    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<u8>], xo: &Dict) -> (HashSet<Vec<u8>>, bool) {
+    let mut drawn = HashSet::new();
+    let mut seen = HashSet::new();
+    let mut pending = vec![(Pieces::join(content).parse().ops, 0)];
+    // A form that will not decode makes the answer uncertain, but it does not stop the forms
+    // already queued from being read: a name drawn only inside one of those need not be unknown.
+    let mut unknown = false;
+    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)),
+                _ => unknown = true,
+            }
         }
     }
-    Some(false)
+    (drawn, unknown)
+}
+
+/// 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<Vec<u8>>, bool)], name: &[u8]) -> Option<bool> {
+    if drawn.iter().any(|(names, _)| names.contains(name)) {
+        Some(true)
+    } else if drawn.iter().any(|(_, unknown)| *unknown) {
+        None
+    } else {
+        Some(false)
+    }
 }
 
 /// Every page-tree node with a /Resources of its own, and the pages that draw through it (the
@@ -271,15 +286,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);
@@ -389,8 +399,12 @@ fn unlink_from_forms(doc: &mut Document, holders: &[ObjRef], doomed: &HashSet<Ob
 
 fn prune_doomed_xobjects(doc: &Document, content: &[Vec<u8>], xo: &mut Dict, doomed_names: &[Vec<u8>]) -> Result<bool, CosError> {
     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() }),

crates/redact/src/tests/surviving_references.rs

@@ -577,3 +577,29 @@ fn a_type3_glyph_a_soft_mask_shows_is_kept() {
     assert!(full_save_holds(&doc, b"MASK_GLYPH_B"), "the glyph the soft mask shows was unlinked");
     assert!(full_save_holds(&doc, b"PAGE_GLYPH_C"), "the glyph the page still shows was unlinked");
 }
+
+/// 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");
+}