From bea66e608170d50c6c1fd79fcc3ba29eec8c1b8b Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Fri, 30 Oct 2026 14:29:39 +0800 Subject: [PATCH] M12: XFAImages: the name tree is walked borrowing each node, not copying its Kids The walk queued every entry of a Kids array before checking the node limit, and resolving a direct node or array deep-copied it (two to four times for a direct tree). It now recurses (depth stays capped at 32), borrowing each node from its resolved owner, and stops at the limit as it goes: only the branch being walked is held. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/xfa/src/linked_images.rs | 101 ++++++++++++++++++++++---------- crates/xfa/src/tests.rs | 21 ++++++- 2 files changed, 88 insertions(+), 34 deletions(-) diff --git a/crates/xfa/src/linked_images.rs b/crates/xfa/src/linked_images.rs index d9c2e2983b..c60d465d52 100644 --- a/crates/xfa/src/linked_images.rs +++ b/crates/xfa/src/linked_images.rs @@ -26,50 +26,87 @@ const MAX_TOTAL: usize = 1 << 10; /// Deepest the template is walked. const MAX_TEMPLATE_DEPTH: usize = 256; +/// `f` of `o`, a reference followed. A direct object is lent as it is, not copied: a tree's +/// arrays can be as long as the file allows. +fn with_resolved(doc: &Document, o: &Object, f: impl FnOnce(&Object) -> R) -> R { + match o { + Object::Ref(r) => f(&doc.get(*r)), + other => f(other), + } +} + /// The `XFAImages` name tree: each name, as text, and the object it names. fn names(doc: &Document) -> HashMap { - let mut out = HashMap::new(); - let Some(root) = doc.root() else { return out }; + let mut walk = Walk { doc, out: HashMap::new(), seen: HashSet::new(), visited: 0 }; + let Some(root) = doc.root() else { return walk.out }; let catalog = doc.get(root); - let tree = - catalog.as_dict().and_then(|c| c.get(b"Names")).map(|n| doc.resolve(n)).and_then(|n| n.as_dict().and_then(|d| d.get(b"XFAImages")).cloned()); - let Some(tree) = tree else { return out }; - let mut seen: HashSet = HashSet::new(); - let mut visited = 0usize; - let mut stack = vec![(tree, 0usize)]; - while let Some((node, depth)) = stack.pop() { - if visited >= MAX_TREE_NODES || out.len() >= MAX_NAMES { - break; - } + if let Some(names) = catalog.as_dict().and_then(|c| c.get(b"Names")) { + with_resolved(doc, names, |n| { + if let Some(tree) = n.as_dict().and_then(|d| d.get(b"XFAImages")) { + walk.node(tree, 0); + } + }); + } + walk.out +} + +/// A walk of the name tree, borrowing each node from its resolved owner: nothing of the tree +/// is copied but the names kept, and only the branch being walked is held. +struct Walk<'a> { + doc: &'a Document, + out: HashMap, + seen: HashSet, + visited: usize, +} + +impl Walk<'_> { + fn full(&self) -> bool { + self.visited >= MAX_TREE_NODES || self.out.len() >= MAX_NAMES + } + + fn node(&mut self, node: &Object, depth: usize) { // Too deep: this branch is left, the others are still walked. - if depth > MAX_DEPTH { - continue; + if self.full() || depth > MAX_DEPTH { + return; } - visited += 1; + self.visited += 1; if let Some(r) = node.as_ref() - && !seen.insert(r) + && !self.seen.insert(r) { - continue; + return; } - let node = doc.resolve(&node); - let Some(d) = node.as_dict() else { continue }; - if let Some(pairs) = d.get(b"Names").map(|n| doc.resolve(n)).and_then(|n| n.as_array().cloned()) { - for pair in pairs.chunks(2) { - if let [key, value] = pair - && let Some(key) = doc.resolve(key).as_string() - { - out.entry(key.to_text()).or_insert_with(|| value.clone()); - if out.len() >= MAX_NAMES { - break; + let doc = self.doc; + with_resolved(doc, node, |node| { + let Some(d) = node.as_dict() else { return }; + if let Some(pairs) = d.get(b"Names") { + with_resolved(doc, pairs, |pairs| self.pairs(pairs)); + } + if let Some(kids) = d.get(b"Kids") { + with_resolved(doc, kids, |kids| { + for kid in kids.as_array().map(Vec::as_slice).unwrap_or_default() { + if self.full() { + break; + } + self.node(kid, depth + 1); } - } + }); + } + }); + } + + fn pairs(&mut self, pairs: &Object) { + let doc = self.doc; + for pair in pairs.as_array().map(Vec::as_slice).unwrap_or_default().chunks(2) { + if self.out.len() >= MAX_NAMES { + break; + } + if let [key, value] = pair + && let Some(key) = with_resolved(doc, key, |k| k.as_string().map(|s| s.to_text())) + { + self.out.entry(key).or_insert_with(|| value.clone()); } } - if let Some(kids) = d.get(b"Kids").map(|k| doc.resolve(k)).and_then(|k| k.as_array().cloned()) { - stack.extend(kids.into_iter().map(|k| (k, depth + 1))); - } } - out } /// Pictures found so far by their link, read from the file once each. diff --git a/crates/xfa/src/tests.rs b/crates/xfa/src/tests.rs index 8038c8d3fa..a8619b4a07 100644 --- a/crates/xfa/src/tests.rs +++ b/crates/xfa/src/tests.rs @@ -4211,8 +4211,8 @@ fn an_over_deep_xfa_images_branch_does_not_stop_the_walk() { for _ in 0..40 { deep = format!("<< /Kids [{deep}] >>"); } - // The deep branch is listed last, so it is walked first. - let root = format!("<< /Kids [7 0 R {deep}] >>"); + // The deep branch is listed first, so it is walked first. + let root = format!("<< /Kids [{deep} 7 0 R] >>"); let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap(); let tpl = crate::script::template_of(&doc).unwrap(); let logo = tpl.root.children.iter().find_map(|n| match n { @@ -4225,6 +4225,23 @@ fn an_over_deep_xfa_images_branch_does_not_stop_the_walk() { assert!(matches!(logo, Some(Value::Image { data, .. }) if !data.is_empty()), "the logo is found beside the deep branch"); } +/// A `Kids` array far longer than the nodes walked is walked in order, up to the limit, and not +/// held whole: the picture its first kid names is found, however many kids follow. +#[test] +fn a_wide_xfa_images_kids_array_is_walked_in_order_within_the_limit() { + let root = format!("<< /Kids [7 0 R {}] >>", "<< >> ".repeat(10_000)); + let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap(); + let tpl = crate::script::template_of(&doc).unwrap(); + let logo = tpl.root.children.iter().find_map(|n| match n { + Node::Subform(s) => s.children.iter().find_map(|c| match c { + Node::Draw(d) if d.common.name.as_deref() == Some("logo") => Some(d.value.clone()), + _ => None, + }), + _ => None, + }); + assert!(matches!(logo, Some(Value::Image { data, .. }) if !data.is_empty()), "the first kid's logo is found"); +} + /// A picture used in many places is held once per place, so every place counts against the /// pictures' total (small in tests): past it the rest are left out, with a warning. #[test]