diff --git a/src/http/AsyncHTTP.rs b/src/http/AsyncHTTP.rs index b7a1a75ee0b2..df4a620f9865 100644 --- a/src/http/AsyncHTTP.rs +++ b/src/http/AsyncHTTP.rs @@ -769,7 +769,33 @@ impl<'a> AsyncHTTP<'a> { // its `Box` is reclaimed. Only the state the clone // built up itself during request processing is torn down. { + // `handle_response_metadata` rewrites per-hop request state in + // place: `client.url`/`connected_url` become self-borrows into + // `client.redirect` (freed just below), the method may be + // downgraded to GET, and a cross-origin hop strips auth headers + // from `client.header_entries`. The copy-back into the JS-thread + // original must not hand a re-scheduled attempt any of that. + // + // `header_entries` is bitwise-shared with the original (see the + // comment above), so it must never be dropped or reallocated + // here. It was cloned from `request_headers` at init and only + // ever shrinks (`ordered_remove`), so its capacity is enough; + // the unchecked append below would corrupt the shared heap + // allocation otherwise, so keep this assertion in release too. + assert!( + (*this).client.header_entries.capacity() >= (*this).request_headers.len() + ); + (*this).client.header_entries.clear_retaining_capacity(); + (*this) + .client + .header_entries + .append_list_assume_capacity(&(*this).request_headers); + let original_url = (*this).url.clone(); + let original_method = (*this).method; let client = &mut (*this).client; + client.url = original_url; + client.connected_url = URL::default(); + client.method = original_method; // Clone-owned (allocated after `ptr::read`). drop(core::mem::take(&mut client.redirect)); drop(core::mem::take(&mut client.prev_redirect)); diff --git a/test/cli/install/bun-install-retry.test.ts b/test/cli/install/bun-install-retry.test.ts index 2538a2cc0a58..b49d0c924750 100644 --- a/test/cli/install/bun-install-retry.test.ts +++ b/test/cli/install/bun-install-retry.test.ts @@ -40,6 +40,192 @@ afterEach(async () => { await dummyAfterEach(); }); +// Manifest request 302-redirects and the redirect target answers a retryable +// 500 once. The install retry must restart from the original manifest URL. +it("retries a manifest whose redirect target 500s once", async () => { + const urls: string[] = []; + let redirectTargetHits = 0; + setHandler(async request => { + const { pathname } = new URL(request.url); + urls.push(pathname); + if (pathname === "/BaR") { + return new Response(null, { status: 302, headers: { Location: `${root_url}/redirected/BaR` } }); + } + if (pathname === "/redirected/BaR") { + if (redirectTargetHits++ === 0) { + return new Response("transient", { status: 500 }); + } + return Response.json({ + name: "BaR", + versions: { + "0.0.2": { name: "BaR", version: "0.0.2", dist: { tarball: `${root_url}/BaR-0.0.2.tgz` } }, + }, + "dist-tags": { latest: "0.0.2" }, + }); + } + if (pathname === "/BaR-0.0.2.tgz") { + return new Response(file(join(import.meta.dir, "bar-0.0.2.tgz"))); + } + return new Response("unexpected", { status: 404 }); + }); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { BaR: "0.0.2" } }), + ); + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install", "--linker=hoisted"], + cwd: package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + expect(err).not.toContain("error:"); + expect(err).toContain("Saved lockfile"); + expect(out).toContain("1 package installed"); + expect(exitCode).toBe(0); + // The retry restarts from the original manifest URL, so the server sees the + // whole redirect chain a second time. + expect(urls).toEqual(["/BaR", "/redirected/BaR", "/BaR", "/redirected/BaR", "/BaR-0.0.2.tgz"]); + expect(await file(join(package_dir, "node_modules", "BaR", "package.json")).json()).toEqual({ + name: "bar", + version: "0.0.2", + }); +}); + +// A cross-origin redirect strips Authorization from the request (per the fetch +// spec). The install retry restarts from the original registry URL and must +// carry the original headers, including Authorization, again. +it("retries an authorized manifest whose cross-origin redirect target 500s once", async () => { + const token = "test-registry-token"; + const registryUrls: string[] = []; + const cdnAuth: (string | null)[] = []; + let cdnHits = 0; + // A second server on its own port stands in for the CDN the registry + // redirects to; a different port makes the redirect cross-origin. + await using cdn = Bun.serve({ + port: 0, + fetch(request) { + if (new URL(request.url).pathname !== "/cdn/BaR") { + return new Response("unexpected", { status: 404 }); + } + cdnAuth.push(request.headers.get("authorization")); + if (cdnHits++ === 0) { + return new Response("transient", { status: 500 }); + } + return Response.json({ + name: "BaR", + versions: { + "0.0.2": { name: "BaR", version: "0.0.2", dist: { tarball: `${root_url}/BaR-0.0.2.tgz` } }, + }, + "dist-tags": { latest: "0.0.2" }, + }); + }, + }); + setHandler(async request => { + const { pathname } = new URL(request.url); + registryUrls.push(pathname); + if (pathname === "/BaR") { + // The registry requires the token on every request, including the retry. + if (request.headers.get("authorization") !== `Bearer ${token}`) { + return new Response("missing authorization", { status: 401 }); + } + return new Response(null, { + status: 302, + headers: { Location: `http://localhost:${cdn.port}/cdn/BaR` }, + }); + } + if (pathname === "/BaR-0.0.2.tgz") { + return new Response(file(join(import.meta.dir, "bar-0.0.2.tgz"))); + } + return new Response("unexpected", { status: 404 }); + }); + await writeFile( + join(package_dir, "bunfig.toml"), + `[install]\ncache = false\nregistry = { url = "${root_url}/", token = "${token}" }\nsaveTextLockfile = false\n`, + ); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { BaR: "0.0.2" } }), + ); + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install", "--linker=hoisted"], + cwd: package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + expect(err).not.toContain("error:"); + expect(err).toContain("Saved lockfile"); + expect(out).toContain("1 package installed"); + expect(exitCode).toBe(0); + // Both registry hits carried the token (the handler 401s otherwise); the + // cross-origin CDN hops must NOT have (the spec strips it for that hop). + expect(registryUrls).toEqual(["/BaR", "/BaR", "/BaR-0.0.2.tgz"]); + expect(cdnAuth).toEqual([null, null]); +}); + +// Sibling retry site (tarball downloads in runTasks): the tarball URL +// 302-redirects and the target 500s once before serving the archive. +it("retries a tarball whose redirect target 500s once", async () => { + const urls: string[] = []; + let redirectTargetHits = 0; + setHandler(async request => { + const { pathname } = new URL(request.url); + urls.push(pathname); + if (pathname === "/BaR") { + return Response.json({ + name: "BaR", + versions: { + "0.0.2": { name: "BaR", version: "0.0.2", dist: { tarball: `${root_url}/BaR-0.0.2.tgz` } }, + }, + "dist-tags": { latest: "0.0.2" }, + }); + } + if (pathname === "/BaR-0.0.2.tgz") { + return new Response(null, { status: 302, headers: { Location: `${root_url}/redirected/BaR-0.0.2.tgz` } }); + } + if (pathname === "/redirected/BaR-0.0.2.tgz") { + if (redirectTargetHits++ === 0) { + return new Response("transient", { status: 500 }); + } + return new Response(file(join(import.meta.dir, "bar-0.0.2.tgz"))); + } + return new Response("unexpected", { status: 404 }); + }); + await writeFile( + join(package_dir, "package.json"), + JSON.stringify({ name: "foo", version: "0.0.1", dependencies: { BaR: "0.0.2" } }), + ); + const { stdout, stderr, exited } = spawn({ + cmd: [bunExe(), "install", "--linker=hoisted"], + cwd: package_dir, + stdout: "pipe", + stdin: "pipe", + stderr: "pipe", + env, + }); + const [err, out, exitCode] = await Promise.all([stderr.text(), stdout.text(), exited]); + expect(err).not.toContain("error:"); + expect(err).toContain("Saved lockfile"); + expect(out).toContain("1 package installed"); + expect(exitCode).toBe(0); + expect(urls).toEqual([ + "/BaR", + "/BaR-0.0.2.tgz", + "/redirected/BaR-0.0.2.tgz", + "/BaR-0.0.2.tgz", + "/redirected/BaR-0.0.2.tgz", + ]); + expect(await file(join(package_dir, "node_modules", "BaR", "package.json")).json()).toEqual({ + name: "bar", + version: "0.0.2", + }); +}); + it("retries on 500", async () => { const urls: string[] = []; setHandler(dummyRegistry(urls, undefined, 4));