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

@@ -204,7 +204,9 @@ pub struct URL<'a> {
     pub href: &'a [u8],
     pub origin: &'a [u8],
     pub password: &'a [u8],
+    /// The path and the query, as written: `//a//b?x=1`. The HTTP client sends it as the request-target.
     pub pathname: &'a [u8],
+    /// The path without the query, with one leading `/` and no trailing `/`: `/a//b`. `/x` collapses to `/`.
     pub path: &'a [u8],
     pub port: &'a [u8],
     pub protocol: &'a [u8],
@@ -500,19 +502,22 @@ impl<'a> URL<'a> {
     }
 
     /// The URL without its userinfo, ending in exactly one `/`: `http://host/`, `http://host/npm/`.
+    /// The leading slashes of the path stay as written: `http://host//npm/`.
     pub fn href_without_auth(&self) -> Box<[u8]> {
         let proto = self.display_protocol();
-        let path = strings::trim(self.pathname, b"/");
+        let path = strings::trim_right(self.pathname, b"/");
 
         let mut buf: Vec<u8> =
             Vec::with_capacity(proto.len() + 3 + self.host.len() + 1 + path.len() + 1);
         buf.extend_from_slice(proto);
         buf.extend_from_slice(b"://");
         // bun_core::io::Write on Vec<u8> is infallible.
         let _ = buf.print(format_args!("{}", self.display_host()));
-        buf.push(b'/');
+        if !path.starts_with(b"/") {
+            buf.push(b'/');
+        }
+        buf.extend_from_slice(path);
         if !path.is_empty() {
-            buf.extend_from_slice(path);
             buf.push(b'/');
         }
         buf.into_boxed_slice()
@@ -821,13 +826,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 +1863,39 @@ mod tests {
         assert_eq!(url.hash, b"#frag?x=2");
     }
 
+    #[test]
+    fn pathname_keeps_its_leading_slashes() {
+        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 href_without_auth_keeps_the_leading_slashes_and_ends_in_one() {
+        let href = |url: &[u8]| URL::parse(url).href_without_auth();
+        assert_eq!(
+            &*href(b"http://u:p@localhost:3000"),
+            b"http://localhost:3000/"
+        );
+        assert_eq!(
+            &*href(b"http://u:p@localhost:3000//"),
+            b"http://localhost:3000/"
+        );
+        assert_eq!(
+            &*href(b"http://u:p@localhost:3000/npm"),
+            b"http://localhost:3000/npm/"
+        );
+        assert_eq!(
+            &*href(b"http://u:p@localhost:3000//npm/"),
+            b"http://localhost:3000//npm/"
+        );
+    }
+
     #[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-create.test.ts

@@ -973,6 +973,20 @@ describe.concurrent("GitHub template", () => {
     },
   );
 
+  it("GITHUB_API_DOMAIN with a trailing slash still requests /repos/..., not //repos/...", async () => {
+    using github = await serveGithub(createTest);
+    using dir = tempDir("create-github-domain-slash", {});
+    const { out, err, exitCode } = await create(String(dir), ["github.com/dylan-conway/create-test", "--no-install"], {
+      ...github.env,
+      GITHUB_API_DOMAIN: `${github.env.GITHUB_API_DOMAIN}/`,
+    });
+
+    expect(err).not.toContain("error:");
+    expect(github.requests).toEqual(["/repos/dylan-conway/create-test/tarball"]);
+    expect(out).toContain("Success! dylan-conway/create-test loaded into create-test");
+    expect(exitCode).toBe(0);
+  });
+
   // The destination lost its .npmignore, and `git add` + `git commit` ran in the user's repository.
   it("leaves the .npmignore and the git repository of an existing destination alone", async () => {
     using github = await serveGithub({ "tpl.txt": "template" });

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

@@ -176,21 +176,25 @@ async function writeFakeReleaseZip(outPath: string, version: string): Promise<vo
   }
 }
 
-type ReleaseServer = Bun.Server<undefined> & { env: Record<string, string> };
+type ReleaseServer = Bun.Server<undefined> & { env: Record<string, string>; requests: string[] };
 
 function startReleaseServer(opts: {
   tagName: string;
   assetNames?: string[];
   zipPath?: string;
   zipBody?: string;
   digest?: string;
+  /** Appended to the `GITHUB_API_DOMAIN` the server hands out. */
+  domainSuffix?: string;
 }): ReleaseServer {
   const assetNames = opts.assetNames ?? allAssetNames();
+  const requests: string[] = [];
   const server = Bun.serve({
     tls: tls,
     port: 0,
     async fetch(req) {
       const { pathname } = new URL(req.url);
+      requests.push(pathname);
       if (pathname.startsWith("/download/")) {
         if (opts.zipPath) return new Response(Bun.file(opts.zipPath));
         return new Response(opts.zipBody ?? "this is not a real zip archive");
@@ -209,10 +213,11 @@ function startReleaseServer(opts: {
       );
     },
   }) as ReleaseServer;
+  server.requests = requests;
   server.env = {
     ...env,
     NODE_TLS_REJECT_UNAUTHORIZED: "0",
-    GITHUB_API_DOMAIN: `${server.hostname}:${server.port}`,
+    GITHUB_API_DOMAIN: `${server.hostname}:${server.port}${opts.domainSuffix ?? ""}`,
     // The upgrade-failure path exits via Global::exit(1) while the HTTP
     // thread and the intentionally-leaked progress/download buffers are
     // still live; LeakSanitizer reports those at exit and abort_on_error
@@ -323,6 +328,25 @@ describe.concurrent(() => {
     expect(err).toEndWith(unpackFailure);
     expect(exitCode).toBe(1);
   });
+
+  it("GITHUB_API_DOMAIN with a trailing slash still requests /repos/..., not //repos/...", async () => {
+    using server = startReleaseServer({ tagName: "bun-v9.9.9", domainSuffix: "/" });
+    using stagingRoot = tempDir("bun-upgrade-domain-slash", {});
+    await using proc = spawn({
+      cmd: [standInExe, "upgrade", "--stable"],
+      cwd: standInDir,
+      stdout: null,
+      stdin: "pipe",
+      stderr: "pipe",
+      env: { ...server.env, BUN_TMPDIR: String(stagingRoot) },
+    });
+
+    const [err, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]);
+    expect(server.requests[0]).toBe("/repos/Jarred-Sumner/bun-releases-for-updater/releases/latest");
+    expectTargetsRelease(err, "9.9.9");
+    expect(err).toEndWith(unpackFailure);
+    expect(exitCode).toBe(1);
+  });
 });
 
 it.concurrent(

test/cli/install/npmrc.test.ts

@@ -2840,6 +2840,82 @@ 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 registry whose path starts with //, with credentials in its URL",
+      { files: { ".npmrc": "registry=http://alice:s3cret@HOST//npm/\n" } },
+      { target: "//npm/no-deps", auth: `Basic ${btoa("alice:s3cret")}` },
+    ],
+    [
+      "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

@@ -156,7 +156,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"]]),
     });
@@ -174,7 +174,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"],
@@ -197,7 +197,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]]),
     });
@@ -211,7 +211,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"],
@@ -238,7 +238,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: {
@@ -325,7 +325,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",
       },
@@ -359,7 +359,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

@@ -20,7 +20,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);
@@ -246,7 +246,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");
@@ -274,7 +274,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

@@ -1661,9 +1661,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 }]) {