From dcf64f104d68bca440ff256df786a337e225edc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Thu, 16 Jul 2026 18:37:40 +0200 Subject: [PATCH] fix(http): Node http client must not follow redirects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Node's `http.request`/`http.get`/`https.request` never follow HTTP redirects — a 3xx is delivered to the caller verbatim (only `fetch` follows, per its WHATWG redirect mode). Perry's Node http client is built on reqwest, whose default `redirect::Policy` silently follows up to 10 hops. That auto-follow is observably wrong (the caller sees the final 200 instead of the 307 + `location`). Worse, it breaks Next.js's `proxyRequest`: its bundled `http-proxy` runs over this very client, so a proxied sub-request that 307-redirects back to the entry path was auto-followed here instead of relayed as a transparent response. With a locale middleware where `/` rewrites to `/en` and `/en` 307s back to `/`, the router re-resolved the same rewrite forever — the request hung (no response, connection leak) instead of returning the 307. Disable redirect-following (`Policy::none()`) on both Node-http client backends: - perry-ext-http `apply_node_proxy_policy` (the `external-http-client- pump` path — every real client build funnels through it: HTTP_CLIENT, the per-Agent client, the TLS-customized client, default_client) - perry-stdlib `js_http_client_request_end`'s reqwest builder (the non-`external-http-client-pump` path) so a 3xx flows back to the caller exactly as Node delivers it. `fetch` has its own client and redirect handling and is unaffected. Claude-Session: https://claude.ai/code/session_01KD4GTmiwZjRrxShzQydJpF --- crates/perry-ext-http/src/lib.rs | 12 +++++ crates/perry-stdlib/src/http.rs | 2 + ...test_gap_http_client_no_redirect_follow.ts | 46 +++++++++++++++++++ 3 files changed, 60 insertions(+) create mode 100644 test-files/test_gap_http_client_no_redirect_follow.ts diff --git a/crates/perry-ext-http/src/lib.rs b/crates/perry-ext-http/src/lib.rs index 7cc0a790e3..e72c42bbbd 100644 --- a/crates/perry-ext-http/src/lib.rs +++ b/crates/perry-ext-http/src/lib.rs @@ -250,7 +250,19 @@ fn node_env_proxy_enabled() -> bool { /// Apply the Node-conformant proxy policy to a reqwest client builder: honor /// the standard proxy env vars only when `NODE_USE_ENV_PROXY=1`, matching Node. +/// +/// Also disables reqwest's default redirect-following. Node's +/// `http.request`/`http.get`/`https.request` NEVER follow redirects — a 3xx is +/// delivered to the caller verbatim (only `fetch` follows, per its WHATWG +/// redirect mode). reqwest follows up to 10 hops by default, which is +/// observably wrong for the Node client and, worse, turned Next.js's +/// `proxyRequest` (its bundled `http-proxy` runs over this client) into an +/// infinite loop: a proxied sub-request that 307-redirects back to the entry +/// path was auto-followed here instead of relayed for a transparent response, +/// so the router re-resolved the same middleware rewrite forever (a locale +/// middleware where `/` rewrites to `/en` and `/en` 307s back to `/`). pub(crate) fn apply_node_proxy_policy(builder: reqwest::ClientBuilder) -> reqwest::ClientBuilder { + let builder = builder.redirect(reqwest::redirect::Policy::none()); if node_env_proxy_enabled() { builder } else { diff --git a/crates/perry-stdlib/src/http.rs b/crates/perry-stdlib/src/http.rs index 887fdee420..cb5e65023e 100644 --- a/crates/perry-stdlib/src/http.rs +++ b/crates/perry-stdlib/src/http.rs @@ -925,6 +925,8 @@ pub unsafe extern "C" fn js_http_client_request_end(handle: Handle, body_f64: f6 let req_handle = handle; spawn(async move { let mut builder = reqwest::Client::builder(); + // Node's http client never follows redirects; disable reqwest's default (rationale: `apply_node_proxy_policy` in `perry-ext-http`). + builder = builder.redirect(reqwest::redirect::Policy::none()); builder = if let Some(timeout) = timeout_ms { builder.timeout(std::time::Duration::from_millis(timeout)) } else { diff --git a/test-files/test_gap_http_client_no_redirect_follow.ts b/test-files/test_gap_http_client_no_redirect_follow.ts new file mode 100644 index 0000000000..d95fc8ec57 --- /dev/null +++ b/test-files/test_gap_http_client_no_redirect_follow.ts @@ -0,0 +1,46 @@ +// Regression: Node's `http.get`/`http.request` client must NOT follow HTTP +// redirects — a 3xx response is delivered to the caller verbatim (only `fetch` +// follows, per its WHATWG redirect mode). Perry's Node http client is built on +// reqwest, whose DEFAULT policy silently follows up to 10 hops. That auto-follow +// is observably wrong (the caller sees the final 200 instead of the 307) and, +// worse, made Next.js's `proxyRequest` (its bundled `http-proxy` runs over this +// client) loop forever: a proxied sub-request that 307-redirects back to the +// entry path was followed instead of relayed, so the router re-resolved the same +// locale-middleware rewrite endlessly (`/` rewrites to `/en`, `/en` 307s to `/`). +// +// The observable: the client must report the 307 and the `location` header, and +// the server must be hit exactly ONCE — byte-for-byte identical to +// `node --experimental-strip-types`. + +import { createServer, get } from "node:http"; + +const PORT = 18993; +let serverHits = 0; + +const server = createServer((req: any, res: any) => { + serverHits++; + if (req.url === "/target") { + res.statusCode = 200; + res.end("FINAL"); + return; + } + res.statusCode = 307; + res.setHeader("location", `http://127.0.0.1:${PORT}/target`); + res.end("redirect-body"); +}); + +server.listen(PORT, () => { + get(`http://127.0.0.1:${PORT}/start`, (res: any) => { + let body = ""; + res.setEncoding("utf8"); + res.on("data", (c: string) => (body += c)); + res.on("end", () => { + console.log(`status=${res.statusCode}`); + console.log(`location=${res.headers.location}`); + console.log(`body=${body}`); + console.log(`serverHits=${serverHits}`); + console.log(`followed=${res.statusCode === 200}`); + server.close(); + }); + }); +});