M8: tag cleanup keeps inherited pages, fails closed over replaced streams and in-place elements (#3302)

Review follow-ups on #3285: walk /K before the parent and ID trees and look up
an element's inherited /Pg through /P; drop the structure tree when a cut-short
walk could leave MCRs naming a replaced stream; drop it when an element written
in place tags a removed annotation.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Gv2WwdgjmBz33GWkwgNn3Q
This commit is contained in:
mtlcl410
2026-10-19 01:33:42 +08:00
co-authored by Claude Opus 5.5
parent ce3c0e46c9
commit f62bf71a77
2 changed files with 134 additions and 12 deletions
+59 -12
View File
@@ -130,9 +130,10 @@ pub(crate) fn clean_within(
};
// Indirect elements with the page they inherit. Elements are reached from the root's /K, and
// also from the parent tree and the ID tree: one listed only there still owns marked content
// (#3302).
// (#3302). The stack pops /K first, so an element reached through /K gets the page it inherits
// from its ancestors before a tree reaches it without one.
let mut todo: Vec<(Object, Option<ObjRef>)> =
[&b"K"[..], b"ParentTree", b"IDTree"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, None)).collect();
[&b"IDTree"[..], b"ParentTree", b"K"].iter().filter_map(|key| root.get(key).cloned()).map(|k| (k, None)).collect();
let mut seen = HashSet::new();
let mut hits: Vec<(ObjRef, Dict, Option<ObjRef>, bool)> = Vec::new();
while let Some((k, pg)) = todo.pop() {
@@ -145,7 +146,13 @@ pub(crate) fn clean_within(
continue;
}
if seen.len() > limit {
return clear_every_element(doc);
let n = clear_every_element(doc)?;
if stream.is_some() {
// Marked-content references the walk didn't reach still name the
// original stream through /Stm, which a full save would then keep.
drop_tree(doc)?;
}
return Ok(n);
}
let object = doc.get(r);
if let Object::Array(a) = &*object {
@@ -158,7 +165,7 @@ pub(crate) fn clean_within(
todo.extend(tree_node_entries(&d).into_iter().map(|x| (x, pg)));
continue;
}
let pg = d.get(b"Pg").and_then(Object::as_ref).or(pg);
let pg = d.get(b"Pg").and_then(Object::as_ref).or(pg).or_else(|| inherited_page(doc, &d));
let kids: Vec<Object> = match d.get(b"K").map(|k| doc.resolve(k)) {
Some(o) => match &*o {
Object::Array(a) => a.clone(),
@@ -251,6 +258,33 @@ fn clear_every_element(doc: &mut Document) -> Result<usize, pdfcraft_cos::CosErr
Ok(elements.len())
}
/// The page an element inherits (§14.7.4.4): the nearest `/Pg` among its ancestors, following
/// `/P` at most [`MAX_PARENTS`] steps.
fn inherited_page(doc: &Document, d: &Dict) -> Option<ObjRef> {
let mut parent = d.get(b"P").and_then(Object::as_ref);
for _ in 0..MAX_PARENTS {
let p = doc.get(parent?);
let p = p.as_dict()?;
if let Some(pg) = p.get(b"Pg").and_then(Object::as_ref) {
return Some(pg);
}
parent = p.get(b"P").and_then(Object::as_ref);
}
None
}
/// The most ancestors [`inherited_page`] looks through; deeper trees, and cycles, go unanswered.
const MAX_PARENTS: usize = 256;
/// Remove the structure tree, with `/MarkInfo`, which would claim tags there no longer are.
fn drop_tree(doc: &mut Document) -> Result<(), pdfcraft_cos::CosError> {
let Some(catalog) = doc.root() else { return Ok(()) };
doc.update_dict(catalog, |c| {
c.remove(b"StructTreeRoot");
c.remove(b"MarkInfo");
})
}
/// Whether `o` is an object reference (`/Type /OBJR`, direct or indirect) to one of `gone`.
fn refers_to(doc: &Document, o: &Object, gone: &HashSet<ObjRef>) -> bool {
doc.resolve(o)
@@ -269,9 +303,10 @@ fn tree_node_entries(d: &Dict) -> Vec<Object> {
/// reference keeps the annotation, its URL or text included, in a full rewrite after the page
/// no longer lists it. Returns how many references were dropped.
///
/// Only elements that are objects of their own are edited: a structure element written in place
/// (a direct dictionary in a `/K` array or a tree) keeps its references, since changing it means
/// rewriting the object that holds it. Producers rarely write elements that way.
/// Only elements that are objects of their own are edited: changing a structure element written in
/// place (a direct dictionary in a `/K` array or a tree) means rewriting the object that holds it,
/// so when one of those refers to a removed annotation the whole tree goes instead (fail-closed).
/// Producers rarely write elements that way.
pub(crate) fn drop_object_refs(doc: &mut Document, gone: &HashSet<ObjRef>) -> Result<usize, pdfcraft_cos::CosError> {
drop_object_refs_within(doc, gone, MAX_STRUCT_REFS)
}
@@ -302,6 +337,22 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet<ObjRef>
while let Some(k) = todo.pop() {
match k {
Object::Array(a) => todo.extend(a),
// A structure element written in place: its kids, and if it tags a removed
// annotation, the whole tree (see above).
Object::Dict(d) if d.contains(b"S") => {
let kids: Vec<Object> = match d.get(b"K").map(|k| doc.resolve(k)) {
Some(o) => match &*o {
Object::Array(a) => a.clone(),
_ => d.get(b"K").cloned().into_iter().collect(),
},
None => Vec::new(),
};
if kids.iter().any(|x| refers_to(doc, x, gone)) {
drop_tree(doc)?;
return Ok(0);
}
todo.extend(kids);
}
// A number or name tree node written in place (§7.9.6, §7.9.7): its values and kids.
Object::Dict(d) => todo.extend(tree_node_entries(&d)),
Object::Ref(r) => {
@@ -309,11 +360,7 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet<ObjRef>
continue;
}
if seen.len() > limit {
let Some(catalog) = doc.root() else { return Ok(0) };
doc.update_dict(catalog, |c| {
c.remove(b"StructTreeRoot");
c.remove(b"MarkInfo");
})?;
drop_tree(doc)?;
return Ok(0);
}
let object = doc.get(r);
+75
View File
@@ -930,6 +930,81 @@ fn elements_listed_only_in_the_parent_tree_or_the_id_tree_lose_their_references(
}
}
/// #3285 review: an element without its own /Pg takes its ancestor's page, also when the parent
/// tree lists it. One under /K must not be reached first through the parent tree without that
/// page, and one only the parent tree lists finds it through /P.
#[test]
fn an_element_inheriting_its_page_loses_its_redacted_text_when_the_parent_tree_lists_it() {
let objs: Vec<Vec<u8>> = vec![
b"<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 5 0 R /MarkInfo << /Marked true >> >>".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 /StructParents 0 /Resources << /Font << /F1 9 0 R >> >> >>".to_vec(),
stream("", b"/P <</MCID 0>> BDC BT /F1 14 Tf 20 150 Td (Codename Bluefin) Tj ET EMC /P <</MCID 1>> BDC BT /F1 14 Tf 20 100 Td (Codename Bluefin) Tj ET EMC"),
// The document element 6 carries the page; 7 sits under it, 8 only points at it.
b"<< /Type /StructTreeRoot /K 6 0 R /ParentTree << /Nums [0 [7 0 R 8 0 R]] >> /ParentTreeNextKey 1 >>".to_vec(),
b"<< /Type /StructElem /S /Document /P 5 0 R /Pg 3 0 R /K [7 0 R] >>".to_vec(),
b"<< /Type /StructElem /S /P /P 6 0 R /K 0 /ActualText (Codename Bluefin) >>".to_vec(),
b"<< /Type /StructElem /S /P /P 6 0 R /K 1 /Alt (Codename Bluefin) >>".to_vec(),
b"<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>".to_vec(),
];
let mut doc = pdf(objs);
mark(&mut doc, 0, &[[10.0, 90.0, 290.0, 170.0]], "");
let r = apply(&mut doc, None).unwrap();
assert!(r.glyphs > 0, "{r:?}");
for n in [7, 8] {
let element = doc.get(ObjRef::new(n, 0)).as_dict().cloned().unwrap();
assert!(!element.contains(b"ActualText") && !element.contains(b"Alt"), "{n}: {element:?}");
}
}
/// #3285 review: when the tag walk is cut short while redaction replaced a form's stream, the
/// marked-content references it never reached still name the original stream through /Stm, so
/// the tree goes, as it does for removed annotations. Without a replaced stream it stays.
#[test]
fn a_tag_walk_cut_short_over_a_replaced_stream_removes_the_tree() {
let objs: Vec<Vec<u8>> = vec![
b"<< /Type /Catalog /Pages 2 0 R /StructTreeRoot 5 0 R /MarkInfo << /Marked true >> >>".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] /StructParents 0 >>".to_vec(),
stream("/Type /XObject /Subtype /Form /BBox [0 0 10 10]", b"/P <</MCID 0>> BDC EMC"),
b"<< /Type /StructTreeRoot /K 6 0 R >>".to_vec(),
b"<< /Type /StructElem /S /Document /P 5 0 R /K [7 0 R] >>".to_vec(),
b"<< /Type /StructElem /S /P /P 6 0 R /Pg 3 0 R /K << /Type /MCR /MCID 0 /Stm 4 0 R >> >>".to_vec(),
stream("/Type /XObject /Subtype /Form /BBox [0 0 10 10]", b""),
];
let page = ObjRef::new(3, 0);
let streams = Some((ObjRef::new(4, 0), ObjRef::new(8, 0)));
let has_tree = |doc: &Document| doc.get(doc.root().unwrap()).as_dict().is_some_and(|c| c.contains(b"StructTreeRoot") && c.contains(b"MarkInfo"));
let mut doc = pdf(objs.clone());
crate::tags::clean_within(&mut doc, page, None, &[0].into(), &Default::default(), 1).unwrap();
assert!(has_tree(&doc), "without a replaced stream the tree stays");
let mut doc = pdf(objs);
crate::tags::clean_within(&mut doc, page, streams, &Default::default(), &Default::default(), 1).unwrap();
assert!(!has_tree(&doc), "a cut-short walk over a replaced stream drops the tree");
}
/// #3285 review: a structure element written in place, here as an ID tree value, can't be
/// edited without rewriting what holds it; when it tags a removed annotation the tree goes
/// instead of keeping the annotation reachable in a full rewrite.
#[test]
fn an_element_written_in_place_that_tags_a_removed_annotation_removes_the_tree() {
let objs: Vec<Vec<u8>> = vec![
b"<< /Type /Catalog /Pages 2 0 R /MarkInfo << /Marked true >> /StructTreeRoot 5 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] >>".to_vec(),
b"<< /Type /Annot /Subtype /Text /Rect [0 0 10 10] /Contents (hushhush note) >>".to_vec(),
b"<< /Type /StructTreeRoot /K 6 0 R /IDTree << /Names [(note1) << /Type /StructElem /S /Annot /P 6 0 R /K [<< /Type /OBJR /Obj 4 0 R >>] >>] >> >>".to_vec(),
b"<< /Type /StructElem /S /Document /P 5 0 R >>".to_vec(),
];
let gone: std::collections::HashSet<ObjRef> = [ObjRef::new(4, 0)].into();
let mut doc = pdf(objs);
crate::tags::drop_object_refs(&mut doc, &gone).unwrap();
let catalog = doc.get(doc.root().unwrap()).as_dict().cloned().unwrap();
assert!(!catalog.contains(b"StructTreeRoot") && !catalog.contains(b"MarkInfo"), "{catalog:?}");
let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap();
assert!(!bytes.windows(8).any(|w| w == b"hushhush"), "the removed comment can't be reached any more");
}
/// Issue #1325: removing comments keeps the annotations that are not comments: the multimedia
/// and print-production ones (Watermark, PrinterMark, TrapNet, Screen, Movie, RichMedia, 3D).
#[test]