fix: added encoding on absolutePointer before decodeURIComponent

Author: DmitryAnansky

.changeset/solid-lands-repair.md

@@ -0,0 +1,6 @@
+---
+"@redocly/openapi-core": patch
+---
+
+Fixed an issue where JSON Pointers containing special characters (like `%`) were not properly URI-encoded.
+When these pointers were used as URI identifiers, they caused validation errors with properties containing percent signs or other special characters.

.github/workflows/performance.yaml

@@ -14,40 +14,7 @@ env:
   REDOCLY_TELEMETRY: off
 
 jobs:
-  latest-vs-next:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-      - uses: actions/setup-node@v3
-        with:
-          node-version: 24
-          cache: npm
-      - name: Install Dependencies
-        run: npm ci
-      - name: Install External
-        run: npm i -g hyperfine @redocly/cli@latest
-      - name: Prepare
-        run: |
-          jq '.bin = {"redocly-next":"bin/cli.js"} | .name = "@redocly/cli-next"' packages/cli/package.json > __tmp__.json
-          mv __tmp__.json packages/cli/package.json
-          cat packages/cli/package.json
-          npm run pack:prepare
-          npm i -g redocly-cli.tgz
-      - run: redocly-next --version
-      - run: redocly --version
-      - name: Run Benchmark
-        run: hyperfine -i --warmup 3 'redocly lint resources/rebilly.yaml' 'redocly-next lint resources/rebilly.yaml' --export-markdown benchmark_check.md --export-json benchmark_check.json
-
-      - name: Comment PR
-        if: ${{ github.event.pull_request.head.repo.full_name == github.repository }}
-        uses: thollander/actions-comment-pull-request@v2
-        with:
-          filePath: benchmark_check.md
-          comment_tag: latest-vs-next-comparison
-
   historical-versions:
-    # Run only on release branch (changeset-release/main):
-    if: github.head_ref == 'changeset-release/main'
     runs-on: ubuntu-latest
     steps:
       - uses: actions/checkout@v4
@@ -59,17 +26,27 @@ jobs:
         run: npm ci
       - name: Install External
         run: npm i -g hyperfine
+
+      - name: Add more versions to test
+        # Run only on the release branch (changeset-release/main):
+        if: github.head_ref == 'changeset-release/main'
+        run: |
+          cd tests/performance/
+          cat package.json | jq ".dependencies = $(cat package.json | jq ._enhancedDependencies)" > package.json
+          cat package.json
+
       - name: Prepare
         run: |
           npm run compile
           npm run pack:prepare
           cd tests/performance/
           npm i
           npm run make-test
+
       - name: Run Benchmark
         run: |
           cd tests/performance/
-          npm test # This command is generated and injected into package.json in the previous step.
+          npm run test # This command is generated and injected into package.json in the previous step.
           cat benchmark_check.md
           npm run chart # Creates benchmark_chart.md with the performance bar chart.
 

CONTRIBUTING.md

@@ -286,10 +286,16 @@ For other commands you'd have to do something similar.
 ### Performance benchmark
 
 To run the performance tests locally, you should have `hyperfine` (v1.16.1+) installed on your machine.
-Prepare the local build, go to the `tests/performance` folder, clean it up, do the preparations, and run the actual test:
+Prepare the local build, go to the `tests/performance` folder, clean it up, do the preparations:
 
 ```sh
-(npm run compile && npm run pack:prepare && cd tests/performance/ && git clean -dX -f . && git clean -dX -ff . && npm i && npm run make-test && npm test)
+(npm run compile && npm run pack:prepare && cd tests/performance/ && git clean -dX -f . && git clean -dX -ff .  && rm -rf node_modules && rm -f package-lock.json && npm i && npm run make-test)
+```
+
+and run the actual test:
+
+```sh
+(cd tests/performance/ && npm run test && cat benchmark_check.md)
 ```
 
 You might need to adjust the CLI versions that need to be tested in the `tests/performance/package.json` file.

packages/core/src/__tests__/ref-utils.test.ts

@@ -1,6 +1,11 @@
 import outdent from 'outdent';
 import { parseYamlToDocument } from '../../__tests__/utils.js';
-import { parseRef, refBaseName } from '../ref-utils.js';
+import {
+  escapePointerFragment,
+  parseRef,
+  refBaseName,
+  unescapePointerFragment,
+} from '../ref-utils.js';
 import { lintDocument } from '../lint.js';
 import { createConfig } from '../config/index.js';
 import { BaseResolver } from '../resolve.js';
