Skip to content

Fix handling of Windows absolute paths. #315

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions lib/refs.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { ono } from "@jsdevtools/ono";
import $Ref from "./ref.js";
import * as url from "./util/url.js";
import { isWindows } from "./util/is-windows.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);

const getPathFromOs = (filePath: string): string => (isWindows() ? filePath.replace(/\\/g, "/") : filePath);

interface $RefsMap {
[url: string]: $Ref;
Expand Down
7 changes: 7 additions & 0 deletions lib/util/is-windows.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/**
* Returns if the system is a Windows system or not
* @returns
*/
export function isWindows() {
return /^win/.test(globalThis.process ? globalThis.process.platform : "");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason this is a function now, and not a constant? globalThis.process should never change during runtime?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah for the test, I see

}
24 changes: 13 additions & 11 deletions lib/util/url.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const isWindows = /^win/.test(globalThis.process ? globalThis.process.platform : ""),
forwardSlashPattern = /\//g,
const forwardSlashPattern = /\//g,
protocolPattern = /^(\w{2,}):\/\//i,
jsonPointerSlash = /~1/g,
jsonPointerTilde = /~0/g;
import { join } from "path";
import { join, isAbsolute } from "path";
import { isWindows } from "./is-windows";

const projectDir = join(__dirname, "..", "..");
// RegExp patterns to URL-encode special characters in local filesystem paths
Expand Down Expand Up @@ -177,14 +177,16 @@ 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) {
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, "/");
const isAbsolutePath = isAbsolute(path);

if (!(hasProjectDir || hasProjectUri || isAbsolutePath)) {
path = join(projectDir,path);
}

path = path.replace(/\\/g, "/");
}

// Step 2: `encodeURI` will take care of MOST characters
Expand Down Expand Up @@ -222,7 +224,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);
}

Expand All @@ -234,12 +236,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, "\\");

Expand Down
47 changes: 45 additions & 2 deletions test/specs/util/url.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
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.js";

describe("Return the extension of a URL", () => {
it("should return an empty string if there isn't any extension", async () => {
Expand All @@ -18,3 +18,46 @@ describe("Return the extension of a URL", () => {
expect(extension).to.equal(".yml");
});
});

describe("Handle Windows file paths", () => {
beforeAll(function (this: any) {
//Force isWindows to always be true for this section of the test
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 = process.cwd().replace(/\\/g, '/');
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'));
});
});