Fix windows paths and external refs (#321)

* chore: bump deps * chore: bump deps * add initial code for relative path fixing * feat: run crawling even when external ref is true BREAKING CHANGE: Change the file path parsing logic (to be safe)
APIDevTools · Sep 19, 2023 · b9f91b2 · b9f91b2
1 parent a5b3946
commit b9f91b2
Show file tree

Hide file tree

Showing 16 changed files with 1,404 additions and 899 deletions.
diff --git a/README.md b/README.md
@@ -124,6 +124,28 @@ JSON Schema $Ref Parser supports recent versions of every major web browser.  Ol
 To use JSON Schema $Ref Parser in a browser, you'll need to use a bundling tool such as [Webpack](https://webpack.js.org/), [Rollup](https://rollupjs.org/), [Parcel](https://parceljs.org/), or [Browserify](http://browserify.org/). Some bundlers may require a bit of configuration, such as setting `browser: true` in [rollup-plugin-resolve](https://github.com/rollup/rollup-plugin-node-resolve).
 
 
+#### Webpack 5
+Webpack 5 has dropped the default export of node core modules in favour of polyfills, you'll need to set them up yourself ( after npm-installing them )
+Edit your `webpack.config.js` :
+```js
+  config.resolve.fallback = {
+    "path": require.resolve("path-browserify"),
+    'util': require.resolve('util/'),
+    'fs': require.resolve('browserify-fs'),
+    "buffer": require.resolve("buffer/"),
+    "http": require.resolve("stream-http"),
+    "https": require.resolve("https-browserify"),
+    "url": require.resolve("url"),
+  }
+
+  config.plugins.push(
+    new webpack.ProvidePlugin({
+      Buffer: [ 'buffer', 'Buffer']
+    })
+  )
+
+```
+
 
 API Documentation
 --------------------------

diff --git a/lib/bundle.ts b/lib/bundle.ts
@@ -94,6 +94,7 @@ function crawl(
  * @param $refParent - The object that contains a JSON Reference as one of its keys
  * @param $refKey - The key in `$refParent` that is a JSON Reference
  * @param path - The full path of the JSON Reference at `$refKey`, possibly with a JSON Pointer in the hash
+ * @param indirections - unknown
  * @param pathFromRoot - The path of the JSON Reference at `$refKey`, from the schema root
  * @param inventory - An array of already-inventoried $ref pointers
  * @param $refs

diff --git a/lib/ref.ts b/lib/ref.ts
@@ -4,7 +4,6 @@ import { InvalidPointerError, isHandledError, normalizeError } from "./util/erro
 import { safePointerToPath, stripHash, getHash } from "./util/url.js";
 import type $Refs from "./refs.js";
 import type $RefParserOptions from "./options.js";
-import type { JSONSchema } from "./types";
 
 type $RefError = JSONParserError | ResolverError | ParserError | MissingPointerError;
 
@@ -167,7 +166,7 @@ class $Ref {
    * @param value - The value to inspect
    * @returns
    */
-  static isExternal$Ref(value: any): value is JSONSchema {
+  static isExternal$Ref(value: any): boolean {
     return $Ref.is$Ref(value) && value.$ref![0] !== "#";
   }
 

diff --git a/lib/refs.ts b/lib/refs.ts
@@ -4,9 +4,7 @@ import * as url from "./util/url.js";
 import type { JSONSchema4Type, JSONSchema6Type, JSONSchema7Type } from "json-schema";
 import type { JSONSchema } from "./types/index.js";
 import type $RefParserOptions from "./options.js";
-
-const isWindows = /^win/.test(globalThis.process ? globalThis.process.platform : "");
-const getPathFromOs = (filePath: string): string => (isWindows ? filePath.replace(/\\/g, "/") : filePath);
+import convertPathToPosix from "./util/convert-path-to-posix";
 
 interface $RefsMap {
   [url: string]: $Ref;
@@ -36,7 +34,7 @@ export default class $Refs {
   paths(...types: string[]): string[] {
     const paths = getPaths(this._$refs, types);
     return paths.map((path) => {
-      return getPathFromOs(path.decoded);
+      return convertPathToPosix(path.decoded);
     });
   }
 
@@ -51,7 +49,7 @@ export default class $Refs {
     const $refs = this._$refs;
     const paths = getPaths($refs, types);
     return paths.reduce<Record<string, any>>((obj, path) => {
-      obj[getPathFromOs(path.decoded)] = $refs[path.encoded].value;
+      obj[convertPathToPosix(path.decoded)] = $refs[path.encoded].value;
       return obj;
     }, {});
   }

diff --git a/lib/resolve-external.ts b/lib/resolve-external.ts
@@ -40,6 +40,7 @@ function resolveExternal(parser: $RefParser, options: Options) {
  *
  * @param obj - The value to crawl. If it's not an object or array, it will be ignored.
  * @param path - The full path of `obj`, possibly with a JSON Pointer in the hash
+ * @param {boolean} external - Whether `obj` was found in an external document.
  * @param $refs
  * @param options
  * @param seen - Internal.
@@ -56,6 +57,7 @@ function crawl(
   $refs: $Refs,
   options: Options,
   seen?: Set<any>,
+  external?: boolean,
 ) {
   seen ||= new Set();
   let promises: any = [];
@@ -64,17 +66,13 @@ function crawl(
     seen.add(obj); // Track previously seen objects to avoid infinite recursion
     if ($Ref.isExternal$Ref(obj)) {
       promises.push(resolve$Ref(obj, path, $refs, options));
-    } else {
-      for (const key of Object.keys(obj)) {
-        const keyPath = Pointer.join(path, key);
-        const value = obj[key] as string | JSONSchema | Buffer | undefined;
-
-        if ($Ref.isExternal$Ref(value)) {
-          promises.push(resolve$Ref(value, keyPath, $refs, options));
-        } else {
-          promises = promises.concat(crawl(value, keyPath, $refs, options, seen));
-        }
-      }
+    }
+
+    const keys = Object.keys(obj) as (keyof typeof obj)[];
+    for (const key of keys) {
+      const keyPath = Pointer.join(path, key);
+      const value = obj[key] as string | JSONSchema | Buffer | undefined;
+      promises = promises.concat(crawl(value, keyPath, $refs, options, seen, external));
     }
   }
 
@@ -99,6 +97,8 @@ async function resolve$Ref($ref: JSONSchema, path: string, $refs: $Refs, options
   const resolvedPath = url.resolve(path, $ref.$ref);
   const withoutHash = url.stripHash(resolvedPath);
 
+  // $ref.$ref = url.relative($refs._root$Ref.path, resolvedPath);
+
   // Do we already have this $ref?
   $ref = $refs._$refs[withoutHash];
   if ($ref) {
@@ -112,7 +112,7 @@ async function resolve$Ref($ref: JSONSchema, path: string, $refs: $Refs, options
 
     // Crawl the parsed value
     // console.log('Resolving $ref pointers in %s', withoutHash);
-    const promises = crawl(result, withoutHash + "#", $refs, options);
+    const promises = crawl(result, withoutHash + "#", $refs, options, new Set(), true);
 
     return Promise.all(promises);
   } catch (err) {

diff --git a/lib/resolvers/file.ts b/lib/resolvers/file.ts
@@ -1,4 +1,4 @@
-import fs from "fs/promises";
+import { promises as fs } from "fs";
 import { ono } from "@jsdevtools/ono";
 import * as url from "../util/url.js";
 import { ResolverError } from "../util/errors.js";

diff --git a/lib/util/convert-path-to-posix.ts b/lib/util/convert-path-to-posix.ts
@@ -0,0 +1,11 @@
+import path from "path";
+
+export default function convertPathToPosix(filePath: string) {
+  const isExtendedLengthPath = filePath.startsWith("\\\\?\\");
+
+  if (isExtendedLengthPath) {
+    return filePath;
+  }
+
+  return filePath.split(path.win32.sep).join(path.posix.sep);
+}
diff --git a/lib/util/is-windows.ts b/lib/util/is-windows.ts
@@ -0,0 +1,2 @@
+const isWindowsConst = /^win/.test(globalThis.process ? globalThis.process.platform : "");
+export const isWindows = () => isWindowsConst;
diff --git a/lib/util/url.ts b/lib/util/url.ts
@@ -1,9 +1,13 @@
-const isWindows = /^win/.test(globalThis.process ? globalThis.process.platform : ""),
-  forwardSlashPattern = /\//g,
-  protocolPattern = /^(\w{2,}):\/\//i,
-  jsonPointerSlash = /~1/g,
-  jsonPointerTilde = /~0/g;
+import convertPathToPosix from "./convert-path-to-posix";
+import path, { win32 } from "path";
+
+const forwardSlashPattern = /\//g;
+const protocolPattern = /^(\w{2,}):\/\//i;
+const jsonPointerSlash = /~1/g;
+const jsonPointerTilde = /~0/g;
+
 import { join } from "path";
+import { isWindows } from "./is-windows";
 
 const projectDir = join(__dirname, "..", "..");
 // RegExp patterns to URL-encode special characters in local filesystem paths
@@ -55,8 +59,8 @@ export function cwd() {
  * @param path
  * @returns
  */
-export function getProtocol(path: any) {
-  const match = protocolPattern.exec(path);
+export function getProtocol(path: string | undefined) {
+  const match = protocolPattern.exec(path || "");
   if (match) {
     return match[1].toLowerCase();
   }
@@ -146,7 +150,7 @@ export function isHttp(path: any) {
  * @param path
  * @returns
  */
-export function isFileSystemPath(path: any) {
+export function isFileSystemPath(path: string | undefined) {
   // @ts-ignore
   if (typeof window !== "undefined" || process.browser) {
     // We're running in a browser, so assume that all paths are URLs.
@@ -177,14 +181,18 @@ export function isFileSystemPath(path: any) {
 export function fromFileSystemPath(path: any) {
   // Step 1: On Windows, replace backslashes with forward slashes,
   // rather than encoding them as "%5C"
-  if (isWindows) {
-    const hasProjectDir = path.toUpperCase().includes(projectDir.replace(/\\/g, "\\").toUpperCase());
-    const hasProjectUri = path.toUpperCase().includes(projectDir.replace(/\\/g, "/").toUpperCase());
-    if (hasProjectDir || hasProjectUri) {
-      path = path.replace(/\\/g, "/");
-    } else {
-      path = `${projectDir}/${path}`.replace(/\\/g, "/");
+  if (isWindows()) {
+    const upperPath = path.toUpperCase();
+    const projectDirPosixPath = convertPathToPosix(projectDir);
+    const posixUpper = projectDirPosixPath.toUpperCase();
+    const hasProjectDir = upperPath.includes(posixUpper);
+    const hasProjectUri = upperPath.includes(posixUpper);
+    const isAbsolutePath = win32.isAbsolute(path);
+
+    if (!(hasProjectDir || hasProjectUri || isAbsolutePath)) {
+      path = join(projectDir, path);
     }
+    path = convertPathToPosix(path);
   }
 
   // Step 2: `encodeURI` will take care of MOST characters
@@ -222,7 +230,7 @@ export function toFileSystemPath(path: string | undefined, keepFileProtocol?: bo
     path = path[7] === "/" ? path.substr(8) : path.substr(7);
 
     // insert a colon (":") after the drive letter on Windows
-    if (isWindows && path[1] === "/") {
+    if (isWindows() && path[1] === "/") {
       path = path[0] + ":" + path.substr(1);
     }
 
@@ -234,12 +242,12 @@ export function toFileSystemPath(path: string | undefined, keepFileProtocol?: bo
       // On Windows, it will start with something like "C:/".
       // On Posix, it will start with "/"
       isFileUrl = false;
-      path = isWindows ? path : "/" + path;
+      path = isWindows() ? path : "/" + path;
     }
   }
 
   // Step 4: Normalize Windows paths (unless it's a "file://" URL)
-  if (isWindows && !isFileUrl) {
+  if (isWindows() && !isFileUrl) {
     // Replace forward slashes with backslashes
     path = path.replace(forwardSlashPattern, "\\");
 
@@ -270,3 +278,15 @@ export function safePointerToPath(pointer: any) {
       return decodeURIComponent(value).replace(jsonPointerSlash, "/").replace(jsonPointerTilde, "~");
     });
 }
+
+export function relative(from: string | undefined, to: string | undefined) {
+  if (!isFileSystemPath(from) || !isFileSystemPath(to)) {
+    return resolve(from, to);
+  }
+
+  const fromDir = path.dirname(stripHash(from));
+  const toPath = stripHash(to);
+
+  const result = path.relative(fromDir, toPath);
+  return result + getHash(to);
+}
diff --git a/package.json b/package.json
@@ -67,33 +67,32 @@
     "test:watch": "vitest -w"
   },
   "devDependencies": {
-    "@types/eslint": "8.4.10",
-    "@types/js-yaml": "^4.0.5",
-    "@types/node": "^18.11.18",
-    "@typescript-eslint/eslint-plugin": "^5.48.2",
-    "@typescript-eslint/eslint-plugin-tslint": "^5.48.2",
-    "@typescript-eslint/parser": "^5.48.2",
-    "@vitest/coverage-c8": "^0.28.1",
+    "@types/eslint": "8.44.2",
+    "@types/js-yaml": "^4.0.6",
+    "@types/node": "^20.6.2",
+    "@typescript-eslint/eslint-plugin": "^6.7.2",
+    "@typescript-eslint/eslint-plugin-tslint": "^6.7.2",
+    "@typescript-eslint/parser": "^6.7.2",
+    "@vitest/coverage-v8": "^0.34.4",
     "abortcontroller-polyfill": "^1.7.5",
-    "c8": "^7.12.0",
     "cross-env": "^7.0.3",
-    "eslint": "^8.32.0",
-    "eslint-config-prettier": "^8.6.0",
-    "eslint-config-standard": "^17.0.0",
-    "eslint-plugin-import": "^2.27.5",
-    "eslint-plugin-prettier": "^4.2.1",
+    "eslint": "^8.49.0",
+    "eslint-config-prettier": "^9.0.0",
+    "eslint-config-standard": "^17.1.0",
+    "eslint-plugin-import": "^2.28.1",
+    "eslint-plugin-prettier": "^5.0.0",
     "eslint-plugin-promise": "^6.1.1",
-    "eslint-plugin-unused-imports": "^2.0.0",
-    "jsdom": "^21.1.0",
-    "lint-staged": "^13.1.0",
-    "node-fetch": "^3.3.0",
-    "prettier": "^2.8.3",
-    "typescript": "^4.9.4",
-    "vitest": "^0.28.1"
+    "eslint-plugin-unused-imports": "^3.0.0",
+    "jsdom": "^22.1.0",
+    "lint-staged": "^14.0.1",
+    "node-fetch": "^3.3.2",
+    "prettier": "^3.0.3",
+    "typescript": "^5.2.2",
+    "vitest": "^0.34.4"
   },
   "dependencies": {
     "@jsdevtools/ono": "^7.1.3",
-    "@types/json-schema": "^7.0.11",
+    "@types/json-schema": "^7.0.13",
     "@types/lodash.clonedeep": "^4.5.7",
     "js-yaml": "^4.1.0",
     "lodash.clonedeep": "^4.5.0"

diff --git a/test/specs/util/url.spec.ts b/test/specs/util/url.spec.ts
@@ -1,7 +1,7 @@
-import { describe, it } from "vitest";
-import { expect } from "vitest";
+import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 import * as $url from "../../../lib/util/url.js";
-
+import * as isWin from "../../../lib/util/is-windows";
+import convertPathToPosix from "../../../lib/util/convert-path-to-posix";
 describe("Return the extension of a URL", () => {
   it("should return an empty string if there isn't any extension", async () => {
     const extension = $url.getExtension("/file");
@@ -18,3 +18,52 @@ describe("Return the extension of a URL", () => {
     expect(extension).to.equal(".yml");
   });
 });
+describe("Handle Windows file paths", () => {
+  beforeAll(function (this: any) {
+    vi.spyOn(isWin, "isWindows").mockReturnValue(true);
+  });
+
+  afterAll(function (this: any) {
+    vi.restoreAllMocks();
+  });
+
+  it("should handle absolute paths", async () => {
+    const result = $url.fromFileSystemPath("Y:\\A\\Random\\Path\\file.json");
+    expect(result)
+      .to.be.a("string")
+      .and.toSatisfy((msg: string) => msg.startsWith("Y:/A/Random/Path"));
+  });
+
+  it("should handle relative paths", async () => {
+    const result = $url.fromFileSystemPath("Path\\file.json");
+    const pwd = convertPathToPosix(process.cwd());
+    expect(result)
+      .to.be.a("string")
+      .and.toSatisfy((msg: string) => msg.startsWith(pwd));
+  });
+});
+
+describe("Handle Linux file paths", () => {
+  beforeAll(function (this: any) {
+    //Force isWindows to always be false for this section of the test
+    vi.spyOn(isWin, "isWindows").mockReturnValue(false);
+  });
+
+  afterAll(function (this: any) {
+    vi.restoreAllMocks();
+  });
+
+  it("should handle absolute paths", async () => {
+    const result = $url.fromFileSystemPath("/a/random/Path/file.json");
+    expect(result)
+      .to.be.a("string")
+      .and.toSatisfy((msg: string) => msg.startsWith("/a/random/Path/file.json"));
+  });
+
+  it("should handle relative paths", async () => {
+    const result = $url.fromFileSystemPath("Path/file.json");
+    expect(result)
+      .to.be.a("string")
+      .and.toSatisfy((msg: string) => msg.startsWith("Path/file.json"));
+  });
+});
diff --git a/test/tsconfig.json b/test/tsconfig.json
@@ -10,7 +10,7 @@
     "declaration": true,
     "esModuleInterop": true,
     "inlineSourceMap": false,
-    "lib": ["esnext", "dom"],
+    "lib": ["esnext", "dom", "DOM"],
     "listEmittedFiles": false,
     "listFiles": false,
     "moduleResolution": "node16",