From eed2afa16558bb753bc1cf0fe88d27817394f899 Mon Sep 17 00:00:00 2001
From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com>
Date: Fri, 23 Oct 2026 18:00:42 +0800
Subject: [PATCH] fix(forms): an inherited signature value and a direct
 /AcroForm are handled when filling (#3441)

Review follow-ups on filling a field converting inline /DR fonts:
- A signature field whose /V sits on its parent counted as unsigned, so
  filling another field rewrote /DR in a signed form. The check now
  follows the /Parent chain (bounded at 64, like the field walk).
- A catalog holding /AcroForm as a direct dictionary was skipped; its
  inline fonts are now converted and written back into the catalog.

Both tests failed before the change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0175FWj4xFhMhSH3UpMU8DSC
---
 crates/forms/src/author.rs | 48 +++++++++++++++++++++++++++++++-------
 crates/forms/src/tests.rs  | 40 +++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 8 deletions(-)

diff --git a/crates/forms/src/author.rs b/crates/forms/src/author.rs
index 4b421c31f..8b2e53c3f 100644
--- a/crates/forms/src/author.rs
+++ b/crates/forms/src/author.rs
@@ -197,22 +197,54 @@ fn store_form_fonts(doc: &mut Document, af: &mut Dict, mut dr: Dict, fonts: Dict
 /// Not in a form a signature covers: there the file stays as the signer saw it, apart from the
 /// value itself.
 pub(crate) fn form_fonts_as_objects(doc: &mut Document, all: &[crate::Field]) -> Result<(), FormError> {
-    let signed = all.iter().any(|f| f.kind == crate::FieldKind::Signature && doc.get(f.obj).as_dict().is_some_and(|d| d.contains(b"V")));
+    let signed = all.iter().any(|f| f.kind == crate::FieldKind::Signature && holds_value(doc, f.obj));
     if signed {
         return Ok(());
     }
     let Some(root) = doc.root() else { return Ok(()) };
-    // A form dictionary that isn't an object of its own is left as it is.
-    let Some(r) = doc.get(root).as_dict().and_then(|c| c.reference(b"AcroForm")) else { return Ok(()) };
-    let Some(mut af) = doc.get(r).as_dict().cloned() else { return Ok(()) };
-    let (dr, mut fonts) = form_fonts(doc, &af);
-    if inline_fonts_as_objects(doc, &mut fonts) {
-        store_form_fonts(doc, &mut af, dr, fonts);
-        doc.set(r, Object::Dict(af));
+    let Some(entry) = doc.get(root).as_dict().and_then(|c| c.get(b"AcroForm").cloned()) else { return Ok(()) };
+    match entry {
+        Object::Ref(r) => {
+            let Some(mut af) = doc.get(r).as_dict().cloned() else { return Ok(()) };
+            let (dr, mut fonts) = form_fonts(doc, &af);
+            if inline_fonts_as_objects(doc, &mut fonts) {
+                store_form_fonts(doc, &mut af, dr, fonts);
+                doc.set(r, Object::Dict(af));
+            }
+        }
+        // A form dictionary written directly in the catalog (the catalog's /AcroForm need not be
+        // an indirect reference) is written back there.
+        Object::Dict(mut af) => {
+            let (dr, mut fonts) = form_fonts(doc, &af);
+            if inline_fonts_as_objects(doc, &mut fonts) {
+                store_form_fonts(doc, &mut af, dr, fonts);
+                doc.update_dict(root, |c| c.set(b"AcroForm".to_vec(), Object::Dict(af)))?;
+            }
+        }
+        _ => {}
     }
     Ok(())
 }
 
+/// Whether the field `r`, or the nearest ancestor that has one, holds a `/V` (the entry is
+/// inheritable, ISO 32000-2 §12.7.4). The `/Parent` chain is followed at most 64 deep, as deep
+/// as the field walk goes, so a cyclic chain ends.
+fn holds_value(doc: &Document, r: ObjRef) -> bool {
+    let mut at = r;
+    for _ in 0..=64 {
+        let obj = doc.get(at);
+        let Some(d) = obj.as_dict() else { return false };
+        if d.contains(b"V") {
+            return true;
+        }
+        match d.reference(b"Parent") {
+            Some(p) => at = p,
+            None => return false,
+        }
+    }
+    false
+}
+
 /// The Appearance tab: borders, colours and text.
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub struct Look {
diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs
index 6773b5b0b..ea124be3a 100644
--- a/crates/forms/src/tests.rs
+++ b/crates/forms/src/tests.rs
@@ -3341,3 +3341,43 @@ fn filling_a_field_makes_inline_form_fonts_objects_unless_the_form_is_signed() {
     set_value(&mut doc, "name", &FieldValue::Text("Ada".into())).unwrap();
     assert!(!doc.modified_objects().contains(&8), "/DR unchanged: {:?}", doc.modified_objects());
 }
+
+/// #3441 (review follow-up): a signature value held by a parent field signs its kid too (`/V` is
+/// inheritable, ISO 32000-2 Table 226), so the form still counts as signed and keeps its `/DR`.
+#[test]
+fn a_signature_value_on_a_parent_field_keeps_the_form_fonts_as_signed() {
+    let mut doc = document(&[
+        "<< /Type /Catalog /Pages 2 0 R /AcroForm 4 0 R >>".into(),
+        "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),
+        "<< /Type /Page /Parent 2 0 R /Annots [5 0 R 8 0 R] >>".into(),
+        "<< /Fields [5 0 R 6 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> >> >> >>".into(),
+        "<< /Type /Annot /Subtype /Widget /FT /Tx /T (name) /DA (/Helv 12 Tf 0 g) /Rect [50 700 300 720] /P 3 0 R >>".into(),
+        "<< /FT /Sig /T (sig) /V 7 0 R /Kids [8 0 R] >>".into(),
+        "<< /Type /Sig /Filter /Adobe.PPKLite /Contents <00> >>".into(),
+        "<< /Type /Annot /Subtype /Widget /T (s1) /Parent 6 0 R /Rect [0 0 0 0] /P 3 0 R >>".into(),
+    ]);
+    let kid = pdfcraft_cos::ObjRef::new(8, 0);
+    assert!(fields(&doc).iter().any(|f| f.obj == kid && f.kind == FieldKind::Signature), "the kid is the signature field");
+    set_value(&mut doc, "name", &FieldValue::Text("Ada".into())).unwrap();
+    let doc = reopen(&doc);
+    assert!(dr_fonts(&doc).get(b"Helv").is_some_and(|h| h.as_dict().is_some()), "an inherited signature value keeps /DR as it was");
+    assert_eq!(field(&fields(&doc), "name").value, ["Ada"]);
+}
+
+/// #3441 (review follow-up): a catalog that holds `/AcroForm` as a direct dictionary gets its
+/// inline `/DR` fonts converted too when a field is filled.
+#[test]
+fn filling_a_field_converts_inline_fonts_of_a_direct_form_dictionary() {
+    let mut doc = document(&[
+        "<< /Type /Catalog /Pages 2 0 R /AcroForm << /Fields [4 0 R] /DA (/Helv 0 Tf 0 g) /DR << /Font << /Helv << /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >> >> >> >> >>".into(),
+        "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 600 800] >>".into(),
+        "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(),
+        "<< /Type /Annot /Subtype /Widget /FT /Tx /T (name) /DA (/Helv 12 Tf 0 g) /Rect [50 700 300 720] /P 3 0 R >>".into(),
+    ]);
+    set_value(&mut doc, "name", &FieldValue::Text("Ada".into())).unwrap();
+    let doc = reopen(&doc);
+    let fonts = dr_fonts(&doc);
+    assert!(fonts.reference(b"Helv").is_some(), "Helv became an object: {fonts:?}");
+    assert_eq!(fonts.len(), 1, "nothing added: {fonts:?}");
+    assert_eq!(field(&fields(&doc), "name").value, ["Ada"]);
+}