fix(redact): bound structure-tree walks by their work, not only their references

The tag cleanup walks recorded a parent relation and queued an element's
kids for every listing, before checking whether the reference had been
seen. An indirect /K array shared by many elements was expanded again for
each, so a few thousand objects could queue and record millions of
relations; only distinct references counted against the limit.

Parent relations are now a set per element, so repeats are held once,
and every queued item is charged to a step allowance (and the queue to a
length cap) derived from the walk's reference limit. Past either the
cleanup can't be shown complete and is refused (RedactError::Tags), as
a tree past the reference limit already is.
This commit is contained in:
Dylan M. Taylor
2026-11-03 15:30:50 +00:00
parent 14ae839a28
commit 19bb96b032
2 changed files with 49 additions and 3 deletions
+25 -3
View File
@@ -193,7 +193,11 @@ pub(crate) fn clean_within(
let touches = |doc: &Document, x: &Object, pg: Option<ObjRef>| {
mcid_of(doc, x, pg, page, stream).is_some_and(|m| stream.is_some() || changed.contains(&m) || empty.contains(&m))
};
let mut steps = 0usize;
while let Some((k, pg, above)) = todo.pop() {
if over_budget(&mut steps, todo.len(), limit) {
return Err(RedactError::Tags);
}
match k {
Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, pg, above))),
// A number or name tree node written in place.
@@ -304,7 +308,7 @@ pub(crate) fn clean_within(
/// Where the walk reached an object from: the nearest element above it that is an object of its
/// own, and whether an element written in place between them carries a description (which only
/// rewriting the object holding it could remove).
#[derive(Clone, Copy, Default)]
#[derive(Clone, Copy, Default, PartialEq, Eq, Hash)]
struct Above {
element: Option<ObjRef>,
described_in_place: bool,
@@ -326,8 +330,22 @@ impl Above {
/// The title (`/T`) is included, as producers often fill it with the element's text.
const DESCRIPTIONS: [&[u8]; 4] = [b"Alt", b"ActualText", b"T", b"E"];
/// Charges one step of a structure-tree walk visiting at most `limit` references. Kids are queued
/// for every element that lists them, and an indirect `/K` array shared by many elements is
/// listed again for each, so the steps and the queue are bounded too, not only the distinct
/// references: past either the cleanup can't be shown complete, and is refused.
fn over_budget(steps: &mut usize, pending: usize, limit: usize) -> bool {
*steps += 1;
*steps > limit.saturating_mul(STEPS_PER_REFERENCE) || pending > limit.saturating_mul(STEPS_PER_REFERENCE)
}
/// Work items a walk may spend per reference it may visit: elements, their marked-content kids and
/// the tree nodes between them, with room to spare for real trees.
const STEPS_PER_REFERENCE: usize = 8;
/// Each element's parents as the walk found them: through `/K`, which `/P` may not agree with.
struct Parents(HashMap<ObjRef, Vec<Above>>);
/// A parent listing the element many times (a shared `/K` array of repeats) is held once.
struct Parents(HashMap<ObjRef, HashSet<Above>>);
impl Parents {
fn new() -> Self {
@@ -336,7 +354,7 @@ impl Parents {
fn record(&mut self, r: ObjRef, above: Above) {
if above.element.is_some() || above.described_in_place {
self.0.entry(r).or_default().push(above);
self.0.entry(r).or_default().insert(above);
}
}
}
@@ -474,7 +492,11 @@ pub(crate) fn drop_object_refs_within(doc: &mut Document, gone: &HashSet<ObjRef>
let mut seen = HashSet::new();
let mut parents = Parents::new();
let mut hits: Vec<ObjRef> = Vec::new();
let mut steps = 0usize;
while let Some((k, above)) = todo.pop() {
if over_budget(&mut steps, todo.len(), limit) {
return Err(RedactError::Tags);
}
match k {
Object::Array(a) => todo.extend(a.into_iter().map(|x| (x, above))),
// A structure element written in place: its kids are followed; one that tags a
+24
View File
@@ -3897,3 +3897,27 @@ fn a_redacted_form_xobject_drops_the_source_text_of_added_content() {
let bytes = write_full(&doc, &SaveOptions { object_streams: false, ..SaveOptions::default() }).unwrap();
assert!(!bytes.windows(4).any(|w| w == b"1234"), "redacted text left in the saved file");
}
/// 2 000 elements share one indirect `/K` array that lists one leaf element 2 000 times: the walk
/// queued and recorded every listing, four million, for a few thousand objects. Its work is now
/// bounded by the references it may visit, and past that the cleanup is refused.
#[test]
fn a_shared_kids_array_of_repeats_is_refused_not_expanded() {
const N: usize = 2_000;
let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (SECRET) Tj ET", "", vec![]);
let leaf = doc.add(Object::Dict(Dict::from_iter([(b"S".to_vec(), Object::name("Span"))])));
let shared = doc.add(Object::Array(vec![Object::Ref(leaf); N]));
let elements: Vec<Object> = (0..N)
.map(|_| Object::Ref(doc.add(Object::Dict(Dict::from_iter([(b"S".to_vec(), Object::name("P")), (b"K".to_vec(), Object::Ref(shared))])))))
.collect();
let root = doc.add(Object::Dict(Dict::from_iter([(b"Type".to_vec(), Object::name("StructTreeRoot")), (b"K".to_vec(), Object::Array(elements))])));
let catalog = doc.root().unwrap();
doc.update_dict(catalog, |d| d.set(b"StructTreeRoot".to_vec(), Object::Ref(root))).unwrap();
let gone: HashSet<ObjRef> = [ObjRef::new(999_999, 0)].into_iter().collect();
let started = std::time::Instant::now();
assert!(matches!(crate::tags::drop_object_refs_within(&mut doc, &gone, 10_000), Err(RedactError::Tags)));
let page = pdfcraft_model::pages(&doc)[0].obj;
let changed: HashSet<i64> = [0].into_iter().collect();
assert!(matches!(crate::tags::clean_within(&mut doc, page, None, &changed, &Default::default(), 10_000), Err(RedactError::Tags)));
assert!(started.elapsed() < std::time::Duration::from_secs(10), "{:?}", started.elapsed());
}