Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/bun_core/fmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/install/NetworkTask.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions test/cli/install/bun-add.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"));
Expand Down
6 changes: 3 additions & 3 deletions test/cli/install/bun-create.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
);
});

Expand All @@ -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;
});

Expand All @@ -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;
});
Expand Down
147 changes: 147 additions & 0 deletions test/cli/install/bun-install-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion test/cli/install/bun-install-security-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion test/cli/install/bun-install-tarball-integrity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions test/cli/install/bun-install.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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"]);
Expand Down Expand Up @@ -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`,
Expand Down
4 changes: 2 additions & 2 deletions test/cli/install/bun-update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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`,
Expand Down
2 changes: 1 addition & 1 deletion test/cli/install/bunx.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 3 additions & 3 deletions test/cli/install/dummy.registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -171,7 +171,7 @@ export function dummyRegistryForContext(
let retryCountsByURL = new Map<string, number>();
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;

Expand Down Expand Up @@ -247,7 +247,7 @@ export function dummyRegistry(
let retryCountsByURL = new Map<string, number>();
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;

Expand Down