Skip to content
Merged
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
26 changes: 18 additions & 8 deletions lib/util/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const urlEncodePatterns = [
] as [RegExp, string][];

// RegExp patterns to URL-decode special characters for local filesystem paths
const urlDecodePatterns = [/%23/g, "#", /%24/g, "$", /%26/g, "&", /%2C/g, ",", /%40/g, "@"];
const urlDecodePatterns = [/%23/g, "#", /%24/g, "$", /%26/g, "&", /%2C/g, ",", /%3F/g, "?", /%40/g, "@"];

const unsafeDomainSuffixes = [".localhost", ".local", ".internal", ".intranet", ".corp", ".home", ".lan"];

Expand Down Expand Up @@ -638,16 +638,26 @@ export function toFileSystemPath(path: string | undefined, keepFileProtocol?: bo
// Escape only the non-encoded ones so percent-encoded sequences still decode normally.
path = path!.replace(/%(?![0-9A-Fa-f]{2})/g, "%25");

// Step 1: `decodeURI` will decode characters such as Cyrillic characters, spaces, etc.
path = decodeURI(path!);

// Step 2: Manually decode characters that are not decoded by `decodeURI`.
// This includes characters such as "#" and "?", which have special meaning in URLs,
// but are just normal characters in a filesystem path.
// Step 1: Manually decode characters that `decodeURI` intentionally leaves alone
// (they're URI-reserved) but are just normal characters in a filesystem path,
// e.g. "#" and "?". This MUST run before `decodeURI`, not after: `fromFileSystemPath`
// percent-escapes a literal "%" as "%25", so a real filename containing the literal
// text "%3F" round-trips as "%253F". If `decodeURI` runs first, "%25" -> "%" reveals
// a literal "%3F" substring that didn't exist in the encoded form, and this pass
// would then wrongly decode that revealed text a second time (%3F -> "?"), silently
// reading the wrong file. Running this pass first, the literal "%253F" doesn't
// contain the substring "%3F" (it's "%25" followed by "3F"), so it's untouched here
// and correctly decoded once by `decodeURI` below (%25 -> %). Each escape in the
// original filename is consumed exactly once, either here or by `decodeURI`, never
// both. Case-insensitive: percent-encoding hex digits are case-insensitive per
// RFC 3986 §2.1, so "%3f" must decode the same as "%3F".
for (let i = 0; i < urlDecodePatterns.length; i += 2) {
path = path.replace(urlDecodePatterns[i], urlDecodePatterns[i + 1] as string);
path = path.replace(new RegExp((urlDecodePatterns[i] as RegExp).source, "gi"), urlDecodePatterns[i + 1] as string);
}

// Step 2: `decodeURI` will decode characters such as Cyrillic characters, spaces, etc.
path = decodeURI(path!);

// Step 3: If it's a "file://" URL, then format it consistently
// or convert it to a local filesystem path
let isFileUrl = path.toLowerCase().startsWith("file://");
Expand Down
50 changes: 50 additions & 0 deletions test/specs/util/url.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,3 +152,53 @@ describe("Handle Linux file paths", () => {
expect($url.toFileSystemPath("FILE:///a/random/Path/file.json")).to.equal("/a/random/Path/file.json");
});
});

describe("Round-trip special characters in filesystem paths", () => {
// POSIX-only: on Windows, fromFileSystemPath prepends cwd() and normalizes
// separators, so these absolute-POSIX-path assertions don't hold. Mocked the
// same way as the "Handle Linux file paths" block above.
beforeAll(function (this: any) {
vi.spyOn(isWin, "isWindows").mockReturnValue(false);
});

afterAll(function (this: any) {
vi.restoreAllMocks();
});

it("should round-trip a literal question mark", () => {
const original = "/a/random/Path/defs?1.json";
const encoded = $url.fromFileSystemPath(original);
expect(encoded).to.equal("/a/random/Path/defs%3F1.json");
expect($url.toFileSystemPath(encoded)).to.equal(original);
});

it("should round-trip a literal hash", () => {
const original = "/a/random/Path/defs#1.json";
const encoded = $url.fromFileSystemPath(original);
expect(encoded).to.equal("/a/random/Path/defs%231.json");
expect($url.toFileSystemPath(encoded)).to.equal(original);
});

it("should round-trip a filename that literally contains the text '%3F' without double-decoding it", () => {
// Regression test (jonluca, PR review 2026-08-07): a filename whose NAME is the
// literal text "%3F" (percent, 3, F -- not an encoded "?") must not be silently
// read as the "?" sibling file. fromFileSystemPath escapes the literal "%" to
// "%25", producing "%253F" -- if toFileSystemPath decoded %25->% before consuming
// reserved escapes, the revealed "%3F" text would wrongly decode to "?" a second
// time. It must decode back to the exact original literal text instead.
const original = "/a/random/Path/defs%3F1.json";
const encoded = $url.fromFileSystemPath(original);
expect(encoded).to.equal("/a/random/Path/defs%253F1.json");
expect($url.toFileSystemPath(encoded)).to.equal(original);
// And the two filenames must remain distinguishable -- this is the actual bug:
// parsing the encoded literal-%3F path must NOT collapse onto the "?" sibling.
const questionMarkOriginal = "/a/random/Path/defs?1.json";
expect($url.toFileSystemPath(encoded)).to.not.equal(questionMarkOriginal);
});

it("should decode a lowercase %3f the same as uppercase %3F (RFC 3986 §2.1: hex digits are case-insensitive)", () => {
const original = "/a/random/Path/defs?1.json";
expect($url.toFileSystemPath("/a/random/Path/defs%3f1.json")).to.equal(original);
expect($url.toFileSystemPath("/a/random/Path/defs%3F1.json")).to.equal(original);
});
});