Skip to content

test(json-pointer): add boundary, escape, and normalization coverage - #1007

Open
vtushar06 wants to merge 4 commits into
json-schema-org:mainfrom
vtushar06:json-pointer-coverage
Open

vtushar06 wants to merge 4 commits into
json-schema-org:mainfrom
vtushar06:json-pointer-coverage

Conversation

@vtushar06

@vtushar06 vtushar06 commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

I walked the RFC 6901 section 3 grammar against the current optional/format/json-pointer.json (40 tests) and found seven distinct cases the suite does not have anywhere, each of which can catch a wrong implementation the existing tests let through. All seven are handled correctly by ajv-formats 3.0.1 (full and fast) and python-jsonschema 4.25.1, so they do not break the dominant validators - they close real gaps.

Changes

Seven tests added to tests/*/optional/format/json-pointer.json (draft6, draft7, draft2019-09, draft2020-12). Control, DEL, and fullwidth characters are stored as \uXXXX escapes, the same way the existing NUL/LF/TAB test does.

Character-class boundaries (a hand-rolled unescaped character class with an off-by-one flips these):

  • /01 (valid) - a leading-zero index-looking token is a valid string. The leading-zero ban in section 4 is about resolving an array-index, not section-3 string validity. The suite has /foo/0 but no multi-digit leading-zero token.
  • /\u007f (valid) - DEL, the first code point of the third range %x7F-10FFFF. The suite covers %x00-1F and an astral char, but nothing pins the %x7E(~)/%x7F boundary.
  • /} (valid) - } is %x7D, the top of the middle range %x30-7D, one code point below the excluded ~ (%x7E). The suite has | (%x7C, in /g|h) but not }, so a class written [\x30-\x7C ...] ships the off-by-one silently.

Escape and NUL handling:

  • /foo\u0000~ (invalid) - a dangling ~ after an embedded NUL. An implementation that copies the string into a null-terminated buffer truncates at the NUL, sees /foo, and wrongly accepts. The existing NUL test is valid data, so a truncating implementation passes it.
  • /~0 (invalid) - a ~ followed by a fullwidth digit zero U+FF10. A ~ must be followed by ASCII 0/1; a validator that completes the escape with a Unicode-aware digit test (\d with the Unicode flag, Character.isDigit, ...) wrongly accepts it. /~2 does not catch this - an ASCII [01] check correctly rejects /~2 yet can still accept /~0.

Normalization resistance (section 3 does no normalization):

  • /~ (valid) - a fullwidth tilde U+FF5E is an ordinary token char, not the escape introducer ~. A validator that applies NFKC before checking folds it to ~ and wrongly rejects it as a dangling escape.

Alternate representation:

  • #/foo (invalid) - the section 6 URI-fragment form with a real pointer tail. The suite has #, #/, #a, but a validator that strips a leading # and validates the rest passes all three (it rejects a bare #) while accepting #/foo. Newtonsoft.Json.Schema 4.0.2 does exactly this.

RFC References

I left out //, ///, and ~01 on purpose - PR #877 already declined those as redundant.

@vtushar06
vtushar06 requested a review from a team as a code owner July 21, 2026 16:18
Copilot AI review requested due to automatic review settings July 21, 2026 16:18

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vtushar06
vtushar06 force-pushed the json-pointer-coverage branch from aa34958 to f526859 Compare July 21, 2026 16:23
Three more distinct cases the suite does not have. "/}" pins the %x7D top of the middle unescaped range (the suite has "|" = %x7C via /g|h but not "}"). Two normalization-resistance cases: a fullwidth tilde U+FF5E is an ordinary char, not the escape introducer ~, so /<U+FF5E> is valid; a ~ followed by a fullwidth digit U+FF10 is an invalid escape (~ must be followed by ASCII 0/1). RFC 6901 section 3 does no normalization.
@vtushar06 vtushar06 changed the title test(json-pointer): cover leading-zero, DEL, post-NUL, and fragment-tail cases test(json-pointer): add boundary, escape, and normalization coverage Jul 22, 2026
@vtushar06

Copy link
Copy Markdown
Contributor Author

hey @jviotti please have a look at this whenever you have timewidth.

@jviotti jviotti left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like good extra coverage to me

Comment thread tests/draft2019-09/optional/format/json-pointer.json
Comment thread tests/draft2019-09/optional/format/json-pointer.json Outdated
No implementation in the matrix fails it - every validator that asserts json-pointer rejects it correctly, so it only guarded against a hypothetical implementation that truncates at a NUL.

@jdesrosiers jdesrosiers left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The v1 tests are out of sync with the other branches.

The v1 fixture had drifted: it still carried the post-NUL dangling tilde
case that was dropped from the other drafts, and it was missing the three
cases added later (right curly bracket, fullwidth tilde, tilde then
fullwidth digit). v1 keeps format tests under tests/v1/format rather than
tests/*/optional/format, so it was missed by the earlier edits. All five
fixtures now hold the same 46 tests and differ only in $schema.
@vtushar06

vtushar06 commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@jdesrosiers i think you are very right on this all five files now hold the same 46 tests and differ only in $schema.

@vtushar06
vtushar06 requested a review from jdesrosiers August 19, 2026 06:30

@vtushar06 vtushar06 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ready for final look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants