diff --git a/lib/util/url.ts b/lib/util/url.ts index f13966eb..1c550307 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"]; @@ -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 9563c059..68c8d18f 100644 --- a/test/specs/util/url.spec.ts +++ b/test/specs/util/url.spec.ts @@ -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); + }); +});