node:http: a request gets its body behind the FIN of socket.end() by robobun · Pull Request #44419 · oven-sh/bun

robobun · 2026-10-02T03:26:04Z

What does this PR do?

Fixes a node:http regression: a handler that rejects an upload early and then half-closes the socket never sees its request end.

http.createServer((req, res) => {
  req.on("data", () => {}).on("end", () => console.log("req end")).on("close", () => console.log("req close"));
  res.writeHead(401, { "content-length": 2 }); res.end("no");
  req.socket.end();
});

A raw client sends the POST head with Content-Length: 1000 and sends the body once it has seen the server's FIN.

	`req` events	body bytes
Node v22.22.0	`end`, `close`	1000
Bun 1.4.2	`end`, `close`	0
Bun 1.4.3, main	none, ever	0
this PR	`end`, `close`	1000

Cause. HttpContext::onData drops every read of a socket that is shut down, unless it is a tunnel. An http.Server socket has allowHalfOpen, so socket.end() only ends the write side. Since #43557 a request ends at its real last chunk, and that chunk is never read. The response has finished, so the close of the socket does not abort the request either.

Fix.

socket.end() marks the connection (HTTP_NODE_HALF_CLOSED, set in deferShutdownUntilResponseDrains). The three shut-down checks of HttpContext::onData let the body of the request in flight through (readsBodyBehindOwnFin). The last chunk stops the parse as before, so no other request is read behind the FIN.
HttpParser::hasIncompleteRequestBody() no longer counts a chunked body whose last chunk was delivered. A data handler that stops the parse there leaves STATE_IS_TRAILERS_DONE. Without this, https reports HPE_INVALID_EOF_STATE for a complete body and the connection never counts as idle.
onDataIncomingMessage: behind the FIN, a reader that is slower than the client pauses the reads like on any other connection. A body that nothing reads is dropped from the next read on, so the client's FIN still closes the connection (the existing server.close() test in this file needs that). The new finSent getter tells "the FIN has left" from "end() was called and the FIN waits for queued bytes", where nothing changes.
socket.destroySoon() goes through the same code. The request now gets the body that came in the read of its head, as in Node. The socket still closes behind that read.

Source change against main: +35/-8 lines.

Unchanged, and still different from Node:

