diff --git a/crates/redact/src/tags.rs b/crates/redact/src/tags.rs index a0883ad6a..2b117fcf4 100644 --- a/crates/redact/src/tags.rs +++ b/crates/redact/src/tags.rs @@ -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)> = - [&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, 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 = 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 Option { + 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) -> bool { doc.resolve(o) @@ -269,9 +303,10 @@ fn tree_node_entries(d: &Dict) -> Vec { /// 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) -> Result { 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 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 = 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 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); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index f25ac197a..c721c5e6d 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -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![ + 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 <> BDC BT /F1 14 Tf 20 150 Td (Codename Bluefin) Tj ET EMC /P <> 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![ + 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 <> 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![ + 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::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]