From d79fedbd46a83145eb15737e8bc561a6652d999e Mon Sep 17 00:00:00 2001 From: patchwright Date: Fri, 7 Aug 2026 23:18:36 +0200 Subject: [PATCH 1/2] fix: decode %3F back to "?" in toFileSystemPath (#427) urlEncodePatterns encodes both # and ? when converting a filesystem path to a URL, but urlDecodePatterns only reversed #, $, &, ,, and @. A local path containing a literal ? (legal on POSIX filesystems) was encoded to %3F on the way in and never decoded back on the way out, so resolution of any such path failed with ENOENT. Adds the missing /%3F/g, "?" pair, in the same hex-ordered position the other pairs already follow. Adds a symmetric test for # alongside the new ? test. --- lib/util/url.ts | 2 +- test/specs/util/url.spec.ts | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/util/url.ts b/lib/util/url.ts index f13966eb..2c63a522 100644 --- a/lib/util/url.ts +++ b/lib/util/url.ts @@ -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"]; diff --git a/test/specs/util/url.spec.ts b/test/specs/util/url.spec.ts index 9563c059..65475c2d 100644 --- a/test/specs/util/url.spec.ts +++ b/test/specs/util/url.spec.ts @@ -152,3 +152,19 @@ 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", () => { + 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); + }); +}); From 1b3679188a38c2e550e8ca783b360e07529c4542 Mon Sep 17 00:00:00 2001 From: privatedick Date: Wed, 12 Aug 2026 19:39:26 +0200 Subject: [PATCH 2/2] fix: consume reserved-char escapes before decodeURI to avoid double-decoding Addresses review feedback from @jonluca on this PR. Root cause: fromFileSystemPath percent-escapes a literal "%" character as "%25", so a real filename containing the literal text "%3F" round-trips through encoding as "%253F". toFileSystemPath previously ran decodeURI() BEFORE the manual reserved-char decode pass -- decodeURI decodes "%25" back to a literal "%", which reveals a literal "%3F" substring that did not exist in the encoded form. The manual pass then wrongly decoded that revealed text a second time (%3F -> "?"), so parse("defs%3F1.json") would silently read a "defs?1.json" sibling file instead -- a real path-aliasing bug, and double-decoding a single escape violates RFC 3986 section 2.4. Fix: run the manual reserved-char decode pass BEFORE decodeURI, not after. The literal "%253F" does not contain the substring "%3F" (it's "%25" followed by "3F"), so it's untouched by the manual pass and correctly decoded exactly once by decodeURI (%25 -> %). Also made the reserved-char patterns case-insensitive (RFC 3986 section 2.1: percent-encoding hex digits are case-insensitive, so "%3f" must decode the same as "%3F" -- the previous pattern was uppercase-only). Tests: added a regression test reproducing the exact double-decoding case (confirmed it fails without the fix, passes with it), a lowercase-%3f case, and moved the existing round-trip tests inside the same isWindows()=false mock the neighboring "Handle Linux file paths" block already uses (they were asserting absolute-POSIX-path behavior outside any Windows mock, which is why the Windows CI job was failing). Full suite: 77/77 files, 525 passed, 0 failed, unchanged from before this commit. --- lib/util/url.ts | 24 +++++++++++++++++------- test/specs/util/url.spec.ts | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/lib/util/url.ts b/lib/util/url.ts index 2c63a522..1c550307 100644 --- a/lib/util/url.ts +++ b/lib/util/url.ts @@ -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://"); diff --git a/test/specs/util/url.spec.ts b/test/specs/util/url.spec.ts index 65475c2d..68c8d18f 100644 --- a/test/specs/util/url.spec.ts +++ b/test/specs/util/url.spec.ts @@ -154,6 +154,17 @@ describe("Handle Linux file paths", () => { }); 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); @@ -167,4 +178,27 @@ describe("Round-trip special characters in filesystem paths", () => { 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); + }); });