Skip to content

forms, initials: walk the field and page trees with the shared walker - #172

Merged
vanbroup merged 3 commits into
mainfrom
agent/forms-field-walk
Sep 23, 2026
Merged

vanbroup merged 3 commits into
mainfrom
agent/forms-field-walk

Conversation

@vanbroup

@vanbroup vanbroup commented Sep 23, 2026 •

Copy link
Copy Markdown
Member

Summary

Follow-up to #169/#170, which moved signing and verification onto the shared AcroForm walker in internal/acroform. Two places outside that path still walked /Kids with loops of their own, without a visited set, a depth bound, or /FT inheritance:

  • forms.Extract / forms.MapFields / Document.applyPendingFields walked the field tree by recursion. A field whose /FT sits on its parent (inheritable, ISO 32000-1 Table 220) was left out, and a crafted /Kids cycle recursed without end.
  • initials placement (findPage) recursed into the page tree /Kids the same way.

Changes

  • internal/acroform gains a general Fields(root, fn) and FieldsOf(field, prefix, fn), reporting every field (a parent before its kids) as a Field{Dict, Name, Type, Value, Terminal} with the inherited /FT and /V resolved and the fully qualified name built from /T decoded as a text string. SignatureFields is built on the same walk with the one signature-specific rule kept: a signature field that carries its own /V is the signed field and is not walked into.
  • Per ISO 32000-1 12.7.3.1 a terminal field is one without field kids, and a parent's /V is the default of its kids (Table 220). forms.Extract now reports terminal fields with their inherited type and value; MapFields maps every typed field as before, and the document's pending field updates are resolved with the same walk.
  • A field's name is its partial names joined with periods, so an untitled intermediate node no longer adds an empty component.
  • The visited set is keyed on the object pointer and the effective type: a conforming field has one parent, but a crafted tree can hold the same object under parents passing down different types. Keyed on the pointer alone, the first path decided, and a signed field reached first under a text field was skipped under its signature parent. It is now walked once per type, while a cycle still ends.
  • findPage carries a visited set and a depth bound, seeds the root node as visited, and returns an error when the page tree holds no such page instead of a null value the caller would write an annotation against.
  • Test fixtures share one PDF builder in internal/testpdf.

Related

This PR is independent of #171 and #173, which fix the verifying path; it can merge in any order with them.

Reviews

Code review and security review were run on the branch; the code review findings (generalized "/V stops descent" rule breaking forms; missing-page error) are fixed, and the security review raised no findings beyond the shared-node dedupe note addressed above.

Test plan

  • go vet ./... && go test ./... (12 packages green)
  • internal/acroform: inherited /FT and /V, terminal flag, untitled intermediate node, UTF-16 /T, self-reference and cross-parent cycles, shared node under parents of different types
  • forms: nested fields named in full with inherited type, parent /V as default, /Kids cycle terminates, MapFields maps typed parents and terminals
  • page tree: cycle terminates, missing page reported as an error
  • extract and verify field tree tests moved onto the shared fixture builder

forms.Extract and MapFields walked the AcroForm field tree with loops of
their own, without a visited set or depth bound, and without inheriting
/FT (ISO 32000-1 Table 220): a field whose type sits on its parent was
left out, and a crafted /Kids cycle recursed without end. Both now use
the walk in internal/acroform, which gains a general Fields for terminal
fields of every type with the inherited type and the fully qualified
name resolved, and FieldsOf for one subtree; SignatureFields is built on
it. A field's name is its partial names joined with periods, so an
untitled intermediate node no longer adds an empty component, and only
terminal fields are reported, as their values live there.

The page tree walk behind initials placement recursed into /Kids the
same way; it now carries a visited set and a depth bound.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fey5UYDEY84too4QCQ28oM
The walk stopped at any field carrying /V, a rule that fits the signed
field (its kids would only inherit that value) but not forms, where a
parent's /V is the default of its kids (ISO 32000-1 Table 220) and a
terminal field is one without field kids (12.7.3.1). The walk now
reports every field, a parent before its kids, with the /FT and /V it
carries or inherits and a terminal flag; SignatureFields alone keeps
the rule that a signature field with its own /V is the signed one and
is not walked into. Names come from /T decoded as a text string, so a
UTF-16 name reads as text.

forms.Extract reports the terminal fields with their inherited values;
MapFields maps every typed field as before, and the document's pending
field updates are resolved with the same walk instead of a loop of
their own. findPage returns an error when the page tree holds no such
page, rather than a null value the caller would write an annotation
against, and the root node is seeded as visited. The test fixtures
share one PDF builder in internal/testpdf.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fey5UYDEY84too4QCQ28oM
A field has one parent, but a crafted tree can hold the same object
under parents that pass down different types. Deduplicating by pointer
alone let the first path decide: a signed field reached first under a
text field inherited /Tx, was not a signature field there, and was then
skipped under the signature field it belonged to, which the walk before
this change did report. The visited set is keyed on the pointer and the
effective type, so a node is walked once per type while a cycle still
ends.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fey5UYDEY84too4QCQ28oM
@vanbroup
vanbroup merged commit 0c91171 into main Sep 23, 2026
6 checks passed
@vanbroup
vanbroup deleted the agent/forms-field-walk branch September 23, 2026 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants