diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index 9c041bdf611c..9bdb08907032 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -661,8 +661,8 @@ struct HttpContext { /* Todo: can this handle timeout for non-post as well? */ if (fin) { - /* If we just got the last chunk (or empty chunk), disable timeout */ - us_socket_timeout((struct us_socket_t *) user, 0); + /* The body is complete: the idle timeout now runs against the pending response */ + ((HttpResponse *) user)->resetTimeout(); } else { /* We still have some more data coming in later, so reset timeout */ /* Only reset timeout if we got enough bytes (16kb/sec) since last time we reset here */ diff --git a/test/js/bun/http/serve.test.ts b/test/js/bun/http/serve.test.ts index bad56ba2379b..b3037977672c 100644 --- a/test/js/bun/http/serve.test.ts +++ b/test/js/bun/http/serve.test.ts @@ -3458,6 +3458,151 @@ it.concurrent( 40_000, ); +it.concurrent( + "idleTimeout closes a request with a body whose handler has not responded", + async () => { + const aborted: string[] = []; + const entered: Record> = {}; + const enter = (path: string) => (entered[path] ??= Promise.withResolvers()); + const held = { + "/timeout-0": Promise.withResolvers(), + "/timeout-60": Promise.withResolvers(), + }; + const never = new Promise(() => {}); + const tlsAborted = Promise.withResolvers(); + + function handler(req: Request, server: Server) { + const path = new URL(req.url).pathname; + req.signal.addEventListener("abort", () => { + aborted.push(path); + if (path === "/tls-late") tlsAborted.resolve(); + }); + enter(path).resolve(); + switch (path) { + case "/text": + return req.text().then(() => never); + case "/stream": + return new Response(new ReadableStream({ start: controller => controller.enqueue("first") })); + case "/timeout-2": + server.timeout(req, 2); + return never; + case "/timeout-0": + server.timeout(req, 0); + return held[path].promise; + case "/timeout-60": + server.timeout(req, 60); + return held[path].promise; + } + return never; + } + + using server = Bun.serve({ + port: 0, + hostname: "127.0.0.1", + idleTimeout: 2, + routes: { "/route": { POST: handler } }, + fetch: handler, + }); + // Only server.timeout(req, 2) can close a request on this one. + using patient = Bun.serve({ port: 0, hostname: "127.0.0.1", idleTimeout: 255, fetch: handler }); + using secure = Bun.serve({ port: 0, hostname: "127.0.0.1", idleTimeout: 2, tls, fetch: handler }); + + const sockets: net.Socket[] = []; + using _ = { [Symbol.dispose]: () => sockets.forEach(socket => socket.destroy()) }; + + function open(socket: net.Socket) { + sockets.push(socket); + let response = ""; + let check = () => {}; + const closed = new Promise(resolve => socket.on("close", () => resolve())); + socket.on("error", () => {}); + socket.on("data", chunk => { + response += chunk; + check(); + }); + return { + closed, + // Settles when the kernel has the bytes, or when the timer closed the socket first. + write: (bytes: string) => + Promise.race([new Promise(resolve => socket.write(bytes, () => resolve())), closed]), + received: async (text: string) => { + const arrived = new Promise(resolve => { + check = () => response.includes(text) && resolve(); + check(); + }); + await Promise.race([arrived, closed]); + return response; + }, + }; + } + type Client = ReturnType; + + // A string step is written to the socket. A function step is a condition to wait for. + async function request(to: number | net.Socket, ...steps: (string | ((client: Client) => Promise))[]) { + const client = open(typeof to === "number" ? net.connect(to, "127.0.0.1") : to); + for (const step of steps) await (typeof step === "string" ? client.write(step) : step(client)); + return client; + } + const head = (path: string, framing: string) => `POST ${path} HTTP/1.1\r\nHost: localhost\r\n${framing}\r\n\r\n`; + const contentLength = (path: string) => head(path, "Content-Length: 1"); + const chunked = (path: string) => head(path, "Transfer-Encoding: chunked"); + + const closedByTimer = await Promise.all([ + // The last body byte arrives with the head. + request(server.port!, contentLength("/content-length") + "x"), + request(server.port!, chunked("/chunked") + "1\r\nx\r\n0\r\n\r\n"), + request(server.port!, contentLength("/route") + "x"), + request(server.port!, contentLength("/text") + "x"), + request(patient.port!, contentLength("/timeout-2") + "x"), + // The last body byte arrives in a later read. + request(server.port!, contentLength("/content-length-late"), () => enter("/content-length-late").promise, "x"), + request(server.port!, chunked("/chunked-late") + "1\r\nx\r\n", () => enter("/chunked-late").promise, "0\r\n\r\n"), + request( + nodeTls.connect({ port: secure.port!, host: "127.0.0.1", rejectUnauthorized: false }), + contentLength("/tls-late"), + () => enter("/tls-late").promise, + "x", + ), + // The last body byte arrives after the last write of a response that then goes quiet. + request(server.port!, contentLength("/stream"), client => client.received("first"), "x"), + ]); + const stillOpen = await Promise.all([ + request(server.port!, contentLength("/timeout-0") + "x"), + request(server.port!, contentLength("/timeout-60") + "x"), + ]); + + // The timer wheel closes a request without a body on its next sweep, at most 4 s away. + // The kernel had every byte of the requests above before this one connected, so no + // timer above was armed later than this one: when this socket closes, every request + // that idleTimeout covers is closed too. + const control = await request(server.port!, "GET /no-body HTTP/1.1\r\nHost: localhost\r\n\r\n"); + await control.closed; + + expect(aborted.filter(path => path !== "/tls-late").sort()).toEqual([ + "/chunked", + "/chunked-late", + "/content-length", + "/content-length-late", + "/no-body", + "/route", + "/stream", + "/text", + "/timeout-2", + ]); + // A TLS close waits for the client's close_notify, so this abort comes a few reads later. + await tlsAborted.promise; + await Promise.all(closedByTimer.map(client => client.closed)); + expect(server.pendingRequests).toBe(2); + + held["/timeout-0"].resolve(new Response("zero")); + held["/timeout-60"].resolve(new Response("sixty")); + const responses = await Promise.all([stillOpen[0].received("zero"), stillOpen[1].received("sixty")]); + expect(responses.map(response => response.split("\r\n")[0])).toEqual(["HTTP/1.1 200 OK", "HTTP/1.1 200 OK"]); + }, + // One sweep of the 4 s timer wheel does not fit the 5 s default. + 15_000, +); + it.concurrent("#6462", async () => { let headers: string[] = []; using server = Bun.serve({