From 80592b82a6f854886862978533bb405e3f2e38c5 Mon Sep 17 00:00:00 2001 From: Shivek <152221558+ShivekRanjan@users.noreply.github.com> Date: Wed, 14 Oct 2026 04:20:29 +0530 Subject: [PATCH] fix(javascript): keep CommonJS percent bytes literal (#3001) Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- .../JavaScriptArtifactPathResolution.ts | 25 +++++-- .../javascriptRequireLiteralPaths.test.ts | 67 ++++++++++++++++++- 2 files changed, 86 insertions(+), 6 deletions(-) diff --git a/src/application/javascript/JavaScriptArtifactPathResolution.ts b/src/application/javascript/JavaScriptArtifactPathResolution.ts index f56623f2e6..b51eeebda4 100644 --- a/src/application/javascript/JavaScriptArtifactPathResolution.ts +++ b/src/application/javascript/JavaScriptArtifactPathResolution.ts @@ -378,6 +378,7 @@ const resolveRuntimeSourceCandidates = ( const rejectDeclaration = ( declared: string, + literalPercent = false, allowEmptyPath = false, ): CandidateResolution | null => { if (declared.length === 0 && !allowEmptyPath) @@ -386,13 +387,23 @@ const rejectDeclaration = ( return unresolvedCandidate("rejected", [ "NUL and backslash path syntax are not admitted for canonical artifact paths.", ]); - if (!admitsCanonicalPathSyntax(declared)) + if (!literalPercent && !admitsCanonicalPathSyntax(declared)) return unresolvedCandidate("rejected", [ "Encoded dot or separator bytes are rejected before artifact path resolution.", ]); return null; }; +/** + * Node's CommonJS loader never percent-decodes a request or a legacy package + * main, so `%2e` and `%2f` there are literal filename bytes, not encoded dots + * or separators. Root confinement still applies to the joined candidate. + */ +const declaresLiteralPercent = (input: ResolveArtifactPathInput): boolean => + input.moduleKind === "require" && + (input.context === "module-specifier" || + input.context === "package-entrypoint"); + const contextualCandidate = ( input: ResolveArtifactPathInput, ): string | CandidateResolution => { @@ -428,7 +439,7 @@ const contextualCandidate = ( "File URL authorities or invalid file URL forms cannot be mapped to artifact-local paths.", ]); } - const rejected = rejectDeclaration(declared); + const rejected = rejectDeclaration(declared, declaresLiteralPercent(input)); if (rejected !== null) return rejected; if (context === "module-specifier") { if (hasScheme(declared)) @@ -831,7 +842,8 @@ const resolveExportTarget = ( const encodedPath = target.url.pathname.slice( PACKAGE_ARTIFACT_ROOT_URL.pathname.length, ); - const rejected = rejectDeclaration(encodedPath); + // Package targets are URLs for both loaders, so their percent bytes encode. + const rejected = rejectDeclaration(encodedPath, false); if (rejected !== null) return withTargetSource(rejected); const decoded = decodeModuleUrlPath(encodedPath); if (typeof decoded !== "string") return withTargetSource(decoded); @@ -912,7 +924,12 @@ const htmlCandidate = ( const declared = stripQueryAndFragment(htmlUrlText(input.declaredPath)); // HTML owns URL-text normalization and pathname admission. A nonempty URL // reference may have no pathname; suffix bytes remain declaration evidence. - const rejected = rejectDeclaration(declared, input.declaredPath !== ""); + // HTML references are URL text, so percent bytes still encode. + const rejected = rejectDeclaration( + declared, + false, + input.declaredPath !== "", + ); if (rejected !== null) return rejected; if (looksExternal(declared)) return unresolvedCandidate("external", [ diff --git a/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts b/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts index 65349b8e4f..d5495a7f25 100644 --- a/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts +++ b/tests/boundary/filesystem/javascriptRequireLiteralPaths.test.ts @@ -1,6 +1,6 @@ -import { writeFile } from "node:fs/promises"; +import { mkdir, writeFile } from "node:fs/promises"; import { createRequire } from "node:module"; -import { join, relative, sep } from "node:path"; +import { dirname, join, relative, sep } from "node:path"; import { describe, expect, it } from "vitest"; @@ -133,3 +133,66 @@ describe("literal CommonJS paths in application graphs", () => { }, ); }); + +describe("literal CommonJS percent bytes in application graphs", () => { + it("agrees with Node for requests whose literal names contain %2e or %2F", async () => { + const root = await createTestTempDirectory("rea-literal-percent-require-"); + const files = { + "app/lit%2eb.js": "module.exports = 'lit%2eb.js';", + "app/x%2Fy.js": "module.exports = 'x%2Fy.js';", + "app/sub/%2e%2e/x.js": "module.exports = 'sub/%2e%2e/x.js';", + "app/x.js": "module.exports = 'decoy for a decoded parent';", + "app/pkg/package.json": JSON.stringify({ main: "lit%2eb" }), + "app/pkg/lit%2eb.js": "module.exports = 'pkg/lit%2eb.js';", + "app/node_modules/dep/package.json": JSON.stringify({ name: "dep" }), + "app/node_modules/dep/lit%2eb.js": "module.exports = 'dep/lit%2eb.js';", + "escape.js": "module.exports = 'decoy outside the request directory';", + }; + const specifiers = [ + "./lit%2eb", + "./x%2Fy", + "./sub/%2e%2e/x", + "./pkg", + "dep/lit%2eb", + ]; + // CommonJS reads %2e%2e as a directory name, never as a parent segment. + const literalSegment = "./%2e%2e/escape"; + for (const [path, text] of Object.entries(files)) { + await mkdir(dirname(join(root, path)), { recursive: true }); + await writeFile(join(root, path), text); + } + const main = join(root, "app", "main.cjs"); + await writeFile( + main, + [...specifiers, literalSegment] + .map((specifier) => `require(${JSON.stringify(specifier)});`) + .join("\n"), + ); + const nodeRequire = createRequire(main); + const nodePath = (specifier: string) => + relative(root, nodeRequire.resolve(specifier)).split(sep).join("/"); + expect(() => nodeRequire.resolve(literalSegment)).toThrow( + /Cannot find module/u, + ); + + const { graph } = await analyze(root); + const requests = (specifier: string) => + graph.edges.filter( + (edge) => + edge.relation === "imports" && + edge.properties.specifier === specifier, + ); + for (const specifier of specifiers) { + const edges = requests(specifier); + expect(edges.length, specifier).toBeGreaterThan(0); + for (const edge of edges) + expect(edge.properties.resolved_path, specifier).toBe( + nodePath(specifier), + ); + } + const unresolved = requests(literalSegment); + expect(unresolved.length).toBeGreaterThan(0); + for (const edge of unresolved) + expect(edge.properties.resolved_path).toBeNull(); + }); +});