@@ -35,8 +40,8 @@ describe('ref-utils', () => {
   });
 
   it(`should unescape complex urlencoded paths`, () => {
-    const referene = 'somefile.yaml#/components/schemas/scope%2Fcomplex~name';
-    expect(parseRef(referene)).toMatchInlineSnapshot(`
+    const reference = 'somefile.yaml#/components/schemas/scope%2Fcomplex~name';
+    expect(parseRef(reference)).toMatchInlineSnapshot(`
       {
         "pointer": [
           "components",
@@ -48,6 +53,20 @@ describe('ref-utils', () => {
     `);
   });
 
+  it(`should unescape escaped paths`, () => {
+    const reference = 'somefile.yaml#/components/schemas/scope~1complex~0name with spaces';
+    expect(parseRef(reference)).toMatchInlineSnapshot(`
+      {
+        "pointer": [
+          "components",
+          "schemas",
+          "scope/complex~name with spaces",
+        ],
+        "uri": "somefile.yaml",
+      }
+    `);
+  });
+
   it(`should validate definition with urlencoded paths`, async () => {
     const document = parseYamlToDocument(
       outdent` 
@@ -139,4 +158,29 @@ describe('ref-utils', () => {
       expect(refBaseName('abcdefg')).toStrictEqual('abcdefg');
     });
   });
+
+  describe('escapePointerFragment', () => {
+    it('should escape a simple pointer fragment with ~ and / correctly', () => {
+      expect(escapePointerFragment('scope/complex~name')).toStrictEqual('scope~1complex~0name');
+    });
+
+    it('should escape a pointer fragment with a number correctly', () => {
+      expect(escapePointerFragment(123)).toStrictEqual(123);
+    });
+
+    it('should not URI-encode other special characters when escaping pointer fragments per https://datatracker.ietf.org/doc/html/rfc6901#section-6', () => {
+      expect(escapePointerFragment('curly{braces}')).toStrictEqual('curly{braces}');
+      expect(escapePointerFragment('plus+')).toStrictEqual('plus+');
+    });
+  });
+
+  describe('unescapePointerFragment', () => {
+    it('should unescape a pointer with a percent sign correctly', () => {
+      expect(unescapePointerFragment('activity_level_%25')).toStrictEqual('activity_level_%');
+    });
+
+    it('should unescape a pointer correctly', () => {
+      expect(unescapePointerFragment('scope~1complex~0name')).toStrictEqual('scope/complex~name');
+    });
+  });
 });

packages/core/src/format/codeframes.ts

@@ -1,5 +1,5 @@
 import * as yamlAst from 'yaml-ast-parser';
-import { unescapePointer } from '../ref-utils.js';
+import { parsePointer } from '../ref-utils.js';
 import { colorize, colorOptions } from '../logger.js';
 
 import type { LineColLocationObject, Loc, LocationObject } from '../walk.js';
@@ -14,11 +14,6 @@ type YAMLNode = YAMLMapping | YAMLMap | YAMLAnchorReference | YAMLSequence | YAM
 const MAX_LINE_LENGTH = 150;
 const MAX_CODEFRAME_LINES = 3;
 
-// TODO: temporary
-function parsePointer(pointer: string) {
-  return pointer.substr(2).split('/').map(unescapePointer);
-}
-
 export function getCodeframe(location: LineColLocationObject, color: boolean) {
   colorOptions.enabled = color;
   const { start, end = { line: start.line, col: start.col + 1 }, source } = location;
@@ -181,7 +176,8 @@ function positionsToLoc(
 }
 
 export function getAstNodeByPointer(root: YAMLNode, pointer: string, reportOnKey: boolean) {
-  const pointerSegments = parsePointer(pointer);
+  const pointerSegments = parsePointer(pointer.substr(2));
+
   if (root === undefined) {
     return undefined;
   }

packages/core/src/index.ts

@@ -52,10 +52,10 @@ export {
 export { YamlParseError } from './errors/yaml-parse-error.js';
 export { parseYaml, stringifyYaml } from './js-yaml/index.js';
 export {
-  unescapePointer,
+  unescapePointerFragment,
   isRef,
   isAbsoluteUrl,
-  escapePointer,
+  escapePointerFragment,
   type Location,
 } from './ref-utils.js';
 export { detectSpec } from './detect-spec.js';

packages/core/src/ref-utils.ts

@@ -26,7 +26,7 @@ export class Location {
       this.source,
       joinPointer(
         this.pointer,
-        (Array.isArray(components) ? components : [components]).map(escapePointer).join('/')
+        (Array.isArray(components) ? components : [components]).map(escapePointerFragment).join('/')
       )
     );
   }
@@ -40,13 +40,19 @@ export class Location {
   }
 }
 
-export function unescapePointer(fragment: string): string {
-  return decodeURIComponent(fragment.replace(/~1/g, '/').replace(/~0/g, '~'));
+export function unescapePointerFragment(fragment: string): string {
+  const unescaped = fragment.replace(/~1/g, '/').replace(/~0/g, '~');
+
+  try {
+    return decodeURIComponent(unescaped);
+  } catch (e) {
+    return unescaped;
+  }
 }
 
-export function escapePointer<T extends string | number>(fragment: T): T {
+export function escapePointerFragment<T extends string | number>(fragment: T): T {
   if (typeof fragment === 'number') return fragment;
-  return (fragment as string).replace(/~/g, '~0').replace(/\//g, '~1') as T;
+  return fragment.replaceAll('~', '~0').replaceAll('/', '~1') as T;
 }
 
 export function parseRef(ref: string): { uri: string | null; pointer: string[] } {
@@ -58,7 +64,7 @@ export function parseRef(ref: string): { uri: string | null; pointer: string[] }
 }
 
 export function parsePointer(pointer: string) {
-  return pointer.split('/').map(unescapePointer).filter(isTruthy);
+  return pointer.split('/').map(unescapePointerFragment).filter(isTruthy);
 }
 
 export function pointerBaseName(pointer: string) {

packages/core/src/resolve.ts

@@ -3,7 +3,7 @@ import * as path from 'node:path';
 import {
   isRef,
   joinPointer,
-  escapePointer,
+  escapePointerFragment,
   parseRef,
   isAbsoluteUrl,
   isAnchor,
@@ -315,7 +315,7 @@ export async function resolveDocument(opts: {
           continue;
         }
 
-        walk(propValue, propType, joinPointer(nodeAbsoluteRef, escapePointer(propName)));
+        walk(propValue, propType, joinPointer(nodeAbsoluteRef, escapePointerFragment(propName)));
       }
 
       if (isRef(node)) {
@@ -426,7 +426,10 @@ export async function resolveDocument(opts: {
           break;
         } else if (target[segment] !== undefined) {
           target = target[segment];
-          resolvedRef.nodePointer = joinPointer(resolvedRef.nodePointer!, escapePointer(segment));
+          resolvedRef.nodePointer = joinPointer(
+            resolvedRef.nodePointer!,
+            escapePointerFragment(segment)
+          );
         } else if (isRef(target)) {
           resolvedRef = await followRef(targetDoc, target, pushRef(refStack, target));
           targetDoc = resolvedRef.document || targetDoc;
@@ -437,7 +440,10 @@ export async function resolveDocument(opts: {
           }
 
           target = resolvedRef.node[segment];
-          resolvedRef.nodePointer = joinPointer(resolvedRef.nodePointer!, escapePointer(segment));
+          resolvedRef.nodePointer = joinPointer(
+            resolvedRef.nodePointer!,
+            escapePointerFragment(segment)
+          );
         } else {
           target = undefined;
           break;

packages/core/src/rules/ajv.ts

@@ -1,6 +1,6 @@
 import addFormats from 'ajv-formats';
 import Ajv from '@redocly/ajv/dist/2020.js';
-import { escapePointer } from '../ref-utils.js';
+import { escapePointerFragment } from '../ref-utils.js';
 
 import type { Location } from '../ref-utils.js';
 import type { ValidateFunction, ErrorObject } from '@redocly/ajv/dist/2020.js';
@@ -44,12 +44,13 @@ function getAjvValidator(
   allowAdditionalProperties: boolean
 ): ValidateFunction | undefined {
   const ajv = getAjv(resolve, allowAdditionalProperties);
+  const $id = encodeURI(loc.absolutePointer);
 
-  if (!ajv.getSchema(loc.absolutePointer)) {
-    ajv.addSchema({ $id: loc.absolutePointer, ...schema }, loc.absolutePointer);
+  if (!ajv.getSchema($id)) {
+    ajv.addSchema({ $id, ...schema }, $id);
   }
 
-  return ajv.getSchema(loc.absolutePointer);
+  return ajv.getSchema($id);
 }
 
 export function validateJsonSchema(
@@ -95,7 +96,7 @@ export function validateJsonSchema(
     if (error.keyword === 'additionalProperties' || error.keyword === 'unevaluatedProperties') {
       const property = error.params.additionalProperty || error.params.unevaluatedProperty;
       message = `${message} \`${property}\``;
-      error.instancePath += '/' + escapePointer(property);
+      error.instancePath += '/' + escapePointerFragment(property);
     }
 
     return {

tests/e2e/lint/no-invalid-schema-examples-oas3.1-error/openapi.yaml

@@ -39,6 +39,10 @@ paths:
                   examples:
                     - test
                     - 25
+                three_%:
+                  type: number
+                  example: wrong
+
       responses:
         '200':
           description: My 200 response