M12: XFAImages: every name-and-value pair counts against the walk, and over-long names are left

A Names array repeating one name kept the walk going, since only new names
counted. Pairs are now counted, repeats included, up to 16,384, and a name
over 4,096 bytes is left before it is turned into text. The node limit no
longer skips the names of the node the walk has just reached.

Tests for a repeated name, an over-long name, the 4,096th node, a linked
stream that isn't a picture, and an over-large picture.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv

crates/xfa/src/fixtures.rs

@@ -750,6 +750,11 @@ pub fn linked_image_shell() -> Vec<u8> {
 /// [`linked_image_shell`] with its own name tree: `root` is object 6 (the tree the catalog names),
 /// `kid` object 7, and object 8 the PNG.
 pub fn linked_image_shell_with(root: &[u8], kid: &[u8]) -> Vec<u8> {
+    linked_image_shell_holding(root, kid, TINY_PNG)
+}
+
+/// [`linked_image_shell_with`], object 8 a stream holding `picture` (a [`tiny_png`] there).
+pub fn linked_image_shell_holding(root: &[u8], kid: &[u8], picture: &[u8]) -> Vec<u8> {
     let xdp = r##"<?xml version="1.0" encoding="UTF-8"?>
 <xdp:xdp xmlns:xdp="http://ns.adobe.com/xdp/">
 <template xmlns="http://www.xfa.org/schema/xfa-template/3.3/">
@@ -763,8 +768,8 @@ pub fn linked_image_shell_with(root: &[u8], kid: &[u8]) -> Vec<u8> {
 </template>
 </xdp:xdp>
 "##;
-    let mut png = format!("<< /Length {} >>\nstream\n", TINY_PNG.len()).into_bytes();
-    png.extend_from_slice(TINY_PNG);
+    let mut png = format!("<< /Length {} >>\nstream\n", picture.len()).into_bytes();
+    png.extend_from_slice(picture);
     png.extend_from_slice(b"\nendstream");
     let objs: Vec<Vec<u8>> = vec![
         b"<< /Type /Catalog /Pages 2 0 R /NeedsRendering true /AcroForm << /XFA 5 0 R /Fields [] >> /Names << /XFAImages 6 0 R >> >>".to_vec(),

crates/xfa/src/linked_images.rs

@@ -13,10 +13,20 @@ use crate::model::{Field, Node, Subform, Template, Value};
 /// Most nodes of the name tree visited, and most names kept from it.
 const MAX_TREE_NODES: usize = 4096;
 const MAX_NAMES: usize = 4096;
+/// Most name-and-value pairs looked at, a name repeated counted each time (only a new name is
+/// kept, so [`MAX_NAMES`] alone doesn't end a `Names` array that repeats one).
+const MAX_PAIRS: usize = 4 * MAX_NAMES;
+/// Longest name kept, in bytes: a template's link is a short path or URI, and a longer name
+/// could only be matched by an equally long link.
+const MAX_NAME_LEN: usize = 4096;
 /// Deepest the name tree is followed.
 const MAX_DEPTH: usize = 32;
-/// Largest picture taken from the file, as for a picture written into the template.
+/// Largest picture taken from the file, as for a picture written into the template. Small in
+/// tests, so a test can pass it.
+#[cfg(not(test))]
 const MAX_IMAGE: usize = 32 << 20;
+#[cfg(test)]
+const MAX_IMAGE: usize = 512;
 /// Most bytes of pictures taken from the file in all, each place a picture is used counted (a
 /// picture is held once per place). Small in tests, so a test can reach it.
 #[cfg(not(test))]
@@ -37,7 +47,7 @@ fn with_resolved<R>(doc: &Document, o: &Object, f: impl FnOnce(&Object) -> R) ->
 
 /// The `XFAImages` name tree: each name, as text, and the object it names.
 fn names(doc: &Document) -> HashMap<String, Object> {
-    let mut walk = Walk { doc, out: HashMap::new(), seen: HashSet::new(), visited: 0 };
+    let mut walk = Walk { doc, out: HashMap::new(), seen: HashSet::new(), visited: 0, pairs: 0 };
     let Some(root) = doc.root() else { return walk.out };
     let catalog = doc.get(root);
     if let Some(names) = catalog.as_dict().and_then(|c| c.get(b"Names")) {
@@ -57,11 +67,13 @@ struct Walk<'a> {
     out: HashMap<String, Object>,
     seen: HashSet<ObjRef>,
     visited: usize,
+    /// Pairs looked at so far, repeats included.
+    pairs: usize,
 }
 
 impl Walk<'_> {
     fn full(&self) -> bool {
-        self.visited >= MAX_TREE_NODES || self.out.len() >= MAX_NAMES
+        self.visited >= MAX_TREE_NODES || self.out.len() >= MAX_NAMES || self.pairs >= MAX_PAIRS
     }
 
     fn node(&mut self, node: &Object, depth: usize) {
@@ -97,11 +109,15 @@ impl Walk<'_> {
     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 {
+            // The names and pairs limits only: the node limit ends the walk at the next node, not
+            // the names of the node it has just come to.
+            if self.out.len() >= MAX_NAMES || self.pairs >= MAX_PAIRS {
                 break;
             }
+            self.pairs += 1;
+            // An over-long name is left before it is turned into text.
             if let [key, value] = pair
-                && let Some(key) = with_resolved(doc, key, |k| k.as_string().map(|s| s.to_text()))
+                && let Some(key) = with_resolved(doc, key, |k| k.as_string().filter(|s| s.bytes.len() <= MAX_NAME_LEN).map(|s| s.to_text()))
             {
                 self.out.entry(key).or_insert_with(|| value.clone());
             }

crates/xfa/src/tests.rs

@@ -4203,6 +4203,108 @@ fn a_hostile_xfa_images_tree_is_walked_once_and_gives_nothing() {
     }
 }
 
+/// The picture data a draw named `name` ends up with, in the template's first subform.
+fn linked_picture(tpl: &crate::model::Template, name: &str) -> Option<Vec<u8>> {
+    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(name) => match &d.value {
+                Value::Image { data, .. } => Some(data.clone()),
+                _ => None,
+            },
+            _ => None,
+        }),
+        _ => None,
+    })
+}
+
+/// #5393 review: a `Names` array that repeats one name counts every pair it holds, not only the
+/// names kept: the walk stops at its limit however often the name repeats, so the name listed
+/// after twenty thousand repeats is never reached.
+#[test]
+fn a_repeated_xfa_images_name_counts_against_the_walk() {
+    let kid = format!(r"<< /Names [{}(.\\logo.png) 8 0 R] >>", r"(.\\dup.png) 8 0 R ".repeat(20_000));
+    let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(b"<< /Kids [7 0 R] >>", kid.as_bytes()))).unwrap();
+    let mut tpl = crate::script::template_of(&doc).unwrap();
+    crate::linked_images::resolve(&mut tpl, &doc);
+    assert_eq!(linked_picture(&tpl, "logo"), Some(Vec::new()), "past the limit the logo's name isn't reached");
+}
+
+/// The last node the walk reaches, the 4,096th, still gives its names: the node limit stops
+/// the walk from going on, not the node it has just come to.
+#[test]
+fn the_last_xfa_images_node_the_walk_reaches_gives_its_names() {
+    // The root, 4,094 empty kids, and the kid holding the logo: 4,096 nodes in all.
+    let root = format!("<< /Kids [{}7 0 R] >>", "<< >> ".repeat(4_094));
+    let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(root.as_bytes(), br"<< /Names [(.\\logo.png) 8 0 R] >>"))).unwrap();
+    let mut tpl = crate::script::template_of(&doc).unwrap();
+    crate::linked_images::resolve(&mut tpl, &doc);
+    assert_eq!(linked_picture(&tpl, "logo"), Some(crate::fixtures::tiny_png()), "the 4,096th node's name is read");
+}
+
+/// #5393 review: a name longer than any link is left before it is turned into text; the names
+/// beside it are still read.
+#[test]
+fn an_over_long_xfa_images_name_is_left() {
+    let long = "L".repeat(5_000);
+    let kid = format!(r"<< /Names [({long}) 8 0 R (.\\logo.png) 8 0 R] >>");
+    let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_with(b"<< /Kids [7 0 R] >>", kid.as_bytes()))).unwrap();
+    let xml = format!(
+        r#"<template xmlns="http://www.xfa.org/schema/xfa-template/3.3/"><subform name="form" layout="tb">
+ <pageSet><pageArea name="p"><contentArea x="0.5in" y="0.5in" w="7.5in" h="10in"/><medium short="8.5in" long="11in"/></pageArea></pageSet>
+ <subform name="page1" layout="tb">
+  <draw name="long" w="1in" h="1in"><value><image contentType="image/png" href="{long}"/></value></draw>
+  <draw name="logo" w="1in" h="1in"><value><image contentType="image/png" href=".\logo.png"/></value></draw>
+ </subform></subform></template>"#
+    );
+    let (mut tpl, _) = crate::parse(&xml).unwrap();
+    crate::linked_images::resolve(&mut tpl, &doc);
+    assert_eq!(linked_picture(&tpl, "long"), Some(Vec::new()), "the over-long name isn't kept");
+    assert_eq!(linked_picture(&tpl, "logo"), Some(crate::fixtures::tiny_png()), "the name beside it is");
+}
+
+/// #5393 review: a name the tree holds whose stream isn't a picture is taken as it is, and
+/// drawing it draws nothing and warns, as a picture the template holds that isn't one does.
+#[test]
+fn a_linked_stream_that_is_not_a_picture_draws_nothing_and_warns() {
+    let mut doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_holding(
+        b"<< /Kids [7 0 R] >>",
+        br"<< /Names [(.\\logo.png) 8 0 R] >>",
+        b"this is not a picture",
+    )))
+    .unwrap();
+    // The stream is found and taken as the logo's data.
+    let mut tpl = crate::script::template_of(&doc).unwrap();
+    assert!(crate::linked_images::resolve(&mut tpl, &doc).is_empty());
+    assert_eq!(linked_picture(&tpl, "logo"), Some(b"this is not a picture".to_vec()));
+    let report = render_into(&mut doc).unwrap();
+    // One warning for the logo and the missing picture alike (warnings are said once).
+    assert!(report.warnings.iter().any(|w| w.contains("not a JPEG, PNG")), "{:?}", report.warnings);
+    let images = doc
+        .object_numbers()
+        .into_iter()
+        .filter(
+            |n| matches!(&*doc.get(pdfcraft_cos::ObjRef { num: *n, generation: 0 }), Object::Stream(s) if s.dict.name(b"Subtype") == Some(b"Image")),
+        )
+        .count();
+    assert_eq!(images, 0, "no picture drawn");
+}
+
+/// #5393 review: a picture larger than the limit (small in tests) is left out with a warning,
+/// not read whole.
+#[test]
+fn an_over_large_linked_picture_is_left_with_a_warning() {
+    let doc = Document::open(Arc::new(crate::fixtures::linked_image_shell_holding(
+        b"<< /Kids [7 0 R] >>",
+        br"<< /Names [(.\\logo.png) 8 0 R] >>",
+        &[7u8; 600],
+    )))
+    .unwrap();
+    let mut tpl = crate::script::template_of(&doc).unwrap();
+    let warnings = crate::linked_images::resolve(&mut tpl, &doc);
+    assert!(warnings.iter().any(|w| w.contains("could not be read")), "{warnings:?}");
+    assert_eq!(linked_picture(&tpl, "logo"), Some(Vec::new()));
+}
+
 /// An over-deep branch of the `XFAImages` tree is left on its own: the walk goes on to the
 /// branches beside it, and the picture one of them holds is found.
 #[test]