socket.end() while response bytes are still queued (a response larger than the send buffer, or a client that does not read). With a finished response the connection closes at the flush and the body is lost. node:http: half-close from the drain gate when socket.end() waited for buffered bytes #44244 covers the case of an unfinished response. This PR no longer builds on it.
The body of an accepted Upgrade request is not read behind the FIN (node:http: an Upgrade request with a body follows Node v26 #45058).
A request that arrives behind the FIN is not dispatched. Node dispatches it, and its response cannot leave.
Over plain http, the peer's FIN behind ours closes the socket with no socket 'end' and no HPE_INVALID_EOF_STATE for a body that was cut short. https reports the cut as on main.
A reader that attaches after the buffer of req filled behind the FIN gets only the buffered part. Node pauses the reads and gives it all.

Changed as a consequence: a handler that ends the socket and never ends res (req.socket.end("HTTP/1.1 400 ...")). On main requestTimeout closes that connection, because the body is never read. Now the request completes, so only the client's FIN closes it, as in Node. With res.end() the keep-alive timeout applies as usual.

How did you verify your code works?

test/js/node/http/node-http-server-socket-end-drain.test.ts: 36 new tests, over http and https.

32 fail on a debug build of main's sources. The other 4 are guards for behavior that must not change: the unread body of an Upgrade request, and a late reader while the FIN waits.
All 45 tests of the file pass on this branch (debug build with ASAN, Linux x64).
Each clause of the fix was removed in turn. Every removal fails at least one test, except the HTTP_CONNECTION_SCOPED entry. That one only matters when a request is dispatched between end() and a FIN that a pending TLS write delays.
35 scenarios, the script above among them, ran under Node v22.22.0, main's sources and this branch, over http and https: Content-Length and chunked bodies, a body that starts in the read of the head, a body that is cut short, a malformed chunk, requestTimeout on a stalled body, server.close(), pipelined requests, CONNECT, Upgrade, Connection: close. What req emits and reads matches Node in every rejection scenario. The scenarios listed as unchanged give the same output as main.

Generated by Claude Code

…r buffered bytes The native end() of the server socket defers its shutdown while uWS holds unsent bytes. For destroySoon() the drain gate of HttpContext::onWritable closed the connection. For a plain end() on a response that never ends, no arm of the gate matched, so the FIN was never sent. The deferral now sets HTTP_NODE_SHUTDOWN_AFTER_DRAIN. When the bytes are out and a response is still in flight, the gate calls JSNodeHTTPServerSocket::halfClose(): the FIN goes out and the socket stays until the peer ends. If the peer ended first, the socket closes behind the FIN. The connection reads behind that FIN as it did before it: - HttpContext::onData keeps parsing, so the rest of a request body reaches the request. - The loop reports the FIN of the peer to HttpContext::onEnd before it closes the socket (us_socket_t::end_after_shutdown). The socket emits 'end', and a request that the FIN cuts gets its 'clientError'. - A pause of flood prevention ends at the FIN. Unsent bytes and the queue of responses can no longer end it. A request body that paused the reads keeps its pause. A response gets no 'drain' once its socket has ended, as in Node. A writer that continues on 'drain' can then no longer hold the FIN back.

… the one that waits The connection reads on behind the FIN of every socket.end(). A request gets the body that arrives there, and an Upgrade request with a body becomes a tunnel when that body ends. A body that nothing reads is dropped when the buffer of the request is full, so that the peer's FIN closes the connection.

…ing queued

…top the reads for ever

robobun · 2026-10-02T04:09:21Z

Status

This pull request is a draft while it is reworked. Its current head (ac65334ac9) is stale: it is stacked on #44244, and a check found that it can reset a connection whose response is still queued.

The rework is narrower. It keeps one rule: on a node:http server, socket.end() does the same thing whether or not bytes are still queued, only later. The FIN follows the queued bytes, also when the response never ends, and no close follows that FIN. server.close() completes for a connection behind its own FIN. The change of #44244 is folded in, and the base becomes main.

Out of this pull request: reads behind the FIN and the Upgrade request with a body (#45058 covers that), and the request timeout window behind the FIN.

I will replace this comment with the reproduction and the measured outputs when the rework is pushed.

claude

Findings marked 🟡 are optional suggestions and need no follow-up push.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

🟡 src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp — A handler that paused a request body and then calls socket.end() receives up to one extra 512 KB read pushed past the 16 KB highWaterMark, where the base delivered nothing after the FIN. halfClose at src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp:391-401 calls us_socket_pause then us_socket_resume unconditionally for the immediate path (keepReadPause false at JSNodeHTTPServerSocketPrototype.cpp:314), lifting the body pause set at src/js/internal/http.ts:200-203, and the next read is parsed because readsBehindOwnFin is now true. Fix: pass keepReadPause true (or skip the resume when socket->flags.is_paused was set by a body pause) on the nothing-queued end() path, as halfCloseAfterDrain already does.

Why this was flagged

Trigger: a request body larger than the IncomingMessage highWaterMark; push() fails at src/js/internal/http.ts:190 and the socket is paused at :200-203. The handler then calls req.socket.end() with nothing queued. jsFunctionNodeHTTPServerSocketEnd at JSNodeHTTPServerSocketPrototype.cpp:314 calls halfClose(globalObject, false); cycleReads at JSNodeHTTPServerSocket.cpp:391 is true, so :401 calls us_socket_resume and clears flags.is_paused. On the base the same resume happened but HttpContext.h:342 dropped every read behind the FIN. On this branch deferShutdownUntilResponseDrains at :356 set HTTP_NODE_SHUTDOWN_AFTER_DRAIN first, so the next read (up to the 512 KB read buffer) is parsed and pushed into a stream whose consumer asked for backpressure; only then does :195-203 re-pause or stopParsing. The deferred-FIN path avoids this with keepReadPause true at :1116. Remedy: pass keepReadPause true from the nothing-queued end() path as well.

Verification: The request body already filled the IncomingMessage buffer and the handler calls req.socket.end() with nothing queued. jsFunctionNodeHTTPServerSocketEnd (JSNodeHTTPServerSocketPrototype.cpp:314) calls halfClose(globalObject, false); cycleReads at JSNodeHTTPServerSocket.cpp:391 is true, so :400 us_socket_resume(socket) clears is_paused and one more read (≤ 524288) is parsed into the already-full IncomingMessage.
🟡 packages/bun-uws/src/HttpContext.h — On Windows a connection that stopped parsing mid-read keeps the event loop alive until the peer closes; the socket also stays in a cork slot. stopParsing() is now called from JavaScript inside a read (src/js/internal/http.ts:197 and src/js/node/_http_server.ts:969); when the same read holds a later request head, the request lambda returns nullptr at packages/bun-uws/src/HttpContext.h:474, so returnedData is null and the us_socket_unref and uncork at HttpContext.h:810/844 are skipped. The ref taken at the top of onData is uv_ref on libuv. Fix: when HTTP_NODE_PARSING_STOPPED ends a parse mid-read, run the same unref and uncork the closed-socket branch at HttpContext.h:459-460 runs before returning nullptr.

Why this was flagged

Trigger: a client sends a request whose body is larger than req's high-water mark together with a pipelined request head in one read, the handler calls req.socket.end() with nothing queued, and nothing reads the body. onDataIncomingMessage at src/js/internal/http.ts:195-197 sees finSent and calls socket[kHandle].stopParsing(), setting HTTP_NODE_PARSING_STOPPED. The parser then reaches the next head in the same read and the request lambda returns nullptr at packages/bun-uws/src/HttpContext.h:474 without the unref/uncork the other early return does at HttpContext.h:459-461. onData's returnedData is therefore null (HttpContext.h:796) and the us_socket_unref at :810 is skipped. On libuv (Windows) us_socket_ref is uv_ref, so this socket keeps the loop alive until it closes, which happens only at the peer's FIN (loop.c:927). On the base this nullptr return was reached only through onSocketCloseStopParsing, a path the user triggers explicitly; the diff makes it a normal consequence of socket.end() with an unread body.

Verification: packages/bun-uws/src/HttpContext.h:328 takes us_socket_ref(s) at the top of onData; the HTTP_NODE_PARSING_STOPPED check at lines 473-474 does a bare return nullptr; with no unref, so the us_socket_unref at line 810 is skipped. On Windows us_socket_ref/unref are uv_ref/uv_unref (packages/bun-usockets/src/socket.c:813-818, 858-863). The cork claim is wrong: line 902 uncorks on the nullptr path.

…er server.close() A full buffer drops the rest of a body behind the FIN only when nothing reads it. A reader gets backpressure, also after server.close().

…n flight and no other request A request that arrives behind the FIN gets no response, so it is not dispatched. A request body that nothing reads pauses the reads while the FIN waits and is dropped behind it. requestTimeout ends a connection that its peer does not end.

robobun · 2026-10-02T08:18:01Z

^{Updated 9:05 PM PT - Oct 21st, 2026}

@claude, your commit 967ab69 is building: #130459

claude

The two points from the earlier round (a slower-than-peer reader losing its body at server.close(), and the new cases running serially) are addressed by the later commits. Beyond the inline findings, I also checked the new us_socket_unref in the HTTP_NODE_PARSING_STOPPED early return of the request lambda in packages/bun-uws/src/HttpContext.h: it is the only nullptr return from that lambda that leaves the socket open without an unref, parsing stops there so it runs at most once per onData, and it mirrors the isDrainingBeforeClose path, so the onData ref stays balanced. I also checked that the requestTimeout re-arm in stopReadsBehindOwnFin is reachable only behind this side's FIN (both call sites gate on us_socket_is_shut_down plus HTTP_NODE_SHUTDOWN_AFTER_DRAIN), so ordinary keep-alive connections do not pick up the new ERR_HTTP_REQUEST_TIMEOUT.

Extended reasoning...

Findings-present ruled-out note: the onData ref/unref balance on the new early return and the reachability of the timeout re-arm were traced through HttpContext.h, HttpResponseData.h and JSNodeHTTPServerSocket.cpp and found consistent; the three confirmed inline findings stand on their own.

Findings marked 🟡 are optional suggestions and need no follow-up push.

github-actions · 2026-10-21T22:36:47Z

+// No `impl Default` on purpose: only `is_single_line`/`close_brace_loc` have
+// sensible defaults; `properties` does not, so every constructor must supply it.
