Fix percent-encoded refs in ajv validation paths#845
Open
morgan-coded wants to merge 2 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR addresses schema $ref resolution issues involving percent-encoded sequences (Ajv fragment decoding vs fast-json-stringify literal key handling), and adds regression tests—covering both normal and standalone serializer generation.
Changes:
- Added
Validator.encodeRefFragment()and applied it to$refconversion and generated validation calls. - Added regression tests for issue #740 cases (oneOf/anyOf/if-then-else, literal
%in property names, and standalone mode). - Added a standalone-mode test ensuring
toJSON(Date) behavior matches inline compilation for oneOf schemas.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/standalone-mode.test.js | Adds a regression test comparing standalone vs inline serialization for toJSON objects under oneOf. |
| test/issue-740.test.js | New test suite covering percent-encoded schema keys and standalone mode regression for issue #740. |
| lib/validator.js | Adds fragment-encoding helper and ensures Ajv sees refs encoded to preserve literal schema keys. |
| index.js | Applies fragment encoding to schema refs used in generated Ajv validate() calls for oneOf and if/then/else. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const path = require('path') | ||
| const build = require('..') | ||
|
|
||
| process.env.TZ = 'UTC' |
Comment on lines
+112
to
+119
| const destination = path.resolve('test/fixtures', 'standalone-issue-740.js') | ||
|
|
||
| after(async () => { | ||
| await fs.promises.rm(destination, { force: true }) | ||
| }) | ||
|
|
||
| await fs.promises.writeFile(destination, code) | ||
| const standalone = require(destination) |
Comment on lines
+240
to
+242
| after(async () => { | ||
| await fs.promises.rm(destination, { force: true }) | ||
| }) |
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.
fast-json-stringifycould compile a schema with percent-encoded definition keys, then crash on the firststringify()when a validation branch went through ajv.The serializer resolves those JSON-pointer keys literally, while ajv decodes URI fragments before lookup. This keeps the existing literal-key behavior and re-encodes
%only for refs handed to ajv, including the schema clone used by standalone mode.I tested the issue repro plus the new branch and standalone regressions. The unit suite passes with 478 tests, tstyche passes 12/12, and lint plus
git diff --checkare clean.Closes #740