Skip to content

fix: escape literal property names in data pointers - #129

Closed
fitchmultz wants to merge 1 commit into
sagold:mainfrom
fitchmultz:fix/escaped-data-pointers
Closed

fitchmultz wants to merge 1 commit into
sagold:mainfrom
fitchmultz:fix/escaped-data-pointers

Conversation

@fitchmultz

@fitchmultz fitchmultz commented Sep 21, 2026 •

Copy link
Copy Markdown

Problem

Data pointers concatenate property names without escaping / or ~. Distinct locations can therefore produce the same error pointer:

const node = compileSchema({
    properties: {
        "a/b": { properties: { c: { type: "number" } } },
        a: { properties: { "b/c": { type: "number" } } }
    }
});
node.validate({ "a/b": { c: "invalid" }, a: { "b/c": "invalid" } });

Both errors currently report #/a/b/c; they should report #/a~1b/c and #/a/b~1c.

Changes

Use the existing @sagold/json-pointer array-form join to append escaped literal segments, preserving the existing prefix verbatim. Apply it to property validation, nested property-name and oneOf-declarator errors, getNode traversal diagnostics, and toDataNodes. Empty names and numeric indexes remain intact. Schema-location strings and validation rules are unchanged.

Verification

Using pnpm 10 with the frozen lockfile:

  • All 15 new regressions fail on the original code and pass with this change, including valid/invalid controls.
  • pnpm run test:unit: 1,037 passing, 3 pending.
  • pnpm run test:spec: 7,513 passing, 9 pending.
  • pnpm run dist: passes; the built ESM entry point also passes the repro and a valid-data control. Generated bundles are left to the release process.
  • lint:src, lint:types, and lint:tests still fail with the same 3, 4, and 107 diagnostics, respectively, reproduced on a clean 65578c1 baseline. No new diagnostics.

Upstream CI is awaiting maintainer approval for this fork contribution; no CI jobs have run yet.

If accepted, would you consider including this in a stable patch release? Thank you.

@fitchmultz

Copy link
Copy Markdown
Author

Closing this as superseded by #130. That pull request is based on current main and already contains this pointer commit unchanged (91722a5), followed by the annotation corrections. Please review #130 rather than both.

@fitchmultz

Copy link
Copy Markdown
Author

Superseded by #130, which includes this commit.

@fitchmultz fitchmultz closed this Sep 21, 2026
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.

1 participant