fix(url): change url parsing behavior to actually respect RFC 6901

Author: jonluca

.claude/settings.local.json

@@ -1,8 +1,6 @@
 {
-  "enableAllProjectMcpServers": false,
   "permissions": {
-    "allow": [
-      "Bash(npx vitest:*)"
-    ]
-  }
+    "allow": ["Bash(npx vitest:*)", "Bash(bash:*)", "Bash(node test-debug.js:*)"]
+  },
+  "enableAllProjectMcpServers": false
 }

README.md

@@ -29,19 +29,15 @@ format. Maybe some of the files contain cross-references to each other.
 {
   "definitions": {
     "person": {
-      // references an external file
       "$ref": "schemas/people/Bruce-Wayne.json"
     },
     "place": {
-      // references a sub-schema in an external file
       "$ref": "schemas/places.yaml#/definitions/Gotham-City"
     },
     "thing": {
-      // references a URL
       "$ref": "http://wayne-enterprises.com/things/batmobile"
     },
     "color": {
-      // references a value in an external file via an internal reference
       "$ref": "#/definitions/thing/properties/colors/black-as-the-night"
     }
   }

lib/pointer.ts

@@ -13,14 +13,6 @@ const tildes = /~/g;
 const escapedSlash = /~1/g;
 const escapedTilde = /~0/g;
 
-const safeDecodeURIComponent = (encodedURIComponent: string): string => {
-  try {
-    return decodeURIComponent(encodedURIComponent);
-  } catch {
-    return encodedURIComponent;
-  }
-};
-
 /**
  * This class represents a single JSON pointer and its resolved value.
  *
@@ -124,7 +116,7 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
         // actually instead pointing to an existing `null` value then we should use that
         // `null` value.
         if (token in this.value && this.value[token] === null) {
-          // We use a `null` symbol for internal tracking to differntiate between a general `null`
+          // We use a `null` symbol for internal tracking to differentiate between a general `null`
           // value and our expected `null` value.
           this.value = nullSymbol;
           continue;
@@ -226,7 +218,7 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
 
     // Decode each part, according to RFC 6901
     for (let i = 0; i < split.length; i++) {
-      split[i] = safeDecodeURIComponent(split[i].replace(escapedSlash, "/").replace(escapedTilde, "~"));
+      split[i] = split[i].replace(escapedSlash, "/").replace(escapedTilde, "~");
     }
 
     if (split[0] !== "") {
@@ -254,7 +246,9 @@ class Pointer<S extends object = JSONSchema, O extends ParserOptions<S> = Parser
     for (let i = 0; i < tokens.length; i++) {
       const token = tokens[i];
       // Encode the token, according to RFC 6901
-      base += "/" + encodeURIComponent(token.replace(tildes, "~0").replace(slashes, "~1"));
+      // RFC 6901 only requires encoding ~ as ~0 and / as ~1
+      // We do NOT use encodeURIComponent as it encodes characters like $ which should remain literal
+      base += "/" + token.replace(tildes, "~0").replace(slashes, "~1");
     }
 
     return base;

lib/ref.ts

@@ -307,7 +307,7 @@ class $Ref<S extends object = JSONSchema, O extends ParserOptions<S> = ParserOpt
             typeof resolvedValue[key] === "object" &&
             resolvedValue[key] !== null
           ) {
-            merged[key] = deepMerge<typeof merged[keyof S]>(resolvedValue[key], merged[key]);
+            merged[key] = deepMerge<(typeof merged)[keyof S]>(resolvedValue[key], merged[key]);
           }
         }
       }

lib/util/url.ts

@@ -33,9 +33,15 @@ export function resolve(from: string, to: string) {
   if (resolvedUrl.hostname === "aaa.nonexistanturl.com") {
     // `from` is a relative URL.
     const { pathname, search, hash } = resolvedUrl;
-    return pathname + search + hash + endSpaces;
+    return pathname + search + decodeURIComponent(hash) + endSpaces;
   }
-  return resolvedUrl.toString() + endSpaces;
+  const resolved = resolvedUrl.toString() + endSpaces;
+  // if there is a #, we want to split on the first one only, and decode the part after
+  if (resolved.indexOf("#") >= 0) {
+    const [base, hash] = resolved.split(/#(.+)/);
+    return base + "#" + decodeURIComponent(hash);
+  }
+  return resolved;
 }
 
 /**
@@ -467,16 +473,16 @@ export function toFileSystemPath(path: string | undefined, keepFileProtocol?: bo
  * @param pointer
  * @returns
  */
-export function safePointerToPath(pointer: any) {
+export function safePointerToPath(pointer: string) {
   if (pointer.length <= 1 || pointer[0] !== "#" || pointer[1] !== "/") {
     return [];
   }
 
   return pointer
     .slice(2)
     .split("/")
-    .map((value: any) => {
-      return decodeURIComponent(value).replace(jsonPointerSlash, "/").replace(jsonPointerTilde, "~");
+    .map((value: string) => {
+      return value.replace(jsonPointerSlash, "/").replace(jsonPointerTilde, "~");
     });
 }
 

package.json

@@ -12,7 +12,7 @@
     "typecheck": "tsc --noEmit",
     "prettier": "prettier --write \"**/*.+(js|jsx|ts|tsx|har||json|css|md)\"",
     "test": "vitest --coverage",
-    "test:specific": "vitest defs-encoding",
+    "test:specific": "vitest invalid",
     "test:node": "yarn test",
     "test:browser": "cross-env BROWSER=\"true\" yarn test",
     "test:update": "vitest -u",

test/specs/absolute-root/bundled.ts

@@ -22,29 +22,29 @@ export default {
       minLength: 1,
     },
     string: {
-      $ref: "#/definitions/required%20string/type",
+      $ref: "#/definitions/required string/type",
     },
     name: {
       title: "name",
       type: "object",
       required: ["first", "last"],
       properties: {
         first: {
-          $ref: "#/definitions/required%20string",
+          $ref: "#/definitions/required string",
         },
         last: {
-          $ref: "#/definitions/required%20string",
+          $ref: "#/definitions/required string",
         },
         middle: {
           type: {
-            $ref: "#/definitions/required%20string/type",
+            $ref: "#/definitions/required string/type",
           },
           minLength: {
-            $ref: "#/definitions/required%20string/minLength",
+            $ref: "#/definitions/required string/minLength",
           },
         },
         prefix: {
-          $ref: "#/definitions/required%20string",
+          $ref: "#/definitions/required string",
           minLength: 3,
         },
         suffix: {

test/specs/absolute-root/definitions/definitions.json

@@ -3,7 +3,7 @@
     "$ref": "required-string.yaml"
   },
   "string": {
-    "$ref": "#/required%20string/type"
+    "$ref": "#/required string/type"
   },
   "name": {
     "$ref": "../definitions/name.yaml"

test/specs/absolute-root/parsed.ts

@@ -26,7 +26,7 @@ export default {
       $ref: "required-string.yaml",
     },
     string: {
-      $ref: "#/required%20string/type",
+      $ref: "#/required string/type",
     },
     name: {
       $ref: "../definitions/name.yaml",

test/specs/bundle-defs-encoding/bundle-defs-encoding.spec.ts

@@ -9,6 +9,9 @@ describe("Bundle $defs encoding", () => {
     const schema = path.rel("test/specs/bundle-defs-encoding/parent.json");
     const bundled = await parser.bundle(schema);
 
+    // Debug: log the bundled schema
+    console.log('Bundled schema:', JSON.stringify(bundled, null, 2));
+
     // The bundled schema should have $defs section with myschema that references otherschema
     expect(bundled).toHaveProperty("$defs");
     expect(bundled.$defs).toHaveProperty("myschema");