From 61a7cbc1c5eac7166e391cf7da20bb18960ee76b Mon Sep 17 00:00:00 2001 From: Erik Date: Wed, 8 Jul 2026 23:49:26 +0600 Subject: [PATCH] install: use uppercase %2F for scoped package manifest URLs (Rust) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Match the npm CLI and RFC 3986 §2.1 canonical form. GitLab's npm registry returns 404 for lowercase %2f but accepts %2F. AWS CodeArtifact and Azure Artifacts accept both. Port of the fix from #30312 to the Rust codebase: - src/install/NetworkTask.rs (bun install manifest fetch) - src/bun_core/fmt.rs (DependencyUrlFormatter for bun pm view/publish) Adds regression tests and updates install test assertions. Fixes #30311 Fixes #26241 --- src/bun_core/fmt.rs | 2 +- src/install/NetworkTask.rs | 2 +- test/cli/install/bun-add.test.ts | 4 +- test/cli/install/bun-create.test.ts | 6 +- test/cli/install/bun-install-registry.test.ts | 147 ++++++++++++++++++ .../bun-install-security-provider.test.ts | 2 +- .../bun-install-tarball-integrity.test.ts | 2 +- test/cli/install/bun-install.test.ts | 6 +- test/cli/install/bun-update.test.ts | 4 +- test/cli/install/bunx.test.ts | 2 +- test/cli/install/dummy.registry.ts | 6 +- 11 files changed, 165 insertions(+), 18 deletions(-) diff --git a/src/bun_core/fmt.rs b/src/bun_core/fmt.rs index 4e3338afb90f..c9e92878c346 100644 --- a/src/bun_core/fmt.rs +++ b/src/bun_core/fmt.rs @@ -327,7 +327,7 @@ impl Display for DependencyUrlFormatter<'_> { let mut remain = self.url; while let Some(slash) = crate::strings::index_of_char_usize(remain, b'/') { write_bytes(f, &remain[..slash])?; - f.write_str("%2f")?; + f.write_str("%2F")?; remain = &remain[slash + 1..]; } write_bytes(f, remain) diff --git a/src/install/NetworkTask.rs b/src/install/NetworkTask.rs index e89078efa2c4..5ec584800a65 100644 --- a/src/install/NetworkTask.rs +++ b/src/install/NetworkTask.rs @@ -425,7 +425,7 @@ impl NetworkTask { // "npm" CLI requests the manifest with the encoded name. let encoded_name_storage; let encoded_name: &[u8] = if strings::index_of_char(name, b'/').is_some() { - encoded_name_storage = name.replace(b"/", b"%2f"); + encoded_name_storage = name.replace(b"/", b"%2F"); &encoded_name_storage } else { name diff --git a/test/cli/install/bun-add.test.ts b/test/cli/install/bun-add.test.ts index 2a0f346eb12b..102419e5d56c 100644 --- a/test/cli/install/bun-add.test.ts +++ b/test/cli/install/bun-add.test.ts @@ -466,10 +466,10 @@ it("should handle @scoped names", async () => { env, }); const err = await stderr.text(); - expect(err.split(/\r?\n/)).toContain(`error: GET http://localhost:${port}/@bar%2fbaz - 404`); + expect(err.split(/\r?\n/)).toContain(`error: GET http://localhost:${port}/@bar%2Fbaz - 404`); expect(await stdout.text()).toEqual(expect.stringContaining("bun add v1.")); expect(await exited).toBe(1); - expect(urls.sort()).toEqual([`${root_url}/@bar%2fbaz`]); + expect(urls.sort()).toEqual([`${root_url}/@bar%2Fbaz`]); expect(requested).toBe(1); try { await access(join(package_dir, "bun.lockb")); diff --git a/test/cli/install/bun-create.test.ts b/test/cli/install/bun-create.test.ts index d939581968f5..55e51ee32e34 100644 --- a/test/cli/install/bun-create.test.ts +++ b/test/cli/install/bun-create.test.ts @@ -48,7 +48,7 @@ it("should create selected template with @ prefix", async () => { const err = await stderr.text(); expect(err.split(/\r?\n/)).toContain( - `error: GET https://registry.npmjs.org/@quick-start%2fcreate-some-template - 404`, + `error: GET https://registry.npmjs.org/@quick-start%2Fcreate-some-template - 404`, ); }); @@ -63,7 +63,7 @@ it("should create selected template with @ prefix implicit `/create`", async () }); const err = await stderr.text(); - expect(err.split(/\r?\n/)).toContain(`error: GET https://registry.npmjs.org/@second-quick-start%2fcreate - 404`); + expect(err.split(/\r?\n/)).toContain(`error: GET https://registry.npmjs.org/@second-quick-start%2Fcreate - 404`); await exited; }); @@ -78,7 +78,7 @@ it("should create selected template with @ prefix implicit `/create` with versio }); const err = await stderr.text(); - expect(err.split(/\r?\n/)).toContain(`error: GET https://registry.npmjs.org/@second-quick-start%2fcreate - 404`); + expect(err.split(/\r?\n/)).toContain(`error: GET https://registry.npmjs.org/@second-quick-start%2Fcreate - 404`); await exited; }); diff --git a/test/cli/install/bun-install-registry.test.ts b/test/cli/install/bun-install-registry.test.ts index 5bf0cca96d9e..19d79bba3237 100644 --- a/test/cli/install/bun-install-registry.test.ts +++ b/test/cli/install/bun-install-registry.test.ts @@ -4935,6 +4935,153 @@ test("name from manifest is scoped and url encoded", async () => { ]); }); +// Bun was encoding the '/' in scoped package manifest URLs as lowercase '%2f'. +// GitLab's npm registry (and some others) return 404 for the lowercase form. +// The npm CLI uses uppercase '%2F' per RFC 3986 §2.1, so we match that. +test("scoped package manifest url uses uppercase %2F", async () => { + const manifestPaths: string[] = []; + using server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url); + // Record the raw (not normalized) path so the test can see the exact + // bytes bun put on the wire. URL constructor preserves %2F/%2f casing. + manifestPaths.push(url.pathname); + + // Simulate GitLab: reject lowercase %2f, accept uppercase %2F or + // an unencoded slash. This is the exact discriminator from the bug. + if (url.pathname.includes("%2f")) { + return new Response("not found", { status: 404 }); + } + + // Manifest for @scoped/has-bin-entry. + if (url.pathname.endsWith("@scoped%2Fhas-bin-entry") || url.pathname.endsWith("@scoped/has-bin-entry")) { + return Response.json({ + name: "@scoped/has-bin-entry", + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { + name: "@scoped/has-bin-entry", + version: "1.0.0", + dist: { + shasum: "611b2b566718e05e8643fe872923ed91342b039a", + tarball: `http://localhost:${server.port}/@scoped/has-bin-entry/-/has-bin-entry-1.0.0.tgz`, + }, + }, + }, + }); + } + + // Tarball download. + if (url.pathname.endsWith("/has-bin-entry-1.0.0.tgz")) { + return new Response( + Bun.file( + join(import.meta.dir, "registry", "packages", "@scoped", "has-bin-entry", "has-bin-entry-1.0.0.tgz"), + ), + ); + } + + return new Response("not found", { status: 404 }); + }, + }); + + await Promise.all([ + write( + packageJson, + JSON.stringify({ + name: "foo", + dependencies: { + "@scoped/has-bin-entry": "1.0.0", + }, + }), + ), + write(join(packageDir, ".npmrc"), `@scoped:registry=http://localhost:${server.port}/\n`), + ]); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install"], + cwd: packageDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + + // Assertion on the raw URL bun produced: every manifest request must use + // uppercase %2F and never lowercase %2f. Filter to manifest paths (they end + // with the package name, tarballs end with .tgz). + const manifestHits = manifestPaths.filter( + p => + p.endsWith("@scoped/has-bin-entry") || + p.endsWith("@scoped%2Fhas-bin-entry") || + p.endsWith("@scoped%2fhas-bin-entry"), + ); + expect(manifestHits.length).toBeGreaterThan(0); + for (const hit of manifestHits) { + expect(hit).not.toContain("%2f"); + expect(hit).toContain("%2F"); + } + + expect(err).not.toContain("error:"); + expect(err).not.toContain("404"); + expect(out).toContain("+ @scoped/has-bin-entry@1.0.0"); + expect(code).toBe(0); +}); + +// Companion to the regression test above: `bun pm view` goes through +// `DependencyUrlFormatter` (src/bun_core/fmt.rs), not the install manifest +// path, and that encoder had the same lowercase-%2f bug. +test("bun pm view uses uppercase %2F for scoped names", async () => { + const manifestPaths: string[] = []; + using server = Bun.serve({ + port: 0, + async fetch(req) { + const url = new URL(req.url); + manifestPaths.push(url.pathname); + // Reject lowercase like GitLab. + if (url.pathname.includes("%2f")) { + return new Response("not found", { status: 404 }); + } + if (url.pathname.endsWith("@scoped%2Fhas-bin-entry")) { + return Response.json({ + name: "@scoped/has-bin-entry", + "dist-tags": { latest: "1.0.0" }, + versions: { + "1.0.0": { name: "@scoped/has-bin-entry", version: "1.0.0" }, + }, + }); + } + return new Response("not found", { status: 404 }); + }, + }); + + // pm view needs a clean project dir without bunfig.toml overriding our + // scoped registry; use a fresh tempDir rather than the shared packageDir. + const viewDir = tempDirWithFiles("pm-view-scoped", { + "package.json": JSON.stringify({ name: "x", version: "0.0.1" }), + ".npmrc": `@scoped:registry=http://localhost:${server.port}/\n`, + }); + + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "pm", "view", "@scoped/has-bin-entry"], + cwd: viewDir, + stdout: "pipe", + stderr: "pipe", + env, + }); + const [out, err, code] = await Promise.all([stdout.text(), stderr.text(), exited]); + + const hits = manifestPaths.filter(p => p.includes("has-bin-entry")); + expect(hits.length).toBeGreaterThan(0); + for (const hit of hits) { + expect(hit).not.toContain("%2f"); + expect(hit).toContain("%2F"); + } + expect(err).not.toContain("error:"); + expect(out).toContain("@scoped/has-bin-entry@1.0.0"); + expect(code).toBe(0); +}); + describe("update", () => { test("duplicate peer dependency (one package is invalid_package_id)", async () => { await write( diff --git a/test/cli/install/bun-install-security-provider.test.ts b/test/cli/install/bun-install-security-provider.test.ts index 81e00a3c420b..20a3f28d4f08 100644 --- a/test/cli/install/bun-install-security-provider.test.ts +++ b/test/cli/install/bun-install-security-provider.test.ts @@ -750,7 +750,7 @@ describe("Large payload via ipc pipe", () => { customRegistry: (urls, ctx) => { return async (request: Request) => { urls.push(request.url); - const url = request.url.replaceAll("%2f", "/"); + const url = request.url.replaceAll("%2f", "/").replaceAll("%2F", "/"); expect(request.method).toBe("GET"); if (url.endsWith(".tgz")) { return new Response(barTarballBytes); diff --git a/test/cli/install/bun-install-tarball-integrity.test.ts b/test/cli/install/bun-install-tarball-integrity.test.ts index 98bc78c8e5a3..fdb05038123b 100644 --- a/test/cli/install/bun-install-tarball-integrity.test.ts +++ b/test/cli/install/bun-install-tarball-integrity.test.ts @@ -757,7 +757,7 @@ describe.concurrent.each(["hoisted", "isolated"] as const)("tarball download fai const urls: string[] = []; let tarballStatus = 200; setContextHandler(ctx, async request => { - const url = request.url.replaceAll("%2f", "/"); + const url = request.url.replaceAll("%2f", "/").replaceAll("%2F", "/"); urls.push(url); if (url.endsWith(".tgz")) { if (tarballStatus !== 200) { diff --git a/test/cli/install/bun-install.test.ts b/test/cli/install/bun-install.test.ts index 51f91d596712..ed07a3bce91f 100644 --- a/test/cli/install/bun-install.test.ts +++ b/test/cli/install/bun-install.test.ts @@ -645,7 +645,7 @@ describe.concurrent("bun-install", () => { it("should handle @scoped authentication", async () => { await withContext(defaultOpts, async ctx => { let seen_token = false; - const url = `${ctx.registry_url}@foo%2fbar`; + const url = `${ctx.registry_url}@foo%2Fbar`; const urls: string[] = []; setContextHandler(ctx, async request => { expect(request.method).toBe("GET"); @@ -3310,7 +3310,7 @@ describe.concurrent("bun-install", () => { "1 package installed", ]); expect(await exited).toBe(0); - expect(urls.sort()).toEqual([`${ctx.registry_url}@barn%2fmoo`, `${ctx.registry_url}@barn/moo-0.1.0.tgz`]); + expect(urls.sort()).toEqual([`${ctx.registry_url}@barn%2Fmoo`, `${ctx.registry_url}@barn/moo-0.1.0.tgz`]); expect(ctx.requested).toBe(2); expect(await readdirSorted(join(ctx.package_dir, "node_modules"))).toEqual([".cache", "@barn", "moo"]); expect(await readdirSorted(join(ctx.package_dir, "node_modules", "@barn"))).toEqual(["moo"]); @@ -3445,7 +3445,7 @@ describe.concurrent("bun-install", () => { ]); expect(await exited1).toBe(0); expect(urls.sort()).toEqual([ - `${ctx.registry_url}@barn%2fmoo`, + `${ctx.registry_url}@barn%2Fmoo`, `${ctx.registry_url}@barn/moo-0.1.0.tgz`, `${ctx.registry_url}bar`, `${ctx.registry_url}bar-0.0.2.tgz`, diff --git a/test/cli/install/bun-update.test.ts b/test/cli/install/bun-update.test.ts index 7859e7f56388..c100320baf9f 100644 --- a/test/cli/install/bun-update.test.ts +++ b/test/cli/install/bun-update.test.ts @@ -201,7 +201,7 @@ for (const { input } of [{ input: { baz: "~0.0.3", moo: "~0.1.0" } }]) { ]); expect(await exited1).toBe(0); expect(urls.sort()).toEqual([ - `${root_url}/@barn%2fmoo`, + `${root_url}/@barn%2Fmoo`, `${root_url}/@barn/moo-0.1.0.tgz`, `${root_url}/baz`, `${root_url}/baz-0.0.3.tgz`, @@ -264,7 +264,7 @@ for (const { input } of [{ input: { baz: "~0.0.3", moo: "~0.1.0" } }]) { } expect(await exited2).toBe(0); expect(urls.sort()).toEqual([ - `${root_url}/@barn%2fmoo`, + `${root_url}/@barn%2Fmoo`, `${root_url}/@barn/moo-0.1.0.tgz`, `${root_url}/baz`, tilde ? `${root_url}/baz-0.0.5.tgz` : `${root_url}/baz-0.0.3.tgz`, diff --git a/test/cli/install/bunx.test.ts b/test/cli/install/bunx.test.ts index cb99066c38c7..c1a339c7259e 100644 --- a/test/cli/install/bunx.test.ts +++ b/test/cli/install/bunx.test.ts @@ -1149,7 +1149,7 @@ describe("package name aliases", () => { const paths = urls.map(u => new URL(u).pathname); // The manifest request must be for the real package, and must never hit // the squatter package name. - expect(paths).toContain("/@anthropic-ai%2fclaude-code"); + expect(paths).toContain("/@anthropic-ai%2Fclaude-code"); expect(paths).not.toContain("/claude"); // Install fails because the mock registry 404s; that's fine, we only care // about which manifest was requested. diff --git a/test/cli/install/dummy.registry.ts b/test/cli/install/dummy.registry.ts index 63798c7e2027..7b6668a91358 100644 --- a/test/cli/install/dummy.registry.ts +++ b/test/cli/install/dummy.registry.ts @@ -82,7 +82,7 @@ function defaultHandler(): Response { /** * Extract the test ID prefix from a URL path. - * URL format: /test-123/package-name or /test-123/@scope%2fpackage-name + * URL format: /test-123/package-name or /test-123/@scope%2Fpackage-name */ function extractTestPrefix(url: string): { prefix: string; remainingPath: string } | null { const urlObj = new URL(url); @@ -171,7 +171,7 @@ export function dummyRegistryForContext( let retryCountsByURL = new Map(); const _handler: Handler = async request => { urls.push(request.url); - const url = request.url.replaceAll("%2f", "/"); + const url = request.url.replaceAll("%2f", "/").replaceAll("%2F", "/"); let status = 200; @@ -247,7 +247,7 @@ export function dummyRegistry( let retryCountsByURL = new Map(); const _handler: Handler = async request => { urls.push(request.url); - const url = request.url.replaceAll("%2f", "/"); + const url = request.url.replaceAll("%2f", "/").replaceAll("%2F", "/"); let status = 200;