diff --git a/src/lib/pinned-http.ts b/src/lib/pinned-http.ts index 97e8d91a9c..b2b04c0705 100644 --- a/src/lib/pinned-http.ts +++ b/src/lib/pinned-http.ts @@ -51,6 +51,7 @@ function pinnedHttpRequest( && options?.inactivityTimeoutMs === undefined; const firstByteTimeoutMs = options?.firstByteTimeoutMs ?? legacyIdleTimeoutMs; const inactivityTimeoutMs = options?.inactivityTimeoutMs ?? legacyIdleTimeoutMs; + const legacyFirstByteDisabled = usesLegacyIdleTimeout && legacyIdleTimeoutMs === 0; const maxBytes = options?.maxBytes; const headers = new Headers(options?.headers); headers.set("host", parsed.host); @@ -88,6 +89,7 @@ function pinnedHttpRequest( }; const startFirstByteTimer = () => { clearFirstByteTimer(); + if (settled || legacyFirstByteDisabled) return; firstByteTimer = setTimeout( () => fail(new PinnedHttpError("first_byte_timeout", `${context} first byte timed out`)), firstByteTimeoutMs, @@ -194,8 +196,8 @@ function pinnedHttpRequest( const requestFn = parsed.protocol === "https:" ? https.request : http.request; req = requestFn(requestOptions, onResponse); + if (usesLegacyIdleTimeout) startFirstByteTimer(); const onAbort = () => fail(signal?.reason instanceof Error ? signal.reason : new Error("aborted")); - signal?.addEventListener("abort", onAbort, { once: true }); req.on("socket", (socket) => { const connectedEvent = parsed.protocol === "https:" ? "secureConnect" : "connect"; if (!socket.connecting) { @@ -233,6 +235,9 @@ function pinnedHttpRequest( clearFirstByteTimer(); signal?.removeEventListener("abort", onAbort); }); + signal?.addEventListener("abort", onAbort, { once: true }); + if (signal?.aborted && !settled) onAbort(); + if (settled) return; req.end(body); }); } @@ -262,4 +267,4 @@ export function pinnedHttpPost( options?: PinnedHttpRequestOptions, ): Promise { return pinnedHttpRequest(url, pinned, "POST", body, signal, options); -} \ No newline at end of file +} diff --git a/tests/pinned-http.test.ts b/tests/pinned-http.test.ts new file mode 100644 index 0000000000..24d81ed456 --- /dev/null +++ b/tests/pinned-http.test.ts @@ -0,0 +1,96 @@ +import { createServer, type Server, type Socket } from "node:net"; +import { afterEach, describe, expect, test } from "bun:test"; +import { PinnedHttpError, pinnedHttpGet } from "../src/lib/pinned-http"; + +let server: Server | undefined; +const sockets = new Set(); + +function trackSocket(socket: Socket): void { + sockets.add(socket); + socket.on("error", () => { /* client timeout or cleanup can reset the peer */ }); + socket.on("close", () => sockets.delete(socket)); +} + +async function listen(handler: (socket: Socket) => void): Promise { + server = createServer((socket) => { + trackSocket(socket); + handler(socket); + }); + return await new Promise((resolve, reject) => { + server!.once("error", reject); + server!.listen(0, "127.0.0.1", () => { + const address = server!.address(); + if (!address || typeof address === "string") { + reject(new Error("test server did not expose a TCP port")); + return; + } + resolve(address.port); + }); + }); +} + +function request(port: number, idleTimeoutMs: number, signal?: AbortSignal): Promise { + return pinnedHttpGet( + `http://slow-header.invalid:${port}/`, + { address: "127.0.0.1", family: 4 }, + signal, + { idleTimeoutMs }, + ); +} + +afterEach(async () => { + for (const socket of sockets) socket.destroy(); + sockets.clear(); + if (server) { + const closing = server; + server = undefined; + await new Promise((resolve) => closing.close(() => resolve())); + } +}); + +describe("pinned HTTP timeouts", () => { + test("legacy idle timeout is also an absolute response-header deadline", async () => { + const port = await listen((socket) => { + socket.write("HTTP/1.1 200 OK\r\nX-Slow: "); + const drip = setInterval(() => socket.write("x"), 20); + socket.on("close", () => clearInterval(drip)); + }); + + const error = await request(port, 150).catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(PinnedHttpError); + expect(error).toMatchObject({ code: "first_byte_timeout" }); + }); + + test("legacy idleTimeoutMs zero still disables the header and socket timers", async () => { + const port = await listen((socket) => { + let bodyReply: ReturnType | undefined; + const headersReply = setTimeout(() => { + socket.write("HTTP/1.1 200 OK\r\nContent-Length: 2\r\nConnection: close\r\n\r\n"); + bodyReply = setTimeout(() => socket.end("ok"), 50); + }, 50); + socket.on("close", () => { + clearTimeout(headersReply); + if (bodyReply !== undefined) clearTimeout(bodyReply); + }); + }); + + const response = await request(port, 0); + expect(response.status).toBe(200); + expect(await response.text()).toBe("ok"); + }); + + test("an abort between the initial check and listener installation is observed", async () => { + const port = await listen(() => { /* hold the socket until cancellation */ }); + const reason = new Error("abort during listener installation"); + let aborted = false; + const signal = { + get aborted() { return aborted; }, + get reason() { return reason; }, + addEventListener() { aborted = true; }, + removeEventListener() { /* no-op test signal */ }, + } as unknown as AbortSignal; + + const error = await request(port, 100, signal).catch((caught: unknown) => caught); + expect(error).toBe(reason); + }); +});