fix: retain the source root when selecting a $defs entry so self-root refs resolve - #47
Open
vishkaty wants to merge 1 commit into
Open
Conversation
… refs resolve
Observed: `validate --def selected_payment_instrument` against the shipped
shopping/types/payment_instrument.json crashes the process with a stack
overflow. The def is `allOf: [{"$ref": "#"}, ...]`, and select_def's
wrapper ($schema + $ref + a copy of $defs) drops the source file's $id
and root, so "#" re-binds to the wrapper root — whose only content is the
$ref back into the def, an unbounded resolution cycle.
Expected: selecting a def that references its file root validates normally,
with "#" meaning the source file's root, as 2020-12 resolves it for
embedded resources.
When the selected def's reachable subgraph (the def itself plus every
sibling def it reaches through #/$defs/... pointers) references the source
root — self-root "#", fragment pointers escaping $defs, or absolute refs
to the file's own $id — the wrapper now embeds the whole source file as an
$id'd $defs resource and roots the selection through that $id,
synthesizing a stable content-hashed URN when the file has no $id (guarded
against colliding with any $id the source already declares). Detection is
scoped to the selected def's reachable subgraph, so a def with no root
dependency of its own keeps the existing wrapper byte-for-byte — including
when an unrelated sibling def carries a self-root ref — and emitted
`resolve --def` output for such defs is unchanged.
Fixes Universal-Commerce-Protocol#45
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.
Fixes #45.
What
validate --def <name>(andresolve --def) stack-overflowed when the selected$defsentry reaches a reference to the source file root.select_defbuilt a wrapper that kept$schema, a$ref: "#/$defs/<name>"and the file's$defs, but dropped the file's$id, so a$ref: "#"inside the selected def resolved to the wrapper root — whose only content is the$refback into the def — and recursed unboundedly. On the shippedpayment_instrument.json,--def selected_payment_instrumentaborts.Fix
When (and only when) the selected def's reachable subgraph references the source root, the wrapper embeds the whole source file as an
$id'd resource and roots the selection via<root_id>#/$defs/<name>, so#, escaping fragment pointers, and absolute self-$idrefs all resolve to the real source root (the 2020-12 embedded-resource rule). Simply carrying$idonto the old wrapper does not work —#would still bind to the wrapper root and the cycle survives; the embedded resource is what gives#its intended target. When the file has no$id, a stable content-hashed URN is synthesized, guarded against colliding with any$idthe source already declares.Root-reference detection is scoped to the selected def's reachable subgraph (a visited-set walk following
#/$defs/Xedges from<name>), so a def with no root dependency of its own keeps the existing plain wrapper byte-for-byte, even when an unrelated sibling def in the same file carries a self-root ref. Transitive dependencies are still detected (a selected def that reaches the root through a def it references embeds correctly).Scope (stated honestly)
This fixes the
$ref: "#"self-root case that #45 reports. It does not fix two pre-existing, separate--def/bundler behaviors (present onmainbefore this change, unchanged by it): a directly self-recursive#/$defs/<name>ref still overflows (the general bundler-recursion class, which overlaps #46), and a root-level$anchorref reportsfragment not found. Those are out of scope here.Verification
#45crash is gone on the real shipped schema: a complete selected payment instrument validatesValid; an incomplete one returns a clean"id" is a required propertyrejection; neither crashes. (Note: the exact payload in the issue omits the schema-requiredid, so post-fix that command correctly exits 1 with a validation error rather than 0 — the crash it demonstrated is fixed regardless of payload validity, since it occurred during schema construction.)$idsynthesis, the sibling-scope case, and a transitive-embed case), each kill-tested. Full suite 326 pass.resolve --defoutput for 8 unaffected defs is byte-identical tomain. Derivedresolve --op/validate --schemapaths are untouched.cargo fmt --check,cargo clippy --all-targets -- -D warnings,cargo build --release, and the pinned pre-commit are all clean.Happy to adjust the approach if you prefer one of the alternatives.