diff --git a/packages/bun-uws/src/HttpContext.h b/packages/bun-uws/src/HttpContext.h index c3fbda0d72c7..e8338a5c9023 100644 --- a/packages/bun-uws/src/HttpContext.h +++ b/packages/bun-uws/src/HttpContext.h @@ -379,6 +379,12 @@ struct HttpContext { us_socket_unref(s); return s; } + /* Same for bytes that arrive behind a finished response that closes the + * connection while its body is still draining (onWritable closes then). */ + if (httpResponseData->isDrainingBeforeClose() && !httpResponseData->isConnectRequest) [[unlikely]] { + us_socket_unref(s); + return s; + } } /* Cork this socket */ @@ -409,16 +415,29 @@ struct HttpContext { auto *nodeHttpResponseData = (HttpResponseData *) httpResponseData; nodeHttpRequestTrailers = &nodeHttpResponseData->nodeHttpRequestTrailers; } + /* node:http compat: set when the request handler below stops the parse at + * a request pipelined behind a response that closes the connection. */ + bool stoppedBehindClosingResponse = false; + + auto result = httpResponseData->template consumePostPadded(httpContextData->maxHeaderSize, httpResponseData->isConnectRequest, httpContextData->flags.requireHostHeader,httpContextData->flags.useStrictMethodValidation, httpContextData->flags.useInsecureHTTPParser, httpContextData->flags.useLenientTransferEncoding, nodeHttpRequestTrailers, &httpResponseData->chunkedExtensionsByteCount, data, (unsigned int) length, s, [httpContextData, &stoppedBehindClosingResponse](void *s, HttpRequest *httpRequest) -> void * { - auto result = httpResponseData->template consumePostPadded(httpContextData->maxHeaderSize, httpResponseData->isConnectRequest, httpContextData->flags.requireHostHeader,httpContextData->flags.useStrictMethodValidation, httpContextData->flags.useInsecureHTTPParser, httpContextData->flags.useLenientTransferEncoding, nodeHttpRequestTrailers, &httpResponseData->chunkedExtensionsByteCount, data, (unsigned int) length, s, [httpContextData](void *s, HttpRequest *httpRequest) -> void * { + HttpResponseData *httpResponseData = (HttpResponseData *) us_socket_ext((us_socket_t *) s); + + /* node:http compat: never dispatch a request pipelined behind a + * response that closes the connection (still draining, or flushed by + * the uncork after this parse). Dispatching it would reset the + * response state, drop HTTP_CONNECTION_CLOSE and answer it behind a + * body the peer reads up to the FIN. */ + if (IsNodeHttp && httpResponseData->isDrainingBeforeClose()) [[unlikely]] { + stoppedBehindClosingResponse = true; + return nullptr; + } /* For every request we reset the timeout and hang until user makes action */ /* Warning: if we are in shutdown state, resetting the timer is a security issue! */ us_socket_timeout((us_socket_t *) s, 0); - HttpResponseData *httpResponseData = (HttpResponseData *) us_socket_ext((us_socket_t *) s); - /* node:http compat: the JS layer stopped HTTP processing on this * connection (the user emitted 'close' on the socket - Node frees * the parser there); abandon the rest of the buffer. */ @@ -734,6 +753,16 @@ struct HttpContext { /* It is okay to uncork a closed socket and we need to */ ((AsyncSocket *) s)->uncork(); + /* node:http compat: parsing stopped behind a response that closes the + * connection. If the uncork above flushed the last of it, close now; + * otherwise onWritable closes once the buffer drains. */ + if constexpr (IsNodeHttp) { + if (stoppedBehindClosingResponse && !us_socket_is_closed(s)) { + us_socket_unref(s); + ((HttpResponse *) s)->closeIfDoneAndMarked(httpResponseData); + } + } + /* We cannot return nullptr to the underlying stack in any case */ return s; } diff --git a/packages/bun-uws/src/HttpResponse.h b/packages/bun-uws/src/HttpResponse.h index c556f4dece84..4b5497c02f02 100644 --- a/packages/bun-uws/src/HttpResponse.h +++ b/packages/bun-uws/src/HttpResponse.h @@ -630,6 +630,11 @@ struct HttpResponse : public AsyncSocket { /* This will be sent always when state is HTTP_WRITE_CALLED inside internalEnd, so no need to write the terminating 0 chunk here */ /* Super::write("\r\n0\r\n\r\n", 7); */ + /* As in end(): the close is what terminates a close-delimited body. */ + if (httpResponseData->state & HttpResponseData::HTTP_CLOSE_DELIMITED) { + httpResponseData->state |= HttpResponseData::HTTP_CONNECTION_CLOSE; + } + return internalEnd({nullptr, 0}, 0, false, false, closeConnection); } diff --git a/packages/bun-uws/src/HttpResponseData.h b/packages/bun-uws/src/HttpResponseData.h index 756007a69d28..9e38a0baf7e9 100644 --- a/packages/bun-uws/src/HttpResponseData.h +++ b/packages/bun-uws/src/HttpResponseData.h @@ -232,6 +232,15 @@ struct HttpResponseData : AsyncSocketData, HttpParser { || ((state & HTTP_NODE_RECEIVED_FIN) && nodeHttpQueuedPipelinedCount == 0) || ((state & HTTP_CLOSE_WHEN_IDLE) && this->isIdle); } + + /* The response that closes this connection (Connection: close, HTTP/1.0, a + * close-delimited body) is complete; the socket only stays open until its + * buffered bytes drain. Nothing received from here on is a request this + * connection may answer (RFC 9112 9.6), and after a close-delimited body the + * peer would read whatever we send as more body. */ + bool isDrainingBeforeClose() const { + return (state & (HTTP_CONNECTION_CLOSE | HTTP_RESPONSE_PENDING)) == HTTP_CONNECTION_CLOSE; + } }; /* Per-connection state that only node:http compat servers need. diff --git a/src/js/node/_http_server.ts b/src/js/node/_http_server.ts index 45fb072718c6..77295bddbd8b 100644 --- a/src/js/node/_http_server.ts +++ b/src/js/node/_http_server.ts @@ -2274,20 +2274,32 @@ function renderNativeHeaders(res) { // header is rendered so the advertised value matches the transport. let closeDelimited = false; let forceChunked = false; + // False for HTTP/1.0 (set by the constructor) or when the user cleared it. + const chunkedByDefault = !!res.useChunkedEncodingByDefault; + // Node's _storeHeader shouldSendKeepAlive: without chunked encoding only an + // explicit Content-Length lets the connection persist. + const canPersist = chunkedByDefault || storedContentLength !== undefined; if (storedContentLength === undefined && storedTransferEncoding === undefined) { if (res._hasBody === false) { // HEAD / 204 / 304 / 1xx: there is no body to delimit, so removing the // framing headers must not close the connection (Node's _storeHeader // checks !_hasBody before its close-delimited else-branch). + } else if (!chunkedByDefault) { + // Node's _storeHeader `else if (!this.useChunkedEncodingByDefault) this._last = true`, + // taken before the Content-Length branch: no framing header at all, the body + // runs until the connection closes. + closeDelimited = true; + res[kMustCloseConnection] = true; } else if (res._removedTE) { closeDelimited = true; res[kMustCloseConnection] = true; } else if (res._removedContLen || res[kFramingFrozenChunked]) { - // Node's _storeHeader falls through to chunked only when useChunkedEncodingByDefault - // (false for HTTP/1.0); the native writer never chunk-frames HTTP/1.0, so the rest is - // close-delimited. An explicit writeHead() reaches the same null-_contentLength fallthrough. + // Node's _storeHeader falls through to chunked here. The flag is still set for an + // HTTP/1.0 request that sent `TE: chunked`, but the native writer never chunk-frames + // HTTP/1.0, so that stays close-delimited. An explicit writeHead() reaches the same + // null-_contentLength fallthrough. const req = res.req; - if (res.useChunkedEncodingByDefault && req.httpVersionMajor >= 1 && req.httpVersionMinor >= 1) { + if (req.httpVersionMajor >= 1 && req.httpVersionMinor >= 1) { forceChunked = true; } else { closeDelimited = true; @@ -2307,6 +2319,7 @@ function renderNativeHeaders(res) { if ( !defectiveNoBodyResponse && !closeDelimited && + canPersist && !res.maxRequestsOnConnectionReached && res.shouldKeepAlive !== false && requestShouldKeepAlive(res.req) @@ -2328,7 +2341,7 @@ function renderNativeHeaders(res) { // Like Node's shouldSendKeepAlive/_last handling: a user-cleared // shouldKeepAlive (graceful-shutdown helpers set it on in-flight // responses) must also end the socket after 'finish'. - if (res.shouldKeepAlive === false) { + if (res.shouldKeepAlive === false || !canPersist) { res[kMustCloseConnection] = true; } autoHeaders |= AUTO_HEADER_CONN_CLOSE; diff --git a/test/js/node/http/node-http-transfer-encoding.test.ts b/test/js/node/http/node-http-transfer-encoding.test.ts index a1f1a3adb8d7..dfce43f6843c 100644 --- a/test/js/node/http/node-http-transfer-encoding.test.ts +++ b/test/js/node/http/node-http-transfer-encoding.test.ts @@ -893,3 +893,289 @@ describe("tearing down a response with its headers on the wire adds no bytes", ( expect(exitCode).toBe(0); }); }); + +// `res.useChunkedEncodingByDefault = false` is Node's switch for serving an +// HTTP/1.1 body with neither Content-Length nor chunked framing: _storeHeader +// takes its `!useChunkedEncodingByDefault` branch before it would write either +// framing header, the body runs until the connection closes, and the response +// advertises `Connection: close`. The expected heads and bodies below are what +// node v26.3.0 puts on the wire for the same handlers. +describe("res.useChunkedEncodingByDefault = false makes the response close-delimited", () => { + type Exchange = { + head: string; + body: string; + framing: "content-length" | "chunked" | "close-delimited"; + connection: "closed" | "reused"; + }; + + // Sends `request` and reads the response the way a client frames it. A + // body-less, Content-Length or chunked message is complete on its own, so + // once it is in, a second request probes whether the server kept the + // connection: either a second response arrives ("reused") or the server's + // FIN does ("closed"). With neither framing header the body is everything up + // to the FIN. + async function exchange(handler: (req: any, res: any) => void, request: string): Promise { + await using server = createServer(handler); + await once(server.listen(0, "127.0.0.1"), "listening"); + const { port } = server.address() as AddressInfo; + + const done = Promise.withResolvers(); + const socket = connect(port, "127.0.0.1", () => socket.write(request)); + let raw = ""; + let head: string | undefined; + let framing: Exchange["framing"] = "close-delimited"; + let firstMessageEnd = -1; + socket.on("data", (chunk: Buffer) => { + raw += chunk.toString("latin1"); + const headerEnd = raw.indexOf("\r\n\r\n"); + if (headerEnd === -1) return; + const bodyStart = headerEnd + 4; + if (head === undefined) { + head = raw.slice(0, headerEnd).replace(/^Date: .*$/m, "Date: "); + if (/^content-length:/im.test(head)) framing = "content-length"; + else if (/^transfer-encoding: .*chunked/im.test(head)) framing = "chunked"; + } + if (firstMessageEnd === -1) { + if (request.startsWith("HEAD ") || /^HTTP\/1\.1 (?:1\d\d|204|304) /.test(head)) { + // RFC 9112 6.3: ends at the blank line whatever the header fields say. + firstMessageEnd = bodyStart; + } else if (framing === "content-length") { + const end = bodyStart + Number(/^content-length: *(\d+)/im.exec(head)![1]); + if (raw.length >= end) firstMessageEnd = end; + } else if (framing === "chunked") { + const terminator = raw.indexOf("0\r\n\r\n", bodyStart); + if (terminator !== -1) firstMessageEnd = terminator + 5; + } + if (firstMessageEnd !== -1) socket.write("GET /probe HTTP/1.1\r\nHost: x\r\nConnection: close\r\n\r\n"); + } else if (raw.indexOf("HTTP/1.1 ", firstMessageEnd) === firstMessageEnd) { + done.resolve({ head: head!, body: raw.slice(bodyStart, firstMessageEnd), framing, connection: "reused" }); + socket.destroy(); + } + }); + const onClosed = () => { + const bodyStart = raw.indexOf("\r\n\r\n") + 4; + done.resolve({ + head: head ?? raw, + body: raw.slice(bodyStart, firstMessageEnd === -1 ? raw.length : firstMessageEnd), + framing, + connection: "closed", + }); + }; + // The probe written after the server's close may be answered with an RST. + socket.on("error", onClosed); + socket.on("close", onClosed); + // Settle before `server` is disposed: server.close() ends the open connection. + return await done.promise; + } + + const GET11 = "GET / HTTP/1.1\r\nHost: x\r\n\r\n"; + + test.concurrent.each([ + [ + "write() + end()", + (res: any) => { + res.write("hel"); + res.end("lo"); + }, + ], + ["end(data) with a known length", (res: any) => res.end("hello")], + [ + "flushHeaders() first", + (res: any) => { + res.flushHeaders(); + res.write("hel"); + res.end("lo"); + }, + ], + [ + "writeHead() first", + (res: any) => { + res.writeHead(200); + res.end("hello"); + }, + ], + ])("%s", async (_, respond) => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + respond(res); + }, GET11); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\nDate: \r\nConnection: close", + body: "hello", + framing: "close-delimited", + connection: "closed", + }); + }); + + test.concurrent("a user Connection: keep-alive header still gets a close-delimited body", async () => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + res.setHeader("connection", "keep-alive"); + res.write("hel"); + res.end("lo"); + }, GET11); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\nconnection: keep-alive\r\nDate: ", + body: "hello", + framing: "close-delimited", + connection: "closed", + }); + }); + + test.concurrent("a removed Connection header still gets a close-delimited body", async () => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + res.setHeader("connection", "keep-alive"); + res.removeHeader("connection"); + res.end("hello"); + }, GET11); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\nDate: ", + body: "hello", + framing: "close-delimited", + connection: "closed", + }); + }); + + // Node's shouldSendKeepAlive is `shouldKeepAlive && (contLen || useChunkedEncodingByDefault)`: + // an explicit Transfer-Encoding keeps its chunked framing but the connection + // is not reused, a body-less response closes too, and only an explicit + // Content-Length keeps the connection alive. + test.concurrent("an explicit Transfer-Encoding: chunked stays chunked but closes the connection", async () => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + res.setHeader("transfer-encoding", "chunked"); + res.write("hel"); + res.end("lo"); + }, GET11); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\ntransfer-encoding: chunked\r\nDate: \r\nConnection: close", + body: "3\r\nhel\r\n2\r\nlo\r\n0\r\n\r\n", + framing: "chunked", + connection: "closed", + }); + }); + + test.concurrent.each([ + ["HEAD", "HEAD / HTTP/1.1\r\nHost: x\r\n\r\n", (res: any) => res.end("hello"), "HTTP/1.1 200 OK"], + [ + "204", + GET11, + (res: any) => { + res.statusCode = 204; + res.end(); + }, + "HTTP/1.1 204 No Content", + ], + ])("a body-less %s response advertises and performs the close", async (_, request, respond, statusLine) => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + respond(res); + }, request); + expect(result).toEqual({ + head: `${statusLine}\r\nDate: \r\nConnection: close`, + body: "", + framing: "close-delimited", + connection: "closed", + }); + }); + + test.concurrent("an explicit Content-Length keeps the connection reusable", async () => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + res.setHeader("content-length", "5"); + res.end("hello"); + }, GET11); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\ncontent-length: 5\r\nDate: \r\nConnection: keep-alive\r\nKeep-Alive: timeout=5", + body: "hello", + framing: "content-length", + connection: "reused", + }); + }); + + // HTTP/1.0 clears the flag in the ServerResponse constructor, so the same + // branch applies: node writes no Content-Length even for a one-shot end(). + test.concurrent("HTTP/1.0 one-shot end(data) carries no Content-Length, like node", async () => { + const result = await exchange((req, res) => res.end("hello"), "GET / HTTP/1.0\r\nHost: x\r\n\r\n"); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\nDate: \r\nConnection: close", + body: "hello", + framing: "close-delimited", + connection: "closed", + }); + }); + + test.concurrent("write() followed by an empty end() is close-delimited too", async () => { + const result = await exchange((req, res) => { + res.useChunkedEncodingByDefault = false; + res.write("hello"); + res.end(); + }, GET11); + expect(result).toEqual({ + head: "HTTP/1.1 200 OK\r\nDate: \r\nConnection: close", + body: "hello", + framing: "close-delimited", + connection: "closed", + }); + }); + + // The body below is larger than the loopback socket buffers, so end() leaves + // part of it queued in the server while the pipelined request behind it is + // parsed from the same read. That request must not be answered (its bytes + // would land inside the close-delimited body) and the close must still come. + test.concurrent("a request pipelined behind a close-delimited response is not answered", async () => { + const body = Buffer.alloc(8 * 1024 * 1024, "a"); + await using server = createServer((req, res) => { + if (req.url === "/first") { + res.useChunkedEncodingByDefault = false; + res.end(body); + } else { + res.end("SECOND"); + } + }); + await once(server.listen(0, "127.0.0.1"), "listening"); + const { port } = server.address() as AddressInfo; + + const done = Promise.withResolvers<{ head: string; bodyLength: number; tail: string; closedByServer: boolean }>(); + const socket = connect(port, "127.0.0.1", () => { + socket.write("GET /first HTTP/1.1\r\nHost: x\r\n\r\nGET /second HTTP/1.1\r\nHost: x\r\n\r\n"); + }); + const chunks: Buffer[] = []; + let received = 0; + let headBytes = Buffer.alloc(0); + let headLength = -1; + const settle = (closedByServer: boolean) => { + const raw = Buffer.concat(chunks); + done.resolve({ + head: headBytes.toString("latin1").replace(/^Date: .*$/m, "Date: "), + bodyLength: raw.length - headLength, + tail: raw.subarray(raw.length - 8).toString("latin1"), + closedByServer, + }); + socket.destroy(); + }; + socket.on("data", (chunk: Buffer) => { + chunks.push(chunk); + received += chunk.length; + if (headLength === -1) { + const soFar = chunks.length === 1 ? chunk : Buffer.concat(chunks); + const headEnd = soFar.indexOf("\r\n\r\n"); + if (headEnd === -1) return; + headBytes = soFar.subarray(0, headEnd); + headLength = headEnd + 4; + } + // More than head + body can only be a second response inside the body. + if (received > headLength + body.length) settle(false); + }); + socket.on("end", () => settle(true)); + socket.on("error", done.reject); + + expect(await done.promise).toEqual({ + head: "HTTP/1.1 200 OK\r\nDate: \r\nConnection: close", + bodyLength: body.length, + tail: "aaaaaaaa", + closedByServer: true, + }); + }); +});