fix(xfa): tell fields apart by object number, and count every entry the walk looks at

crates/xfa/src/pdf.rs

@@ -161,17 +161,19 @@ fn existing_fields(doc: &Document, catalog: &Dict) -> (HashMap<String, Signed>,
 #[derive(Default)]
 struct FieldQueue {
     stack: Vec<(ObjRef, String, bool, usize)>,
-    seen: HashSet<ObjRef>,
+    /// Object numbers queued: [`Document::get`] finds a field by number alone, so `5 0 R` and
+    /// `5 1 R` are the same field.
+    seen: HashSet<u32>,
     read: usize,
 }
 
 impl FieldQueue {
     /// Queue the fields `kids` lists, each the first time it is listed. Fields and kids are
     /// indirect references (ISO 32000-2 §12.7.3.1); anything else is skipped. A field listed
-    /// again, by this list or another, takes no room, and at most [`MAX_FIELDS_SEARCHED`] are
-    /// ever queued: a field listing one kid many times, or fields sharing one long `/Kids`, would
-    /// otherwise queue it all on every visit. Reading the repeats costs too, so at most
-    /// [`MAX_KIDS_READ`] entries are read in all.
+    /// again, by this list or another, under any generation, takes no room, and at most
+    /// [`MAX_FIELDS_SEARCHED`] are ever queued: a field listing one kid many times, or fields
+    /// sharing one long `/Kids`, would otherwise queue it all on every visit. Reading the repeats
+    /// costs too, so at most [`MAX_KIDS_READ`] entries are read in all.
     fn queue(&mut self, kids: &[Object], prefix: &str, is_sig: bool, depth: usize) {
         for k in kids {
             self.read += 1;
@@ -179,7 +181,7 @@ impl FieldQueue {
                 return;
             }
             let Some(r) = k.as_ref() else { continue };
-            if self.seen.insert(r) {
+            if self.seen.insert(r.num) {
                 self.stack.push((r, prefix.to_string(), is_sig, depth));
             }
         }
@@ -989,17 +991,15 @@ pub fn pdf_rect(r: Rect, page_height: f64) -> [f64; 4] {
 
 #[cfg(test)]
 mod tests {
-    use pdfcraft_cos::{Dict, Document, Object, PdfString};
+    use std::collections::{HashMap, HashSet};
 
-    use super::{MAX_FIELDS_SEARCHED, existing_fields, parse_hex32};
+    use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString};
+
+    use super::{MAX_FIELDS_SEARCHED, Signed, existing_fields, parse_hex32};
 
     /// The fields `existing_fields` finds under an AcroForm listing `fields`.
     fn names_found(doc: &Document, fields: Vec<Object>) -> Vec<String> {
-        let mut acro = Dict::new();
-        acro.set(b"Fields".to_vec(), Object::Array(fields));
-        let mut catalog = Dict::new();
-        catalog.set(b"AcroForm".to_vec(), Object::Dict(acro));
-        let (signed, names) = existing_fields(doc, &catalog);
+        let (signed, names) = existing(doc, fields);
         assert!(signed.is_empty());
         let mut names: Vec<String> = names.into_iter().collect();
         names.sort();
@@ -1038,20 +1038,43 @@ mod tests {
         // The AcroForm lists one field 19,999 times. The copies take no room, so its kids are
         // still queued, and the signed one among them is found.
         let mut doc = Document::new_empty();
+        let parent = signed_parent(&mut doc);
+        let (signed, names) = existing(&doc, vec![Object::Ref(parent); MAX_FIELDS_SEARCHED - 1]);
+        assert!(signed.contains_key("c.sig"), "{:?}", signed.keys().collect::<Vec<_>>());
+        assert!(names.contains("c.plain"));
+    }
+
+    #[test]
+    fn a_field_listed_under_many_generations_is_queued_once() {
+        // `Document::get` finds a field by object number alone, so `5 0 R` and `5 1 R` are one
+        // field. Listed 19,999 times, each under another generation, it is still queued once,
+        // and the signed field under it is found.
+        let mut doc = Document::new_empty();
+        let parent = signed_parent(&mut doc);
+        let fields = (0..MAX_FIELDS_SEARCHED - 1).map(|g| Object::Ref(ObjRef::new(parent.num, u16::try_from(g).unwrap()))).collect();
+        let (signed, names) = existing(&doc, fields);
+        assert!(signed.contains_key("c.sig"), "{:?}", signed.keys().collect::<Vec<_>>());
+        assert!(names.contains("c.plain"));
+    }
+
+    /// Field `c` with two kids: `plain`, and `sig`, a signed signature field.
+    fn signed_parent(doc: &mut Document) -> ObjRef {
         let plain = doc.add(field("plain", Object::Array(Vec::new())));
         let mut sig = Dict::new();
         sig.set(b"T".to_vec(), Object::String(PdfString::literal("sig")));
         sig.set(b"FT".to_vec(), Object::Name(b"Sig".to_vec()));
         sig.set(b"V".to_vec(), Object::Dict(Dict::new()));
         let sig = doc.add(Object::Dict(sig));
-        let parent = doc.add(field("c", Object::Array(vec![Object::Ref(plain), Object::Ref(sig)])));
+        doc.add(field("c", Object::Array(vec![Object::Ref(plain), Object::Ref(sig)])))
+    }
+
+    /// What `existing_fields` finds under an AcroForm listing `fields`.
+    fn existing(doc: &Document, fields: Vec<Object>) -> (HashMap<String, Signed>, HashSet<String>) {
         let mut acro = Dict::new();
-        acro.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(parent); MAX_FIELDS_SEARCHED - 1]));
+        acro.set(b"Fields".to_vec(), Object::Array(fields));
         let mut catalog = Dict::new();
         catalog.set(b"AcroForm".to_vec(), Object::Dict(acro));
-        let (signed, names) = existing_fields(&doc, &catalog);
-        assert!(signed.contains_key("c.sig"), "{:?}", signed.keys().collect::<Vec<_>>());
-        assert!(names.contains("c.plain"));
+        existing_fields(doc, &catalog)
     }
 
     #[test]

crates/xfa/src/script.rs

@@ -15,7 +15,8 @@ pub const OVERRIDES_KEY: &[u8] = b"PCXfaOverrides";
 
 const MAX_DEPTH: usize = 64;
 const MAX_EVENTS: usize = 20_000;
-/// Most `/Kids` entries [`fields_by_som`] follows in all, fields already read included.
+/// Most `/Fields` and `/Kids` entries [`fields_by_som`] looks at in all: fields already read, and
+/// entries that are not references, included.
 const MAX_FIELD_STEPS: usize = 1 << 20;
 /// Most bytes of script the events hold together: each instance of a repeated subform holds its
 /// own copy, so a long script repeated many times stops here (as much as the XFA packets hold).
@@ -546,20 +547,27 @@ pub fn apply_overrides(tpl: &Template, ov: &Overrides) -> Template {
 /// The generated fields of the AcroForm by SOM path: `som → (field name, is a radio group)`.
 pub fn fields_by_som(doc: &Document) -> HashMap<String, String> {
     // Each field is read once: a field listed twice in `/Kids` (or among its own kids) would
-    // otherwise be walked again for every path to it, doubling the work at each level. `steps`
-    // counts every entry followed, so fields sharing one long `/Kids` array stay bounded too;
-    // `walk` returns false once it is spent, and every loop then stops.
+    // otherwise be walked again for every path to it, doubling the work at each level. Fields
+    // are told apart by object number, as `Document::get` finds them, so `5 0 R` and `5 1 R`
+    // are one. `steps` counts every entry looked at, references or not, so fields sharing one
+    // long `/Kids` array stay bounded too; `walk_all` returns false once it is spent, and every
+    // loop then stops.
     struct Walk {
         out: HashMap<String, String>,
-        seen: HashSet<pdfcraft_cos::ObjRef>,
+        seen: HashSet<u32>,
         steps: usize,
     }
+    fn walk_all(doc: &Document, kids: &[Object], prefix: &str, w: &mut Walk, depth: usize) -> bool {
+        kids.iter().all(|k| {
+            w.steps += 1;
+            if w.steps > MAX_FIELD_STEPS || w.out.len() > 100_000 {
+                return false;
+            }
+            k.as_ref().is_none_or(|r| walk(doc, r, prefix, w, depth))
+        })
+    }
     fn walk(doc: &Document, r: pdfcraft_cos::ObjRef, prefix: &str, w: &mut Walk, depth: usize) -> bool {
-        w.steps += 1;
-        if w.steps > MAX_FIELD_STEPS || w.out.len() > 100_000 {
-            return false;
-        }
-        if depth > MAX_DEPTH || !w.seen.insert(r) {
+        if depth > MAX_DEPTH || !w.seen.insert(r.num) {
             return true;
         }
         let o = doc.get(r);
@@ -577,12 +585,12 @@ pub fn fields_by_som(doc: &Document) -> HashMap<String, String> {
         }
         // Read in place: a copy of the array at every level would cost its length each time.
         let kids = d.get(b"Kids").map(|k| doc.resolve(k));
-        kids.as_deref().and_then(Object::as_array).into_iter().flatten().filter_map(Object::as_ref).all(|k| walk(doc, k, &name, w, depth + 1))
+        walk_all(doc, kids.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default(), &name, w, depth + 1)
     }
     let Some((_, acro)) = acroform(doc) else { return HashMap::new() };
     let fields = acro.get(b"Fields").map(|f| doc.resolve(f));
     let mut w = Walk { out: HashMap::new(), seen: HashSet::new(), steps: 0 };
-    let _ = fields.as_deref().and_then(Object::as_array).into_iter().flatten().filter_map(Object::as_ref).all(|f| walk(doc, f, "", &mut w, 0));
+    walk_all(doc, fields.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default(), "", &mut w, 0);
     w.out
 }
 

crates/xfa/src/tests.rs

@@ -4094,25 +4094,75 @@ fn clear_password_values_clears_static_xfa_password_fields() {
 fn fields_by_som_reads_a_field_listed_many_times_once() {
     // A field lists itself twice and its one widget twice: walked once per path to it, that is
     // 2^64 visits at the depth limit.
-    let s = |t: &str| Object::String(pdfcraft_cos::PdfString::literal(t));
     let mut doc = Document::new_empty();
-    let mut widget = Dict::new();
-    widget.set(b"T".to_vec(), s("name"));
-    widget.set(crate::pdf::SOM_KEY.to_vec(), s("form1[0].name[0]"));
-    let widget = Object::Ref(doc.add(Object::Dict(widget)));
+    let widget = Object::Ref(som_field(&mut doc, "name", Some("form1[0].name[0]"), None));
     let parent = doc.add(Object::Null);
+    let kids = Object::Array(vec![Object::Ref(parent), widget.clone(), Object::Ref(parent), widget]);
+    doc.set(parent, field_named("form1", None, Some(kids)));
+    let by_som = fields_by_som_of(doc, vec![Object::Ref(parent)]);
+    assert_eq!(by_som.len(), 1, "{by_som:?}");
+    assert_eq!(by_som.get("form1[0].name[0]").map(String::as_str), Some("form1.name"));
+}
+
+#[test]
+fn fields_by_som_reads_a_field_listed_under_many_generations_once() {
+    // `Document::get` finds a field by object number alone, so `5 0 R` and `5 1 R` are one field.
+    // The AcroForm lists one under 2,000 generations: read again for each, its 1,000 kids would
+    // spend the walk's 2^20 steps before the field listed after it.
+    let mut doc = Document::new_empty();
+    let leaf = som_field(&mut doc, "leaf", None, None);
+    let a = som_field(&mut doc, "a", None, Some(Object::Array(vec![Object::Ref(leaf); 1_000])));
+    let b = som_field(&mut doc, "b", Some("form1[0].b[0]"), None);
+    let mut fields: Vec<Object> = (0..2_000).map(|g| Object::Ref(pdfcraft_cos::ObjRef::new(a.num, g))).collect();
+    fields.push(Object::Ref(b));
+    let by_som = fields_by_som_of(doc, fields);
+    assert_eq!(by_som.get("form1[0].b[0]").map(String::as_str), Some("b"), "{by_som:?}");
+}
+
+#[test]
+fn fields_by_som_counts_kids_that_are_not_references() {
+    // 2,000 fields share one `/Kids` array of 1,000 nulls. Skipped uncounted, the nulls would
+    // cost nothing against the walk's 2^20 steps, however many fields share however long an
+    // array. Counted, they spend the steps here, and the walk stops before the last field.
+    let mut doc = Document::new_empty();
+    let nulls = doc.add(Object::Array(vec![Object::Null; 1_000]));
+    let first = som_field(&mut doc, "first", Some("form1[0].first[0]"), None);
+    let last = som_field(&mut doc, "last", Some("form1[0].last[0]"), None);
+    let mut fields = vec![Object::Ref(first)];
+    fields.extend((0..2_000).map(|i| Object::Ref(som_field(&mut doc, &format!("f{i}"), None, Some(Object::Ref(nulls))))));
+    fields.push(Object::Ref(last));
+    let by_som = fields_by_som_of(doc, fields);
+    assert!(by_som.contains_key("form1[0].first[0]"), "{by_som:?}");
+    assert!(!by_som.contains_key("form1[0].last[0]"), "{by_som:?}");
+}
+
+/// An AcroForm field named `t`, with SOM path `som` and `kids` when given.
+fn field_named(t: &str, som: Option<&str>, kids: Option<Object>) -> Object {
+    let s = |t: &str| Object::String(pdfcraft_cos::PdfString::literal(t));
     let mut d = Dict::new();
-    d.set(b"T".to_vec(), s("form1"));
-    d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(parent), widget.clone(), Object::Ref(parent), widget]));
-    doc.set(parent, Object::Dict(d));
+    d.set(b"T".to_vec(), s(t));
+    if let Some(som) = som {
+        d.set(crate::pdf::SOM_KEY.to_vec(), s(som));
+    }
+    if let Some(kids) = kids {
+        d.set(b"Kids".to_vec(), kids);
+    }
+    Object::Dict(d)
+}
+
+/// [`field_named`], added to `doc`.
+fn som_field(doc: &mut Document, t: &str, som: Option<&str>, kids: Option<Object>) -> pdfcraft_cos::ObjRef {
+    doc.add(field_named(t, som, kids))
+}
+
+/// [`fields_by_som`] of `doc`, its catalog's AcroForm listing `fields`.
+fn fields_by_som_of(mut doc: Document, fields: Vec<Object>) -> std::collections::HashMap<String, String> {
     let mut acro = Dict::new();
-    acro.set(b"Fields".to_vec(), Object::Array(vec![Object::Ref(parent)]));
+    acro.set(b"Fields".to_vec(), Object::Array(fields));
     let mut cat = Dict::new();
     cat.set(b"Type".to_vec(), Object::Name(b"Catalog".to_vec()));
     cat.set(b"AcroForm".to_vec(), Object::Dict(acro));
     let cat = doc.add(Object::Dict(cat));
     doc.trailer_mut().set(b"Root".to_vec(), Object::Ref(cat));
-    let by_som = fields_by_som(&doc);
-    assert_eq!(by_som.len(), 1, "{by_som:?}");
-    assert_eq!(by_som.get("form1[0].name[0]").map(String::as_str), Some("form1.name"));
+    fields_by_som(&doc)
 }