diff --git a/src/http/lib.rs b/src/http/lib.rs index eca6a662c26a..f3bba661e5f8 100644 --- a/src/http/lib.rs +++ b/src/http/lib.rs @@ -3618,11 +3618,18 @@ impl<'a> HTTPClient<'a> { buffer.list.as_slice() }; - // Persist the unparsed tail for the next `on_data` and re-arm the - // receive timeout. When `needs_move`, `to_read` is a suffix of - // `incoming_data` and is copied into the (currently empty) accumulation - // buffer; otherwise `to_read` is a suffix of `buffer`, so the consumed - // prefix is drained and `buffer` is moved back into state. + // Persist the unparsed tail for the next `on_data`. When `needs_move`, + // `to_read` is a suffix of `incoming_data` and is copied into the + // (currently empty) accumulation buffer; otherwise `to_read` is a + // suffix of `buffer`, so the consumed prefix is drained and `buffer` + // is moved back into state. + // + // Deliberately does NOT re-arm the socket timer: the timer armed at + // request-write time stays monotonic for the whole response-header + // phase, so it acts as an absolute headers deadline (undici's + // `headersTimeout`). Re-arming here let a server that drips one + // header line per pin the request forever. The timer + // is re-armed below once headers are complete, for the body phase. macro_rules! short_read { () => {{ bun_core::scoped_log!(fetch, "handleShortRead"); @@ -3638,7 +3645,6 @@ impl<'a> HTTPClient<'a> { .drain_front(buffer.list.len().saturating_sub(keep)); self.state.response_message_buffer = buffer; } - self.set_timeout(&socket); return; }}; } @@ -3727,6 +3733,12 @@ impl<'a> HTTPClient<'a> { } }; + // Headers are complete: re-arm the idle timer for the body phase. + // `short_read!()` above deliberately leaves the timer untouched so the + // header phase has an absolute deadline; this is the boundary where + // the body-idle semantics begin. + self.set_timeout(&socket); + if (self.state.content_encoding_i as usize) < response.headers.list.len() && !self.state.flags.did_set_content_encoding { diff --git a/test/js/web/fetch/fetch.test.ts b/test/js/web/fetch/fetch.test.ts index 88180c6d016b..b0ffe19d3166 100644 --- a/test/js/web/fetch/fetch.test.ts +++ b/test/js/web/fetch/fetch.test.ts @@ -3068,3 +3068,95 @@ it("an explicit numeric `timeout` extends the socket idle deadline past the defa expect(out.withDefault).toStartWith("ERR:"); expect(exitCode).toBe(0); }, 60_000); + +// The response-header phase is an absolute deadline, not an idle timer: a +// server that drips one header line per second must still time out. Before the +// fix, every partial-header read re-armed the idle timer, so a drip faster +// than the idle window pinned the request (and its socket/request-cap slot) +// forever; undici rejects the same drip with HeadersTimeoutError. +it("a dripping response header does not reset the idle timer", async () => { + // The child runs with BUN_CONFIG_HTTP_IDLE_TIMEOUT=5 (2 ticks of uSockets' + // 4s sweep; 1-4s map to a single tick and would fire on the next sweep + // regardless of re-arming, which would mask the bug) and talks to a raw TCP + // server. + // /drip: writes the status line then one header line every second, never + // finishing the header block. Each drip is well under the 5s idle + // window, so an idle timer would never fire. The header-phase + // deadline must fire anyway (within ~8s). + // /body: writes complete headers immediately, then one body byte every + // second for 12s. Body bytes must still re-arm the idle timer, so + // this resolves with the full payload even though it runs longer + // than the header deadline. + const script = /* js */ ` + const net = require("node:net"); + const DRIP_MS = 1000; + const BODY_CHUNKS = 12; + const WATCHDOG_MS = 20_000; + const srv = net.createServer(s => { + s.on("error", () => {}); + s.once("data", d => { + if (d.includes("GET /drip")) { + s.write("HTTP/1.1 200 OK\\r\\n"); + let n = 0; + const iv = setInterval( + () => (s.destroyed ? clearInterval(iv) : s.write("X-Drip-" + n++ + ": v\\r\\n")), + DRIP_MS, + ); + } else { + s.write("HTTP/1.1 200 OK\\r\\nConnection: close\\r\\n\\r\\n"); + let n = 0; + const iv = setInterval(() => { + if (s.destroyed) return clearInterval(iv); + s.write("x"); + if (++n === BODY_CHUNKS) { clearInterval(iv); s.end(); } + }, DRIP_MS); + } + }); + }); + await new Promise(r => srv.listen(0, "127.0.0.1", r)); + const base = "http://127.0.0.1:" + srv.address().port; + const t0 = Date.now(); + const pending = tag => new Promise(r => setTimeout(r, WATCHDOG_MS, { [tag]: "pending" })); + const [drip, body] = await Promise.all([ + Promise.race([ + fetch(base + "/drip").then( + () => ({ drip: "resolved" }), + e => ({ drip: String(e?.name ?? e?.code ?? e), ms: Date.now() - t0 }), + ), + pending("drip"), + ]), + Promise.race([ + fetch(base + "/body").then(r => r.text()).then( + t => ({ body: "ok", len: t.length }), + e => ({ body: "ERR:" + (e?.name ?? e?.code ?? e) }), + ), + pending("body"), + ]), + ]); + console.log(JSON.stringify({ ...drip, ...body })); + process.exit(0); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", script], + env: { ...bunEnv, BUN_CONFIG_HTTP_IDLE_TIMEOUT: "5" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + const out = JSON.parse(stdout.trim().split("\n").pop()!) as { + drip: string; + ms?: number; + body: string; + len?: number; + }; + // /body dripped for ~12s at 1s/byte with a 5s idle window and still + // resolved: body bytes still re-arm the idle timer. + expect({ body: out.body, len: out.len }).toEqual({ body: "ok", len: 12 }); + // /drip must have rejected with a timeout inside the watchdog window. + // Before the fix this stayed "pending": every dripped header line re-armed + // the 5s idle timer, and 1s < 5s keeps it ahead of the sweep forever. + expect(out.drip).toMatch(/Timeout/i); + expect(out.ms).toBeLessThan(20_000); + expect(exitCode).toBe(0); +}, 60_000);