fix(orval): decode escaped JSON Pointer tokens in external $refs (#3380) - #3382
Merged
melloware merged 2 commits intoMay 18, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughDecodes percent-encoded URI fragment tokens and JSON Pointer escapes when resolving external ChangesExternal Path-Item Reference Decoding
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
packages/orval/src/import-specs.ts (1)
347-357: 💤 Low valueConsider adding JSDoc to document the decoding order.
The inline comment is helpful, but a JSDoc block would make the two-layer decoding contract (percent-encoding outer, JSON Pointer inner) more discoverable for future maintainers.
📝 Optional JSDoc addition
+/** + * Decode a single JSON Pointer reference token taken from an x-ext `$ref`. + * + * The token carries two layers of encoding: it sits in a URI fragment, so it + * may be percent-encoded (e.g. `%7B` for `{` in templated paths), and it is a + * JSON Pointer token, so `~1`/`~0` stand for `/`/`~` (RFC 6901). Percent- + * encoding is the outer layer and is removed first; a malformed sequence is + * left as-is rather than throwing. Without this, tokens such as `~1pets` + * never match the real `/pets` key and the external `$ref` fails to resolve. + * + * `@param` token - Raw reference token from a split `$ref` path + * `@returns` Decoded token ready for object property lookup + */ function decodeRefToken(token: string): string {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orval/src/import-specs.ts` around lines 347 - 357, Add a JSDoc block above the decodeRefToken function that documents the two-layer decoding contract: percent-decoding (URI fragment) is applied first and then JSON Pointer unescaping (~1 → /, ~0 → ~), note that malformed percent-encodings are left unchanged (no throw), and include an example showing a token like "%7B~1pets" -> "{/pets" to make the decoding order and behavior explicit for future maintainers; reference the function name decodeRefToken in the JSDoc.packages/orval/src/import-specs.test.ts (1)
764-801: 💤 Low valueConsider adding test coverage for
~0(tilde escape).The test validates
~1(slash) and percent-encoding (%7B/%7D), but doesn't explicitly test~0→~decoding. While the implementation is straightforward, adding a path like/pets~0dogs(for/pets~dogs) would complete the RFC 6901 coverage.🧪 Optional test case addition
Add a third path to the test input:
'/pets/{petId}': { $ref: '`#/x-ext/abc1234/paths/`~1pets~1%7BpetId%7D', }, + '/pets~dogs': { + $ref: '`#/x-ext/abc1234/paths/`~1pets~0dogs', + }, },And corresponding external definition plus assertion:
'/pets/{petId}': { get: { operationId: 'getPet', parameters: [ { name: 'petId', in: 'path', required: true, schema: { type: 'string' }, }, ], responses: { '200': { description: 'ok' } }, }, }, + '/pets~dogs': { + get: { + operationId: 'listPetsDogs', + responses: { '200': { description: 'ok' } }, + }, + }, }, }, },expect(result.paths?.['/pets/{petId}']).toEqual({ get: { operationId: 'getPet', parameters: [ { name: 'petId', in: 'path', required: true, schema: { type: 'string' }, }, ], responses: { '200': { description: 'ok' } }, }, }); + expect(result.paths?.['/pets~dogs']).toEqual({ + get: { + operationId: 'listPetsDogs', + responses: { '200': { description: 'ok' } }, + }, + }); expect(result).not.toHaveProperty('x-ext');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orval/src/import-specs.test.ts` around lines 764 - 801, Add a third path to the test input to verify RFC6901 tilde-unescape: add an entry in the top-level input.paths with the key '/pets~0dogs' pointing via $ref to the external definition (use the JSON Pointer-escaped path under x-ext, e.g. '`#/x-ext/abc1234/paths/`~1pets~00dogs'), add the matching definition under input['x-ext'].abc1234.paths with an operation (e.g. get with operationId 'listPetsTilde') and then add an assertion that the resolved spec contains the decoded path '/pets~dogs' (or that the operationId is present for '/pets~dogs') to ensure '~0' is decoded to '~'.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/orval/src/import-specs.test.ts`:
- Around line 764-801: Add a third path to the test input to verify RFC6901
tilde-unescape: add an entry in the top-level input.paths with the key
'/pets~0dogs' pointing via $ref to the external definition (use the JSON
Pointer-escaped path under x-ext, e.g. '`#/x-ext/abc1234/paths/`~1pets~00dogs'),
add the matching definition under input['x-ext'].abc1234.paths with an operation
(e.g. get with operationId 'listPetsTilde') and then add an assertion that the
resolved spec contains the decoded path '/pets~dogs' (or that the operationId is
present for '/pets~dogs') to ensure '~0' is decoded to '~'.
In `@packages/orval/src/import-specs.ts`:
- Around line 347-357: Add a JSDoc block above the decodeRefToken function that
documents the two-layer decoding contract: percent-decoding (URI fragment) is
applied first and then JSON Pointer unescaping (~1 → /, ~0 → ~), note that
malformed percent-encodings are left unchanged (no throw), and include an
example showing a token like "%7B~1pets" -> "{/pets" to make the decoding order
and behavior explicit for future maintainers; reference the function name
decodeRefToken in the JSDoc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb77f9d4-dfd0-4541-9e17-fb8008d775ba
📒 Files selected for processing (7)
packages/orval/src/import-specs.test.tspackages/orval/src/import-specs.tstests/__snapshots__/default/issue-3380-external-path-ref/endpoints.tstests/api-generation.spec.tstests/configs/default.config.tstests/specifications/issue-3380/issue-3380-common.yamltests/specifications/issue-3380/issue-3380.yaml
Contributor
Author
|
Thanks for the review! Addressed the two nitpicks:
|
wadakatu
force-pushed
the
fix/issue-3380-external-path-ref
branch
from
May 18, 2026 18:25
49b6cf1 to
d172bac
Compare
melloware
approved these changes
May 18, 2026
This was referenced May 21, 2026
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
2 participants
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 #3380.
A cross-file path-item
$ref(e.g.$ref: "common.yaml#/paths/~1pets") aborts generation with anINVALID_REFERENCEvalidation error. External path-item$refis valid OpenAPI 3.x, and the same problem affects external$refs into any non-components/schemaslocation whose JSON Pointer contains escaped characters.Root cause
@scalar/json-magicbundles external files underx-extand rewrites refs to#/x-ext/<key>/....replaceXExtRefs(packages/orval/src/import-specs.ts) then resolves those by splitting the pointer on/and indexing the external document segment by segment.The segments are used verbatim, without decoding:
~1for/,~0for~(RFC 6901)%7B/%7Dfor{/}in templated pathsSo the segment
~1petsnever matches the real/petskey, the#/x-ext/...ref is left unresolved, and validation rejects it. Because every path key starts with/, every external path-item$refis affected. Component-schema refs happen to work only because their pointers contain no escaped characters.This is the external-file counterpart of #398 — #398 fixed escaped-token resolution in the core resolver (
resolveValue, PR #3355), but the x-ext bundling path inimport-specs.tswas not covered.Fix
Add
decodeRefToken, applied to each pointer segment before walking the external document: percent-decoding first (outer layer; malformed sequences fall back to the raw token instead of throwing), then JSON Pointer unescaping (~1→/,~0→~). The change is scoped to the inline-walk branch, which covers path items and any other non-components/schemasexternal ref.Tests
import-specs.test.ts) —dereferenceExternalRefnow resolves an x-ext path-item ref containing both~1and%7B/%7D. Verified red before the fix, green after.tests/specifications/issue-3380/) — a spec whose path items are cross-file$refs (~1petsand~1pets~1%7BpetId%7D); a focused assertion inapi-generation.spec.tsconfirms both operations are generated, alongside the snapshot.All checks pass locally:
format:check,lint,typecheck,test,test:snapshots, generated typecheck, and mock verification.Summary by CodeRabbit
Bug Fixes
Tests