forms, initials: walk the field and page trees with the shared walker - #172
Merged
Merged
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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/Kidswith loops of their own, without a visited set, a depth bound, or/FTinheritance:/FTsits on its parent (inheritable, ISO 32000-1 Table 220) was left out, and a crafted/Kidscycle recursed without end.findPage) recursed into the page tree/Kidsthe same way.Changes
internal/acroformgains a generalFields(root, fn)andFieldsOf(field, prefix, fn), reporting every field (a parent before its kids) as aField{Dict, Name, Type, Value, Terminal}with the inherited/FTand/Vresolved and the fully qualified name built from/Tdecoded as a text string.SignatureFieldsis built on the same walk with the one signature-specific rule kept: a signature field that carries its own/Vis the signed field and is not walked into./Vis the default of its kids (Table 220).forms.Extractnow reports terminal fields with their inherited type and value;MapFieldsmaps every typed field as before, and the document's pending field updates are resolved with the same walk.findPagecarries 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.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/FTand/V, terminal flag, untitled intermediate node, UTF-16/T, self-reference and cross-parent cycles, shared node under parents of different typesforms: nested fields named in full with inherited type, parent/Vas default,/Kidscycle terminates,MapFieldsmaps typed parents and terminalsextractandverifyfield tree tests moved onto the shared fixture builder