fix(stamp): font-path hardening — unresolvable /Resources references and direct-dict /Font entries - #11
Merged
alexjcoles merged 5 commits intoJul 9, 2026
Conversation
Pre-existing in the old pdf-stamp binary and carried through the port: an unresolvable /Resources reference broke out of the /Parent walk as Missing, so a dangling leaf reference was silently replaced with a fresh direct dict and a corrupt intermediate node stopped inheritance short. The nearest /Resources is Phase 2's write target, so the walk now fails loudly there — uniform with the /Font (AMPHTT-1232) and /Annots (AMPHTT-1157) reference contracts; callers discard the document on error. AMPHTT-1234 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
find_base_font_in only matched entries stored as Object::Reference, so a font embedded as a direct dictionary (legal per ISO 32000 §7.8.3) was never found and ensure_font registered a duplicate F_PS_* object on every stamp pass. Bounded and rendering-neutral, but needless — extract the dictionary from either entry shape. Unresolvable entry references stay lookup-misses: this function is pure lookup and never becomes a write target, so the AMPHTT-1232 lookup/registration split is unchanged. AMPHTT-1234 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etry add_font_to_resources silently replaces a non-dict, non-reference direct /Font value — the crate's established direct-repair / indirect-error convention (annotation.rs states it for /Annots), but undocumented and untested here. Comment the arm and pin it with a test; behaviour is unchanged. AMPHTT-1234 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s tail Code-review follow-up on the AMPHTT-1234 branch: the new Phase-1 chain was the second hand-rolled immutable copy of the resolve-id-to-dict-or- error idiom (the other in the Inherited branch), and the two Phase-1 match arms ended in a token-identical lookup/location/break tail. Name the immutable sibling of require_dict_mut and run the shared tail once; error strings and behaviour are unchanged. AMPHTT-1234 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Code-review follow-up on the AMPHTT-1234 branch. The branch had doubled (2 to 4, plus a split-syntax fifth) the copies of the page -> /Resources -> /Font match-panic pyramid; a #[track_caller] page_font_dict helper collapses them in the module's parent_pages_id/set_page_font style. Also closes two pin gaps the review sweep found: the non-dict leaf /Resources shape now re-checks the page was left untouched, and all three walk-Err tests assert the object count is unchanged, pinning the "Err from the /Parent walk adds no orphan objects" half of the ensure_font contract. AMPHTT-1234 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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
Fixes two pre-existing gaps in the
stamp.rsfont path, surfaced (not introduced) by the AMPHTT-1232 review — AMPHTT-1234:ensure_fonterrors on unresolvable/Resourcesreferences — previously a dangling reference silently fell through to theMissingbranch (replacing the page's corrupt entry with a fresh dict) or stopped the inheritance walk short. Now a/Resourcesreference that doesn't resolve to a dictionary is a hardErron the page and on any/Parentancestor consulted during the walk — uniform with the/Font(AMPHTT-1232) and/Annots(AMPHTT-1157) contracts. Callers discard the document on error.find_base_font_inmatches direct-dictionary/Fontentries (legal per ISO 32000 §7.8.3) — previously onlyReference-shaped entries were found, so an existing direct-dict font was duplicated as a freshF_PS_*object on every stamp pass./Fontrepair inadd_font_to_resources(the crate's direct-repair/indirect-error asymmetry, mirroringannotation.rs). Behaviour unchanged.Follow-up commits from an xhigh
/code-reviewon this branch: extractedrequire_dict(immutable sibling ofrequire_dict_mut) and the shared Phase-1 tail; added a#[track_caller]page_font_dicttest helper and tightened the walk-Err pins (page untouched + no orphan objects).One review finding was deliberately not fixed here:
/Fontresource keys flow unescaped/UTF-8-lossy intoTfoperators — pre-existing on main, filed as AMPHTT-1235.Plan (decision record incl. the uniform-Err vs walk-past-corrupt-ancestor argument):
docs/plans/pdf-native-library/amphtt-1234-stamprs-font-path-hardening.mdin the Rails repo.Test plan
cargo test --workspace— 99 tests green after every commit/Resourcesreference →Err(two shapes, page untouched, no orphan objects); unresolvable intermediate/Resourceswith a valid grandparent →Err(pins the uniform contract); direct-dict/Fontentry found with no duplicate registration; malformed direct/Fontvalue repairedAMPHTT-1234
🤖 Generated with Claude Code