Fix percent-encoded refs in ajv validation paths#845
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' |
| 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) |
| after(async () => { | ||
| await fs.promises.rm(destination, { force: true }) | ||
| }) |
|
|
||
| const code = build(buildIssueSchema(oneOfComposition), { mode: 'standalone' }) | ||
|
|
||
| const destination = path.resolve('test/fixtures', 'standalone-issue-740.js') |
There was a problem hiding this comment.
Can you use a temp folder instead?
| // https://github.com/fastify/fast-json-stringify/issues/740 | ||
| // Schema keys containing percent-encoded sequences (e.g. '%3C') are resolved | ||
| // literally by fast-json-stringify, but the refs handed to ajv were not | ||
| // re-encoded, so ajv percent-decoded them and failed to find the keys. |
There was a problem hiding this comment.
I checked the Ajv behavior behind that note. The schema key is literal (Some%3Cloremipsum%3E), but Ajv decodes the ref fragment while resolving it, so %3C/%3E can become </> and miss the literal key.
That’s why the fix goes through encodeRefFragment: it changes % to %25 before Ajv sees the ref, then Ajv’s decode step lands back on the literal % key. The focused issue/standalone run passes 10/10, covering oneOf, anyOf, if/then/else, and generated standalone output.
morgan-coded
force-pushed
the
fix/740-percent-encoded-ref-ajv
branch
from
63b8237 to
fa38320
Compare
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