fetch: send the leading slashes of the pathname in the request-target

src/runtime/cli/create_command.rs

@@ -1666,6 +1666,7 @@ impl Example {
 
         let mut github_api_domain: &[u8] = b"api.github.com";
         if let Some(api_domain) = env_loader.map.get(b"GITHUB_API_DOMAIN") {
+            let api_domain = strings::trim_right(api_domain, b"/");
             if !api_domain.is_empty() {
                 github_api_domain = api_domain;
             }

src/runtime/cli/upgrade_command.rs

@@ -201,6 +201,7 @@ impl UpgradeCommand {
         // Incase they're using a GitHub proxy in e.g. China
         let mut github_api_domain: &[u8] = b"api.github.com";
         if let Some(api_domain) = env_loader.map.get(b"GITHUB_API_DOMAIN") {
+            let api_domain = strings::trim_right(api_domain, b"/");
             if !api_domain.is_empty() {
                 github_api_domain = api_domain;
             }

src/url/lib.rs

@@ -821,13 +821,6 @@ impl<'a> URL<'a> {
             url.pathname = b"/";
         }
 
-        const SLASH_SLASH: u16 = u16::from_le_bytes(*b"//");
-        while url.pathname.len() > 1
-            && u16::from_le_bytes([url.pathname[0], url.pathname[1]]) == SLASH_SLASH
-        {
-            url.pathname = &url.pathname[1..];
-        }
-
         // Only the right side: a protocol-relative origin keeps its leading `//`.
         url.origin = strings::trim_right(url.origin, b"/ ?#");
         url
@@ -1865,6 +1858,20 @@ mod tests {
         assert_eq!(url.hash, b"#frag?x=2");
     }
 
+    #[test]
+    fn pathname_keeps_its_leading_slashes() {
+        // The request-target is `new URL(s).pathname + search`, and WHATWG
+        // keeps every leading `/` of a special URL's path.
+        let url = URL::parse(b"http://localhost:3000//a//b?x=1");
+        assert_eq!(url.pathname, b"//a//b?x=1");
+        assert_eq!(url.search, b"?x=1");
+        assert_eq!(url.origin, b"http://localhost:3000");
+
+        let url = URL::parse(b"http://localhost:3000///#frag");
+        assert_eq!(url.pathname, b"///");
+        assert_eq!(url.hash, b"#frag");
+    }
+
     #[test]
     fn the_authority_ends_where_new_url_ends_it() {
         let url = URL::parse(br"http://u:p@first.example:8080\x@second.example/path");

test/cli/install/bun-upgrade.test.ts

@@ -323,6 +323,42 @@ describe.concurrent(() => {
     expect(err).toEndWith(unpackFailure);
     expect(exitCode).toBe(1);
   });
+
+  it("GITHUB_API_DOMAIN with a trailing slash still requests /repos/..., not //repos/...", async () => {
+    const paths: string[] = [];
+    using server = Bun.serve({
+      tls,
+      port: 0,
+      fetch(req) {
+        const { pathname } = new URL(req.url);
+        paths.push(pathname);
+        if (pathname !== "/repos/Jarred-Sumner/bun-releases-for-updater/releases/latest") {
+          return new Response("not found", { status: 404 });
+        }
+        return Response.json({ tag_name: "bun-v9.9.9", assets: [] });
+      },
+    });
+    using stagingRoot = tempDir("bun-upgrade-domain-slash", {});
+    await using proc = spawn({
+      cmd: [standInExe, "upgrade", "--stable"],
+      cwd: standInDir,
+      stdout: null,
+      stdin: "pipe",
+      stderr: "pipe",
+      env: {
+        ...env,
+        NODE_TLS_REJECT_UNAUTHORIZED: "0",
+        GITHUB_API_DOMAIN: `${server.hostname}:${server.port}/`,
+        BUN_TMPDIR: String(stagingRoot),
+        ASAN_OPTIONS: [env.ASAN_OPTIONS, "detect_leaks=0"].filter(Boolean).join(":"),
+      },
+    });
+
+    const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+    expect(paths).toEqual(["/repos/Jarred-Sumner/bun-releases-for-updater/releases/latest"]);
+    expect(err).toContain("Bun v9.9.9 is out, but not for this platform");
+    expect(exitCode).toBe(0);
+  });
 });
 
 it.concurrent(

test/cli/install/npmrc.test.ts

@@ -2842,6 +2842,77 @@ describe.concurrent("a registry URL whose host is easy to misread", () => {
   });
 });
 
+describe.concurrent("the path of a request to the registry", () => {
+  // bun sends the path of the URL it built, a leading `//` included, as npm does. Each row has a
+  // registry of its own, which records the request target and answers 404.
+  type Row = { files: Record<string, string>; cmd?: string[] };
+
+  async function run({ files, cmd = ["install"] }: Row) {
+    const requests: { target: string; auth: string | null }[] = [];
+    await using registry = Bun.serve({
+      port: 0,
+      hostname: "127.0.0.1",
+      fetch(req) {
+        const url = new URL(req.url);
+        requests.push({ target: url.pathname + url.search, auth: req.headers.get("authorization") });
+        return Response.json({ error: "not found" }, { status: 404 });
+      },
+    });
+    const host = `127.0.0.1:${registry.port}`;
+    using dir = tempDir("registry-request-path", {
+      "package.json": JSON.stringify({ name: "app", version: "1.0.0", dependencies: { "no-deps": "1.0.0" } }),
+      ...Object.fromEntries(Object.entries(files).map(([name, text]) => [name, text.replaceAll("HOST", host)])),
+    });
+    await using proc = Bun.spawn({
+      cmd: [bunExe(), ...cmd],
+      cwd: String(dir),
+      // An ambient proxy would take the requests to the local registry.
+      env: {
+        ...env,
+        BUN_INSTALL_CACHE_DIR: join(String(dir), ".cache"),
+        http_proxy: "",
+        https_proxy: "",
+        HTTP_PROXY: "",
+        HTTPS_PROXY: "",
+      },
+      stdout: "pipe",
+      stderr: "pipe",
+    });
+    const [, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+    return { requests, exitCode };
+  }
+
+  test.each<[string, Row, { target: string; auth: string | null }]>([
+    [
+      "a registry at the root, written with two slashes",
+      { files: { ".npmrc": "registry=http://HOST//\n" } },
+      { target: "/no-deps", auth: null },
+    ],
+    [
+      "a registry whose path starts with //",
+      { files: { ".npmrc": "registry=http://HOST//npm/\n" } },
+      { target: "//npm/no-deps", auth: null },
+    ],
+    [
+      "a registry whose path starts with //, for bun pm view",
+      { files: { ".npmrc": "registry=http://HOST//npm/\n" }, cmd: ["pm", "view", "no-deps"] },
+      { target: "//npm/no-deps", auth: null },
+    ],
+    [
+      "a token keyed to the path of the registry as written",
+      { files: { ".npmrc": "registry=http://HOST//npm/\n//HOST//npm/:_authToken=abc\n" } },
+      { target: "//npm/no-deps", auth: "Bearer abc" },
+    ],
+    [
+      "a token keyed to /npm/ is not for the registry at //npm/",
+      { files: { ".npmrc": "registry=http://HOST//npm/\n//HOST/npm/:_authToken=abc\n" } },
+      { target: "//npm/no-deps", auth: null },
+    ],
+  ])("%s", async (_, row, request) => {
+    expect(await run(row)).toEqual({ requests: [request], exitCode: 1 });
+  });
+});
+
 describe("protocol-relative registry", () => {
   test("bun install rejects registry=//host:port/path/ instead of requesting http://localhost/host/", async () => {
     // The `:port` used to be taken for a `:option=value` suffix of the pathname and the

test/js/bun/cookie/cookie.test.ts

@@ -155,7 +155,7 @@ describe("Bun.serve() cookies", () => {
   });
 
   test("set-cookie", async () => {
-    const res = await fetch(server.url + "/tester", {
+    const res = await fetch(new URL("/tester", server.url), {
       method: "POST",
       body: JSON.stringify([["test", "test"]]),
     });
@@ -173,7 +173,7 @@ describe("Bun.serve() cookies", () => {
     `);
   });
   test("set two cookies", async () => {
-    const res = await fetch(server.url + "/tester", {
+    const res = await fetch(new URL("/tester", server.url), {
       method: "POST",
       body: JSON.stringify([
         ["test", "test"],
@@ -196,7 +196,7 @@ describe("Bun.serve() cookies", () => {
     `);
   });
   test("delete cookie", async () => {
-    const res = await fetch(server.url + "/tester", {
+    const res = await fetch(new URL("/tester", server.url), {
       method: "POST",
       body: JSON.stringify([["test", null]]),
     });
@@ -210,7 +210,7 @@ describe("Bun.serve() cookies", () => {
     `);
   });
   test("request with cookies", async () => {
-    const res = await fetch(server.url + "/tester", {
+    const res = await fetch(new URL("/tester", server.url), {
       method: "POST",
       body: JSON.stringify([
         ["do_modify", "c"],
@@ -237,7 +237,7 @@ describe("Bun.serve() cookies", () => {
     `);
   });
   test("request that doesn't modify cookies doesn't set cookies", async () => {
-    const res = await fetch(server.url + "/tester", {
+    const res = await fetch(new URL("/tester", server.url), {
       method: "POST",
       body: JSON.stringify([]),
       headers: {
@@ -324,7 +324,7 @@ describe("Bun.serve() cookies 2", () => {
     `);
   });
   test("server sets cookie on redirect", async () => {
-    const response = await fetch(server.url + "/redirect", {
+    const response = await fetch(new URL("/redirect", server.url), {
       headers: {
         "Cookie": "abc=def; ghi=jkl",
       },
@@ -358,7 +358,7 @@ describe("cookie path option", () => {
   afterAll(() => server.stop());
 
   test("cookie path option", async () => {
-    const response = await fetch(server.url + "/x/y");
+    const response = await fetch(new URL("/x/y", server.url));
     expect(response.status).toBe(200);
     expect(response.headers.getAll("Set-Cookie")).toMatchInlineSnapshot(`
       [

test/js/bun/http/async-iterator-stream.test.ts

@@ -19,7 +19,7 @@ describe.concurrent("Streaming body via", () => {
       },
     });
 
-    const res = await fetch(`${server.url}/`);
+    const res = await fetch(server.url);
     const chunks: Uint8Array[] = [];
     for await (const chunk of res.body!) {
       chunks.push(chunk);
@@ -244,7 +244,7 @@ describe.concurrent("Streaming body via", () => {
       },
     });
     try {
-      const res = await fetch(`${server.url}/`, { signal: aborter.signal });
+      const res = await fetch(server.url, { signal: aborter.signal });
     } catch (e: any) {
       expect(e).toBeInstanceOf(DOMException);
       expect(e.name).toBe("AbortError");
@@ -271,7 +271,7 @@ describe.concurrent("Streaming body via", () => {
       },
     });
 
-    const res = await fetch(`${server.url}/`);
+    const res = await fetch(server.url);
     const chunks: Uint8Array[] = [];
     for await (const chunk of res.body!) {
       chunks.push(chunk);

test/js/bun/http/bun-serve-html.test.ts

@@ -1264,9 +1264,9 @@ test("you can have HTML imports apply to only specific methods outside of the de
   expect(response3.status).toBe(200);
   expect(await response3.text()).toBe("PATCH!");
 
-  expect(await (await fetch(server.url + "/boop")).text()).toEqual(htmlText);
-  expect(await (await fetch(server.url + "/boop", { method: "POST" })).text()).toEqual(htmlText);
-  expect(await (await fetch(server.url + "/boop", { method: "PATCH" })).text()).toBe(htmlText);
+  expect(await (await fetch(new URL("/boop", server.url))).text()).toEqual(htmlText);
+  expect(await (await fetch(new URL("/boop", server.url), { method: "POST" })).text()).toEqual(htmlText);
+  expect(await (await fetch(new URL("/boop", server.url), { method: "PATCH" })).text()).toBe(htmlText);
 });
 
 for (let development of [true, false, { hmr: false }]) {

test/js/bun/http/bun-serve-static.test.ts

@@ -54,7 +54,7 @@ describe.todoIf(isBroken && isMacOS)("static", () => {
     it("GET", async () => {
       const previousCallCount = handler.mock.calls.length;
 
-      const res = await fetch(`${server.url}${path}`);
+      const res = await fetch(new URL(path, server.url));
       expect(res.status).toBe(200);
       expect(await res.bytes()).toEqual(await static_responses[path].bytes());
       expect(handler.mock.calls.length, "Handler should not be called").toBe(previousCallCount);
@@ -63,7 +63,7 @@ describe.todoIf(isBroken && isMacOS)("static", () => {
     it("HEAD", async () => {
       const previousCallCount = handler.mock.calls.length;
 
-      const res = await fetch(`${server.url}${path}`, { method: "HEAD" });
+      const res = await fetch(new URL(path, server.url), { method: "HEAD" });
       expect(res.status).toBe(200);
       expect(await res.bytes()).toHaveLength(0);
       expect(res.headers.get("Content-Length")).toBe(static_responses[path].size.toString());
@@ -73,15 +73,15 @@ describe.todoIf(isBroken && isMacOS)("static", () => {
 
   it("/redirect", async () => {
     const previousCallCount = handler.mock.calls.length;
-    const res = await fetch(`${server.url}/redirect`, { redirect: "manual" });
+    const res = await fetch(new URL("/redirect", server.url), { redirect: "manual" });
     expect(res.status).toBe(302);
     expect(res.headers.get("Location")).toBe("/foo/bar");
     expect(handler.mock.calls.length, "Handler should not be called").toBe(previousCallCount);
   });
 
   it("/redirect (follow)", async () => {
     const previousCallCount = handler.mock.calls.length;
-    const res = await fetch(`${server.url}/redirect`);
+    const res = await fetch(new URL("/redirect", server.url));
     expect(res.status).toBe(200);
     expect(res.url).toBe(`${server.url}foo/bar`);
     expect(await res.text()).toBe("/foo/bar");
@@ -91,7 +91,7 @@ describe.todoIf(isBroken && isMacOS)("static", () => {
 
   it("/redirect/fallback", async () => {
     const previousCallCount = handler.mock.calls.length;
-    const res = await fetch(`${server.url}/redirect/fallback`);
+    const res = await fetch(new URL("/redirect/fallback", server.url));
     expect(res.status).toBe(200);
     expect(await res.text()).toBe(`${server.url}foo/bar/fallback`);
     expect(handler.mock.calls.length, "Handler should be called").toBe(previousCallCount + 1);