From 90c69229050cd251f86294a6f3ff5e22abf6912f Mon Sep 17 00:00:00 2001 From: Kaoru0822 Date: Wed, 14 Oct 2026 19:12:49 -0700 Subject: [PATCH] fix(javascript): keep empty URL path segments significant (#3792) * fix(javascript): keep empty URL path segments significant ESM imports and HTML script references were joined with POSIX path rules, which collapse '//' before applying '..'. URL resolution removes the empty segment instead, so './assets//../app.js' names assets/app.js, not app.js, and the graph linked the importer and renderer to the wrong file. URL references refused every '//' even when dot segments cancel it. Resolve ESM, HTML (including document base) and URL references through one URL dot-segment helper. A remaining empty segment stays unavailable, references above the artifact root are still rejected, and CommonJS keeps filesystem path semantics. Fixes #3307 Co-Authored-By: Claude Opus 5.5 * fix(javascript): resolve HTML base and reference as one URL path The base href was turned into a canonical path before the script reference was applied. A base empty segment that the reference's '..' would remove was rejected early, and an above-root '..' from the base became a removable segment that the reference's '..' could cancel. Keep one URL path state through base and reference resolution, with above-root escapes held apart from removable segments, and reject empty segments only in the final path. Remove the superseded htmlBaseDirectory. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- docs/javascript-artifact-reconstruction.md | 4 + .../JavaScriptArtifactPathResolution.test.ts | 51 ++++++- .../JavaScriptArtifactPathResolution.ts | 129 ++++++++++++++---- .../JavaScriptHtmlBaseResolution.test.ts | 21 +++ 4 files changed, 176 insertions(+), 29 deletions(-) diff --git a/docs/javascript-artifact-reconstruction.md b/docs/javascript-artifact-reconstruction.md index 18175ef8c9..07548202e9 100644 --- a/docs/javascript-artifact-reconstruction.md +++ b/docs/javascript-artifact-reconstruction.md @@ -629,6 +629,10 @@ href are read without surrounding whitespace or embedded tabs and newlines, and their percent-encoded path bytes are decoded; encoded dot and separator bytes are still rejected. CommonJS module lookups retain extension and directory resolution. +ESM, HTML, and URL references apply URL dot segments to empty path segments: +`./assets//../app.js` selects `assets/app.js`, while a remaining empty segment +such as `./assets//app.js` stays unavailable. CommonJS keeps filesystem path +semantics and selects `app.js` for the same text. Unresolved HTML references retain their declaration, source range, and resolution reason in the renderer observations. HTML script source ranges follow the HTML parser across LF, CRLF, and bare CR diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts index 77a448b4a3..dc0ccf5e61 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.test.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.test.ts @@ -2495,20 +2495,61 @@ describe("url-reference directory and empty-segment semantics", () => { }); }); - it.each(["./nested//worker.js", "./nested//../worker.js"])( - "does not collapse empty URL path segments: %s", - (declaredPath) => { + it.each([ + ["./nested//worker.js", null], + ["./nested//../worker.js", "dist/nested/worker.js"], + ])( + "keeps empty URL path segments significant: %s", + (declaredPath, resolvedPath) => { + // A dot segment removes the preceding empty segment, never `nested`. + expect(new URL(declaredPath, "file:///root/dist/app.mjs").pathname).toBe( + resolvedPath === null + ? "/root/dist/nested//worker.js" + : `/root/${resolvedPath}`, + ); + for (const context of ["url-reference", "html-reference"] as const) + expect( + resolve({ + declaredPath, + sourcePath: "dist/app.mjs", + context, + files: fileMap([ + file("dist/worker.js", "root"), + file("dist/nested/worker.js", "root"), + ]), + }), + context, + ).toMatchObject({ resolved_path: resolvedPath }); expect( resolve({ declaredPath, sourcePath: "dist/app.mjs", - context: "url-reference", + context: "module-specifier", + moduleKind: "import", files: fileMap([ file("dist/worker.js", "root"), file("dist/nested/worker.js", "root"), ]), }), - ).toMatchObject({ resolved_path: null }); + ).toMatchObject({ resolved_path: resolvedPath }); + // CommonJS keeps filesystem path semantics. + expect( + resolve({ + declaredPath, + sourcePath: "dist/app.mjs", + context: "module-specifier", + moduleKind: "require", + files: fileMap([ + file("dist/worker.js", "root"), + file("dist/nested/worker.js", "root"), + ]), + }), + ).toMatchObject({ + resolved_path: + declaredPath === "./nested//worker.js" + ? "dist/nested/worker.js" + : "dist/worker.js", + }); }, ); diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index 7a3e4c89e0..94ffbc7725 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -551,22 +551,24 @@ const contextualCandidate = ( return unresolvedCandidate("rejected", [ "The URL reference path percent-decodes to NUL or to bytes that are not UTF-8.", ]); - // Empty URL segments are significant; canonical filesystem paths cannot - // preserve them, including their interaction with following dot segments. - if (decoded.includes("//")) - return unresolvedCandidate("unavailable", [ - "Repeated URL path separators cannot be mapped to an exact canonical artifact file.", - ]); - return exactUrlPathCandidate( + const candidate = urlRelativeCandidate( + posix.dirname(input.sourcePath), decoded, - relativeCandidate(input.sourcePath, decoded), - "URL", ); + return typeof candidate === "string" + ? exactUrlPathCandidate(decoded, candidate, "URL") + : candidate; } - const candidate = relativeCandidate(input.sourcePath, declared); - return context === "module-specifier" && input.moduleKind === "import" - ? exactUrlPathCandidate(declared, candidate, "module URL") - : candidate; + if (context === "module-specifier" && input.moduleKind === "import") { + const candidate = urlRelativeCandidate( + posix.dirname(input.sourcePath), + declared, + ); + return typeof candidate === "string" + ? exactUrlPathCandidate(declared, candidate, "module URL") + : candidate; + } + return relativeCandidate(input.sourcePath, declared); }; const decodeModuleUrlPath = ( @@ -593,6 +595,78 @@ const relativeCandidate = (sourcePath: string, declared: string): string => ? declared.slice(1) : posix.join(posix.dirname(sourcePath), declared); +/** + * A URL path during reference resolution. Above-root `..` markers are kept + * apart from segments, so a later `..` cannot cancel an escape, while empty + * segments stay removable until the final path is formed. + */ +interface UrlPathState { + readonly escapes: number; + readonly segments: readonly string[]; +} + +const urlDirectoryState = (directory: string): UrlPathState => ({ + escapes: 0, + segments: directory === "" || directory === "." ? [] : directory.split("/"), +}); + +/** + * Apply URL dot segments: `..` removes the previous segment even when it is + * empty, so `assets//../app.js` names `assets/app.js`. A final `.` or `..` + * leaves directory form, as the URL parser does. + */ +const applyUrlReference = ( + base: UrlPathState, + reference: string, +): UrlPathState => { + const absolute = reference.startsWith("/"); + const segments = absolute ? [] : [...base.segments]; + let escapes = absolute ? 0 : base.escapes; + const parts = (absolute ? reference.slice(1) : reference).split("/"); + for (const [index, part] of parts.entries()) { + if (part !== "." && part !== "..") { + segments.push(part); + continue; + } + if (part === "..") + if (segments.length > 0) segments.pop(); + else escapes += 1; + if (index === parts.length - 1) segments.push(""); + } + return { escapes, segments }; +}; + +/** A relative reference replaces the base URL's final path segment. */ +const urlBaseDirectory = (base: UrlPathState): UrlPathState => ({ + escapes: base.escapes, + segments: base.segments.slice(0, -1), +}); + +/** + * Form the artifact candidate. A remaining empty segment cannot name a + * canonical artifact file; a trailing one is directory form. Confinement + * rejects a candidate that keeps an above-root marker. + */ +const urlPathCandidate = (path: UrlPathState): string | CandidateResolution => { + const segments = + path.segments.at(-1) === "" ? path.segments.slice(0, -1) : path.segments; + if (segments.includes("")) + return unresolvedCandidate("unavailable", [ + "Repeated URL path separators cannot be mapped to an exact canonical artifact file.", + ]); + return [ + ...Array.from({ length: path.escapes }, () => ".."), + ...segments, + ].join("/"); +}; + +/** Resolve a URL path reference against a canonical directory. */ +const urlRelativeCandidate = ( + directory: string, + reference: string, +): string | CandidateResolution => + urlPathCandidate(applyUrlReference(urlDirectoryState(directory), reference)); + const bareModuleCandidate = ( input: ResolveArtifactPathInput, ): CandidateResolution => { @@ -1066,11 +1140,12 @@ const htmlCandidate = ( return unresolvedCandidate("rejected", [ "The HTML reference path percent-decodes to NUL or to bytes that are not UTF-8.", ]); - if (declaredPath.startsWith("/")) return declaredPath.slice(1); + if (declaredPath.startsWith("/")) + return urlRelativeCandidate("", declaredPath); if (base === undefined || base === null || base === "") return declaredPath === "" ? input.sourcePath - : posix.join(posix.dirname(input.sourcePath), declaredPath); + : urlRelativeCandidate(posix.dirname(input.sourcePath), declaredPath); // A local base href is a second untrusted path input; apply the same // admission rules a declared path gets so it cannot smuggle traversal or // separator syntax past canonicalization. @@ -1083,12 +1158,21 @@ const htmlCandidate = ( return unresolvedCandidate("rejected", [ "The document base href path percent-decodes to NUL or to bytes that are not UTF-8.", ]); - const basePath = decodedBase.startsWith("/") - ? decodedBase.slice(1) - : posix.join(posix.dirname(input.sourcePath), decodedBase); - if (declaredPath === "") - return exactUrlPathCandidate(decodedBase, basePath, "HTML"); - return posix.join(htmlBaseDirectory(decodedBase, basePath), declaredPath); + // Keep the base's empty segments and escapes until the reference is + // applied; only the final path must name a canonical artifact file. + const baseState = applyUrlReference( + urlDirectoryState(posix.dirname(input.sourcePath)), + decodedBase, + ); + if (declaredPath === "") { + const basePath = urlPathCandidate(baseState); + return typeof basePath === "string" + ? exactUrlPathCandidate(decodedBase, basePath, "HTML") + : basePath; + } + return urlPathCandidate( + applyUrlReference(urlBaseDirectory(baseState), declaredPath), + ); }; const exactUrlPathCandidate = ( @@ -1112,9 +1196,6 @@ const urlPathIsDirectory = (path: string): boolean => path === "." || path === ".."; -const htmlBaseDirectory = (base: string, basePath: string): string => - urlPathIsDirectory(base) ? basePath : posix.dirname(basePath); - const confineCandidate = (candidate: string): string | CandidateResolution => { const normalized = normalizeJoinedLogicalPath(candidate); if (logicalPathEscapesRoot(normalized)) diff --git a/src/application/javascript/JavaScriptHtmlBaseResolution.test.ts b/src/application/javascript/JavaScriptHtmlBaseResolution.test.ts index 1da6539a03..1a4bbd8503 100644 --- a/src/application/javascript/JavaScriptHtmlBaseResolution.test.ts +++ b/src/application/javascript/JavaScriptHtmlBaseResolution.test.ts @@ -6,6 +6,7 @@ const paths = [ "assets/app.js", "renderer/app.js", "renderer/a/app.js", + "renderer/assets/app.js", "app.js", ]; const files = new Map( @@ -105,6 +106,26 @@ describe("HTML base href dot segments", () => { }, ); }); +describe("HTML base href empty segments", () => { + it("lets the script reference remove an empty segment from the base", () => { + // URL resolution keeps the base's empty segment until `..` removes it. + expect( + new URL("../app.js", new URL("./assets//", documentUrl)).pathname, + ).toBe("/renderer/assets/app.js"); + expect(resolve("../app.js", "./assets//")).toMatchObject({ + resolution_status: "resolved", + resolved_path: "renderer/assets/app.js", + }); + }); + it("keeps an above-root base escape when the reference also climbs", () => { + expect(resolve("../app.js", "../../")).toMatchObject({ + resolution_status: "rejected", + limitations: [ + "The resolved candidate escapes the canonical artifact root.", + ], + }); + }); +}); describe("HTML base href path syntax", () => { it.each(["%2e%2e/secret/", "/a/%2e%2e/", "/a\\b/", "a\0b"])( "rejects base href %j with the same admission rules a declared path gets",