Unlinking undrawn images matches what a reader draws

A page's content streams are read as one, and forms named through a
reference-only object or listed in a Type 3 font's own resources are
read too, so the images they draw stay. A page tree that lists a node
twice is left alone. The pass repeats while names go, so an image only
an unused form drew now leaves with that form.

crates/redact/src/interp/prune.rs

@@ -51,14 +51,18 @@ fn content_draws(doc: &Document, content: &[Vec<u8>], xo: &Dict, name: &[u8], de
 
 /// Every page-tree node with a /Resources of its own, and the pages that draw through it (the
 /// nearest such node above a page, or the page itself). Cycles are cut as in
-/// `pdfcraft_model::pages`.
-fn resource_holders(doc: &Document) -> Vec<(ObjRef, Vec<ObjRef>)> {
+/// `pdfcraft_model::pages`. The flag is false when a node was reached twice (listed under two
+/// parents, or in a cycle) or the walk stopped early: a page may then draw through a holder it
+/// isn't listed under.
+fn resource_holders(doc: &Document) -> (Vec<(ObjRef, Vec<ObjRef>)>, bool) {
     let mut holders: Vec<(ObjRef, Vec<ObjRef>)> = Vec::new();
-    let Some(top) = doc.root().and_then(|root| doc.get(root).as_dict().and_then(|d| d.reference(b"Pages"))) else { return holders };
+    let mut whole = true;
+    let Some(top) = doc.root().and_then(|root| doc.get(root).as_dict().and_then(|d| d.reference(b"Pages"))) else { return (holders, whole) };
     let mut seen = HashSet::new();
     let mut stack: Vec<(ObjRef, Option<usize>)> = vec![(top, None)];
     while let Some((node, mut holder)) = stack.pop() {
         if !seen.insert(node) || seen.len() > 1_000_000 {
+            whole = false;
             continue;
         }
         let obj = doc.get(node);
@@ -75,19 +79,24 @@ fn resource_holders(doc: &Document) -> Vec<(ObjRef, Vec<ObjRef>)> {
             h.1.push(node);
         }
     }
-    holders
+    (holders, whole)
 }
 
 /// Unlink the image and form XObjects nothing draws from the page-tree resource dictionaries
 /// that still name them: Delete Image and Replace Image take an image off the page but leave it
 /// listed there, and so in a full-rewrite save. A name stays when anything that draws through the
 /// dictionary uses it ([`names_drawn`]), and a dictionary whose use can't be told is left as it
-/// is. A dictionary that loses an entry is replaced by a private copy on its node, as in
-/// [`unlink_doomed`]. Returns how many names went.
+/// is, as is every one when a page is listed twice in the page tree. A dictionary that loses an
+/// entry is replaced by a private copy on its node, as in [`unlink_doomed`]. Returns how many
+/// names went.
 pub(crate) fn unlink_undrawn(doc: &mut Document) -> Result<usize, RedactError> {
     let index: HashMap<ObjRef, usize> = pdfcraft_model::pages(doc).iter().enumerate().map(|(i, p)| (p.obj, i)).collect();
+    let (holders, whole) = resource_holders(doc);
+    if !whole {
+        return Ok(0);
+    }
     let mut n = 0;
-    for (node, users) in resource_holders(doc) {
+    for (node, users) in holders {
         let Some(mut res) = doc.get(node).as_dict().and_then(|d| d.get(b"Resources")).and_then(|r| doc.resolve(r).as_dict().cloned()) else {
             continue;
         };
@@ -102,15 +111,22 @@ pub(crate) fn unlink_undrawn(doc: &mut Document) -> Result<usize, RedactError> {
         if listed.is_empty() {
             continue;
         }
-        let Some(drawn) = names_drawn(doc, &res, &users, &index) else { continue };
         let mut pruned = false;
-        for name in listed.iter().filter(|name| !drawn.contains(*name)) {
-            xo.remove(name);
+        // A form that goes may have been all that drew another name: look again while names go.
+        for _ in 0..MAX_DEPTH {
+            let Some(drawn) = names_drawn(doc, &res, &users, &index) else { break };
+            let mut gone = 0;
+            for name in listed.iter().filter(|name| !drawn.contains(*name)) {
+                gone += usize::from(xo.remove(name).is_some());
+            }
+            if gone == 0 {
+                break;
+            }
+            n += gone;
             pruned = true;
-            n += 1;
+            res.set(b"XObject".to_vec(), Object::Dict(xo.clone()));
         }
         if pruned {
-            res.set(b"XObject".to_vec(), Object::Dict(xo));
             doc.update_dict(node, |d| d.set(b"Resources".to_vec(), Object::Dict(res)))?;
         }
     }
@@ -139,14 +155,15 @@ fn names_drawn(doc: &Document, res: &Dict, pages: &[ObjRef], index: &HashMap<Obj
     for page in pages {
         let dict = doc.get(*page).as_dict().cloned().unwrap_or_default();
         let (_, content) = page_streams(doc, &dict, index.get(page).copied().unwrap_or(0)).ok()?;
-        content.iter().for_each(|c| note(c));
+        // One stream may end with a name its `Do` begins the next with: read them as one.
+        note(Pieces::join(&content).data());
         let annots = dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default();
         for annot in annots {
             let ap = doc.resolve(&annot).as_dict().and_then(|a| a.get(b"AP")).map(|ap| doc.resolve(ap)).and_then(|ap| ap.as_dict().cloned());
             // Each of /N, /R and /D is an appearance, or a dictionary of them by state.
             for (_, look) in ap.unwrap_or_default().iter() {
                 match look.as_ref() {
-                    Some(r) if matches!(&*doc.get(r), Object::Stream(_)) => streams.push(r),
+                    Some(r) if matches!(&*doc.resolve(look), Object::Stream(_)) => streams.push(r),
                     _ => streams.extend(doc.resolve(look).as_dict().into_iter().flat_map(|states| states.iter().filter_map(|(_, v)| v.as_ref()))),
                 }
             }
@@ -157,14 +174,23 @@ fn names_drawn(doc: &Document, res: &Dict, pages: &[ObjRef], index: &HashMap<Obj
     let mut looked = 0;
     loop {
         while let Some(dict) = dicts.get(looked) {
-            streams.extend(drawn_through(doc, dict));
+            let (found, fonts) = drawn_through(doc, dict);
             looked += 1;
+            streams.extend(found);
+            // Type 3 fonts' own resources, each look charged like a form so fonts naming each
+            // other can't go round for ever.
+            for font_res in fonts {
+                if !budget.enter_form() {
+                    return None;
+                }
+                dicts.push(font_res);
+            }
         }
         let Some(r) = streams.pop() else { break };
         if !read.insert(r) {
             continue;
         }
-        let obj = doc.get(r);
+        let obj = doc.resolve(&Object::Ref(r));
         let Object::Stream(s) = &*obj else { continue };
         if !budget.enter_form() {
             return None;
@@ -178,28 +204,32 @@ fn names_drawn(doc: &Document, res: &Dict, pages: &[ObjRef], index: &HashMap<Obj
 }
 
 /// The content streams the resource dictionary `res` lists: its form XObjects, its tiling
-/// patterns, its Type 3 fonts' glyph procedures and its soft masks' groups.
-fn drawn_through(doc: &Document, res: &Dict) -> Vec<ObjRef> {
-    let is_stream = |r: &ObjRef| matches!(&*doc.get(*r), Object::Stream(_));
+/// patterns, its Type 3 fonts' glyph procedures and its soft masks' groups. Then the /Resources
+/// of those Type 3 fonts, which their glyphs look names up in first. An object that holds only a
+/// reference stands for what it names, as the renderer reads it.
+fn drawn_through(doc: &Document, res: &Dict) -> (Vec<ObjRef>, Vec<Dict>) {
+    let is_stream = |r: &ObjRef| matches!(&*doc.resolve(&Object::Ref(*r)), Object::Stream(_));
     let mut found: Vec<ObjRef> = Vec::new();
+    let mut fonts: Vec<Dict> = Vec::new();
     for (_, x) in res_dict(doc, res, b"XObject").iter() {
-        found.extend(x.as_ref().filter(|r| matches!(&*doc.get(*r), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Form"))));
+        found.extend(x.as_ref().filter(|_| matches!(&*doc.resolve(x), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Form"))));
     }
     for (_, p) in res_dict(doc, res, b"Pattern").iter() {
-        found.extend(p.as_ref().filter(|r| matches!(&*doc.get(*r), Object::Stream(s) if s.dict.int(b"PatternType") == Some(1))));
+        found.extend(p.as_ref().filter(|_| matches!(&*doc.resolve(p), Object::Stream(s) if s.dict.int(b"PatternType") == Some(1))));
     }
     for (_, f) in res_dict(doc, res, b"Font").iter() {
         let font = doc.resolve(f);
         let Some(font) = font.as_dict().filter(|f| f.name(b"Subtype") == Some(b"Type3")) else { continue };
         let procs = font.get(b"CharProcs").map(|c| doc.resolve(c)).and_then(|c| c.as_dict().cloned()).unwrap_or_default();
         found.extend(procs.iter().filter_map(|(_, v)| v.as_ref()).filter(is_stream));
+        fonts.extend(font.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()));
     }
     for (_, g) in res_dict(doc, res, b"ExtGState").iter() {
         let gs = doc.resolve(g);
         let mask = gs.as_dict().and_then(|g| g.get(b"SMask")).map(|m| doc.resolve(m));
         found.extend(mask.as_ref().and_then(|m| m.as_dict()).and_then(|m| m.reference(b"G")).filter(is_stream));
     }
-    found
+    (found, fonts)
 }
 
 /// Unlink the XObjects a rewrite dropped or replaced (`doomed`) from every page-tree resource
@@ -220,7 +250,7 @@ pub(crate) fn unlink_doomed(doc: &mut Document, doomed: &[ObjRef], retired_prope
         return Ok(());
     }
     let doomed: HashSet<ObjRef> = doomed.iter().copied().collect();
-    let holders = resource_holders(doc);
+    let (holders, _) = resource_holders(doc);
     let index: HashMap<ObjRef, usize> = pdfcraft_model::pages(doc).iter().enumerate().map(|(i, p)| (p.obj, i)).collect();
     let holder_nodes: Vec<ObjRef> = holders.iter().map(|(node, _)| *node).collect();
     for (node, users) in holders {

crates/redact/src/tests.rs

@@ -4228,31 +4228,56 @@ fn gray_image(data: &[u8]) -> Vec<u8> {
 }
 
 /// An image Delete Image took off the page is still listed in its resources: Remove Hidden
-/// Information unlinks it, so the full rewrite leaves it out. Images a page still draws stay,
-/// directly, through a form without /Resources, or on another page sharing the dictionary.
+/// Information unlinks it, so the full rewrite leaves it out, as it does one only an unused form
+/// draws. Images a page still draws stay, directly, through a form without /Resources, or on
+/// another page sharing the dictionary, its name and `Do` in two content streams.
 #[test]
 fn images_no_page_draws_leave_with_hidden_information() {
     let bytes = hidden_information_removed(vec![
         b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(),
         b"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>".to_vec(),
         b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 5 0 R /Resources 7 0 R >>".to_vec(),
-        b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 6 0 R /Resources 7 0 R >>".to_vec(),
+        b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents [6 0 R 15 0 R] /Resources 7 0 R >>".to_vec(),
         stream("", b"q 50 0 0 50 20 20 cm /Shown Do Q /Fm Do"),
-        stream("", b"q 50 0 0 50 20 20 cm /Other Do Q"),
-        b"<< /XObject << /Shown 8 0 R /Deleted 9 0 R /Nested 10 0 R /Other 11 0 R /Fm 12 0 R >> >>".to_vec(),
+        stream("", b"q 50 0 0 50 20 20 cm /Other"),
+        b"<< /XObject << /Shown 8 0 R /Deleted 9 0 R /Nested 10 0 R /Other 11 0 R /Fm 12 0 R /Unused 13 0 R /Orphan 14 0 R >> >>".to_vec(),
         gray_image(b"SHOWNIMAGEBYTES"),
         gray_image(b"DELETEDIMAGEBYTES"),
         gray_image(b"NESTEDIMAGEBYTES"),
         gray_image(b"OTHERPAGEIMAGE"),
         stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"q 10 0 0 10 200 200 cm /Nested Do Q"),
+        stream("/Type /XObject /Subtype /Form /BBox [0 0 300 300]", b"/Orphan Do"),
+        gray_image(b"ORPHANIMAGEBYTES"),
+        stream("", b"Do Q"),
     ]);
     let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w == needle);
     assert!(!has(b"DELETEDIMAGEBYTES"), "the deleted image survived the save");
+    assert!(!has(b"ORPHANIMAGEBYTES"), "an image only an unused form draws survived the save");
     for kept in [&b"SHOWNIMAGEBYTES"[..], b"NESTEDIMAGEBYTES", b"OTHERPAGEIMAGE"] {
         assert!(has(kept), "{} is drawn and must stay", String::from_utf8_lossy(kept));
     }
 }
 
+/// A page listed under two /Pages nodes draws through either one's resources, whichever a reader
+/// comes to it from: neither loses the image it draws.
+#[test]
+fn a_page_listed_twice_keeps_both_parents_images() {
+    let bytes = hidden_information_removed(vec![
+        b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(),
+        b"<< /Type /Pages /Kids [3 0 R 4 0 R] /Count 2 >>".to_vec(),
+        b"<< /Type /Pages /Parent 2 0 R /Kids [5 0 R] /Count 1 /Resources << /XObject << /Im 7 0 R >> >> >>".to_vec(),
+        b"<< /Type /Pages /Parent 2 0 R /Kids [5 0 R] /Count 1 /Resources << /XObject << /Im 8 0 R >> >> >>".to_vec(),
+        b"<< /Type /Page /Parent 3 0 R /MediaBox [0 0 300 300] /Contents 6 0 R >>".to_vec(),
+        stream("", b"q 50 0 0 50 20 20 cm /Im Do Q"),
+        gray_image(b"FIRSTPARENTIMAGE"),
+        gray_image(b"SECONDPARENTIMAGE"),
+    ]);
+    let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w == needle);
+    for kept in [&b"FIRSTPARENTIMAGE"[..], b"SECONDPARENTIMAGE"] {
+        assert!(has(kept), "{} is drawn and must stay", String::from_utf8_lossy(kept));
+    }
+}
+
 /// Unlinking an image nothing draws is a removal of its own: with nothing else to remove, it is
 /// still applied, not reported as nothing to do.
 #[test]
@@ -4271,16 +4296,17 @@ fn unlinking_alone_still_applies() {
 
 /// A name a pattern, a form, an appearance or a soft mask doesn't define is looked up in the
 /// page's resources, /Resources of its own or not: an image drawn only that way is still shown,
-/// so it stays. One no content names goes.
+/// so it stays, as one drawn by a form named through an object that holds only a reference. One
+/// no content names goes.
 #[test]
 fn images_drawn_through_the_page_resources_stay() {
     let bytes = hidden_information_removed(vec![
         b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(),
         b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
         b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R /Annots [6 0 R] >>".to_vec(),
-        stream("", b"q /Pattern cs /Pt scn 0 0 10 10 re f Q /Own Do /Loop Do q /Gs gs 0 0 5 5 re f Q"),
-        b"<< /XObject << /ByPattern 7 0 R /ByOwnForm 8 0 R /ByAppearance 9 0 R /ByMask 10 0 R /Deleted 11 0 R /Own 12 0 R /Loop 16 0 R >> \
-           /Pattern << /Pt 13 0 R >> /ExtGState << /Gs << /SMask << /S /Luminosity /G 14 0 R >> >> >> >>"
+        stream("", b"q /Pattern cs /Pt scn 0 0 10 10 re f Q /Own Do /Loop Do q /Gs gs 0 0 5 5 re f Q /Chained Do"),
+        b"<< /XObject << /ByPattern 7 0 R /ByOwnForm 8 0 R /ByAppearance 9 0 R /ByMask 10 0 R /Deleted 11 0 R /Own 12 0 R /Loop 16 0 R \
+           /Chained 17 0 R /ByChain 19 0 R >> /Pattern << /Pt 13 0 R >> /ExtGState << /Gs << /SMask << /S /Luminosity /G 14 0 R >> >> >> >>"
             .to_vec(),
         b"<< /Type /Annot /Subtype /Square /Rect [0 0 50 50] /AP << /N 15 0 R >> >>".to_vec(),
         gray_image(b"BYPATTERNIMAGE"),
@@ -4294,16 +4320,19 @@ fn images_drawn_through_the_page_resources_stay() {
         stream("/Type /XObject /Subtype /Form /BBox [0 0 50 50]", b"/ByAppearance Do"),
         // Draws itself: read once.
         stream("/Type /XObject /Subtype /Form /BBox [0 0 10 10] /Resources << /XObject << /Loop 16 0 R >> >>", b"/Loop Do"),
+        b"18 0 R".to_vec(),
+        stream("/Type /XObject /Subtype /Form /BBox [0 0 10 10]", b"/ByChain Do"),
+        gray_image(b"BYCHAINIMAGE"),
     ]);
     let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w == needle);
     assert!(!has(b"DELETEDIMAGEBYTES"), "the deleted image survived the save");
-    for kept in [&b"BYPATTERNIMAGE"[..], b"BYOWNFORMIMAGE", b"BYAPPEARANCEIMAGE", b"BYMASKIMAGE"] {
+    for kept in [&b"BYPATTERNIMAGE"[..], b"BYOWNFORMIMAGE", b"BYAPPEARANCEIMAGE", b"BYMASKIMAGE", b"BYCHAINIMAGE"] {
         assert!(has(kept), "{} is drawn and must stay", String::from_utf8_lossy(kept));
     }
 }
 
-/// A Type 3 glyph draws through the page's resources too: its image stays, and the dictionary
-/// is still cleaned of what nothing draws.
+/// A Type 3 glyph draws through the page's resources too, as does a form in the font's own
+/// resources: their images stay, and the dictionary is still cleaned of what nothing draws.
 #[test]
 fn images_drawn_by_type3_glyphs_stay() {
     let bytes = hidden_information_removed(vec![
@@ -4311,15 +4340,18 @@ fn images_drawn_by_type3_glyphs_stay() {
         b"<< /Type /Pages /Kids [3 0 R] /Count 1 >>".to_vec(),
         b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources 5 0 R >>".to_vec(),
         stream("", b"BT /T3 12 Tf (a) Tj ET"),
-        b"<< /XObject << /ByGlyph 7 0 R /Deleted 8 0 R >> /Font << /T3 6 0 R >> >>".to_vec(),
+        b"<< /XObject << /ByGlyph 7 0 R /Deleted 8 0 R /ByFontForm 11 0 R >> /Font << /T3 6 0 R >> >>".to_vec(),
         b"<< /Type /Font /Subtype /Type3 /FontBBox [0 0 10 10] /FontMatrix [0.1 0 0 0.1 0 0] /CharProcs << /a 9 0 R >> \
-           /Encoding << /Differences [97 /a] >> /FirstChar 97 /LastChar 97 /Widths [10] >>"
+           /Encoding << /Differences [97 /a] >> /FirstChar 97 /LastChar 97 /Widths [10] /Resources << /XObject << /Fx 10 0 R >> >> >>"
             .to_vec(),
         gray_image(b"BYGLYPHIMAGE"),
         gray_image(b"DELETEDIMAGEBYTES"),
-        stream("", b"10 0 d0 /ByGlyph Do"),
+        stream("", b"10 0 d0 /ByGlyph Do /Fx Do"),
+        stream("/Type /XObject /Subtype /Form /BBox [0 0 10 10]", b"/ByFontForm Do"),
+        gray_image(b"BYFONTFORMIMAGE"),
     ]);
     let has = |needle: &[u8]| bytes.windows(needle.len()).any(|w| w == needle);
     assert!(!has(b"DELETEDIMAGEBYTES"), "the deleted image survived the save");
     assert!(has(b"BYGLYPHIMAGE"), "the glyph's image is drawn and must stay");
+    assert!(has(b"BYFONTFORMIMAGE"), "the image a form in the font's resources draws must stay");
 }