apply review suggestinos

Author: tatomyr

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

@@ -1,6 +1,11 @@
 import outdent from 'outdent';
 import { parseYamlToDocument } from '../../__tests__/utils.js';
-import { escapePointer, parseRef, refBaseName, unescapePointer } 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~1complex~0name';
-    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` 
@@ -140,32 +159,37 @@ describe('ref-utils', () => {
     });
   });
 
-  describe('escapePointer', () => {
-    it('should escape a pointer correctly', () => {
-      expect(escapePointer('scope/complex~name')).toStrictEqual('scope~1complex~0name');
+  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 with a number correctly', () => {
-      expect(escapePointer(123)).toStrictEqual(123);
+    it('should escape a pointer fragment with a number correctly', () => {
+      expect(escapePointerFragment(123)).toStrictEqual(123);
     });
 
-    it('should escape a pointer with a string correctly', () => {
-      expect(escapePointer('scope/complex~name')).toStrictEqual('scope~1complex~0name');
+    it('should URI-encode special characters when escaping pointer fragments per https://datatracker.ietf.org/doc/html/rfc6901#section-6', () => {
+      expect(escapePointerFragment('percent%')).toStrictEqual('percent%25');
+      expect(escapePointerFragment('caret^')).toStrictEqual('caret%5E');
+      expect(escapePointerFragment('pipe|')).toStrictEqual('pipe%7C');
+      expect(escapePointerFragment('doublebackslash\\\\')).toStrictEqual('doublebackslash%5C');
+      expect(escapePointerFragment('backslash\\')).toStrictEqual('backslash%22');
+      expect(escapePointerFragment('space ')).toStrictEqual('space%20');
     });
 
-    it('should not URI-encode special characters when escaping pointers', () => {
-      // escapePointer should only handle JSON Pointer escaping, not URI encoding
-      expect(escapePointer('activity_level_%')).toStrictEqual('activity_level_%25');
+    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('unescapePointer', () => {
+  describe('unescapePointerFragment', () => {
     it('should unescape a pointer with a percent sign correctly', () => {
-      expect(unescapePointer('activity_level_%25')).toStrictEqual('activity_level_%');
+      expect(unescapePointerFragment('activity_level_%25')).toStrictEqual('activity_level_%');
     });
 
     it('should unescape a pointer correctly', () => {
-      expect(unescapePointer('scope~1complex~0name')).toStrictEqual('scope/complex~name');
+      expect(unescapePointerFragment('scope~1complex~0name')).toStrictEqual('scope/complex~name');
     });
   });
 });

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,11 +40,11 @@ export class Location {
   }
 }
 
-export function unescapePointer(fragment: string): string {
+export function unescapePointerFragment(fragment: string): string {
   return decodeURIComponent(fragment).replaceAll('~1', '/').replaceAll('~0', '~');
 }
 
-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 encodeURIFragmentIdentifier(fragment.replaceAll('~', '~0').replaceAll('/', '~1')) as T;
 }
@@ -69,7 +69,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';
@@ -95,7 +95,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 {