node:http: keep receiving the request body after the response has ended

packages/bun-uws/src/HttpResponse.h

@@ -864,6 +864,22 @@ struct HttpResponse : public AsyncSocket<SSL> {
         return !(httpResponseData->state & HttpResponseData<SSL>::HTTP_RESPONSE_PENDING);
     }
 
+    /* node:http compat: the response is complete, but the read that carried its
+     * request is still being parsed and a body handler was (re-)armed after the
+     * response ended, i.e. the request body is still being delivered out of the
+     * current buffer. A shutdown issued now makes the parser stop right after
+     * the request head (HttpContext's request hook bails on a shut-down socket)
+     * and drops that body; onData's post-parse close gate is the place to close
+     * such a connection, once the buffer has been consumed. */
+    bool isDeliveringBodyAfterResponse() {
+        HttpResponseData<SSL> *httpResponseData = getHttpResponseData();
+
+        return hasResponded()
+            && !httpResponseData->isConnectRequest
+            && httpResponseData->inStream != nullptr
+            && HttpContext<SSL>::fromSocket((us_socket_t *) this)->getSocketContextData()->parsingSocket == (us_socket_t *) this;
+    }
+
      /* Corks the response if possible. Leaves already corked socket be. */
     HttpResponse *cork(MoveOnlyFunction<void()> &&handler) {
         if (!Super::isCorked()) {

src/js/node/_http_incoming.ts

@@ -389,11 +389,10 @@ IncomingMessage.prototype._read = function _read(_n) {
 
   const bodyReadState = handle.hasBody;
 
-  if (
-    (bodyReadState & NodeHTTPBodyReadState.done) !== 0 ||
-    bodyReadState === NodeHTTPBodyReadState.none ||
-    this._dumped
-  ) {
+  // A dumped request (res.end() before the body was read) is not complete
+  // here: like Node, it only ends once the rest of its body has actually
+  // arrived, which onDataIncomingMessage reports with isLast.
+  if ((bodyReadState & NodeHTTPBodyReadState.done) !== 0 || bodyReadState === NodeHTTPBodyReadState.none) {
     emitEOFIncomingMessage(this);
   }
 
@@ -406,7 +405,6 @@ IncomingMessage.prototype._read = function _read(_n) {
 
   if (!handle.ondata) {
     handle.ondata = onDataIncomingMessage.bind(this);
-    handle.hasCustomOnData = false;
   }
 };
 
@@ -759,10 +757,9 @@ IncomingMessage.prototype._dump = function _dump() {
     // If there is buffered data, it may trigger 'data' events.
     // Remove 'data' event listeners explicitly.
     this.removeAllListeners("data");
-    const handle = this[kHandle];
-    if (handle) {
-      handle.ondata = undefined;
-    }
+    // The native ondata callback stays armed: onDataIncomingMessage drops the
+    // chunks of a dumped request and still delivers the body's fin, which is
+    // what completes the request (see _read).
     this.resume();
   }
 };

src/js/node/_http_server.ts

@@ -898,10 +898,12 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort
         handle.onabort = socket[kBoundOnAbort] ??= onServerRequestEvent.bind(socket);
         // Like Node's connectionListener -> parserOnBody: body bytes flow into
         // the IncomingMessage as they arrive, and the push callback readStop()s
-        // the socket (which emits 'pause' on it) once the buffer fills.
+        // the socket (which emits 'pause' on it) once the buffer fills. The
+        // callback stays armed until the body's fin chunk even if the response
+        // is ended first (native keeps delivering), so req 'end'/'close' and
+        // req.complete track the body actually being received, like Node.
         if (hasBody) {
           handle.ondata = onDataIncomingMessage.bind(http_req);
-          handle.hasCustomOnData = false;
         }
         drainMicrotasks();
 
@@ -2463,10 +2465,17 @@ function stopServerResponsePerf(this: any) {
 // arm keep-alive) runs first because onResponseFinishHandleSocket's guards
 // read pre-detach state, then detach the socket and advance the pipeline.
 function emitResponseFinish() {
+  const req = this.req;
+  // If the user never called req.read(), and didn't pipe() or
+  // .resume() or .on('data'), then we call req._dump() so that the
+  // bytes will be pulled off the wire.
+  if (req && !req._consuming && !req._readableState?.resumeScheduled) {
+    req._dump();
+  }
   // req.socket is nulled by the stream destroyer (pipeline/compose cleanup);
   // the response's own socket (set by assignSocket, cleared only by
   // detachSocket) still references the connection then.
-  const socket = this.req?.socket ?? this.socket;
+  const socket = req?.socket ?? this.socket;
   onResponseFinishHandleSocket(socket?.server, socket, this);
   // The dispatcher detached a synchronously-finished response itself;
   // advancing the pipeline again here would skip a queued response.
@@ -3248,10 +3257,9 @@ ServerResponse.prototype.end = function (chunk, encoding, callback) {
     }
   }
   this._header = " ";
-  const req = this.req;
-  if (!req._consuming && !req?._readableState?.resumeScheduled) {
-    req._dump();
-  }
+  // The unread-body dump decision is made on 'finish' (emitResponseFinish),
+  // like Node.js's resOnFinish: a consumer attached in the same tick as
+  // res.end() still gets the body.
   // The socket is NOT detached here: like Node.js, res.socket stays assigned
   // until the response 'finish' machinery runs (the dispatcher detaches it
   // right after a synchronously-finished handler returns, or via its 'finish'

src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp

@@ -244,12 +244,19 @@ bool JSNodeHTTPServerSocket::isClosed() const
 template<bool SSL>
 static bool deferShutdownUntilResponseDrains(us_socket_t* socket)
 {
-    if (reinterpret_cast<uWS::AsyncSocket<SSL>*>(socket)->getBufferedAmount() == 0) {
+    auto* response = reinterpret_cast<uWS::HttpResponse<SSL>*>(socket);
+    /* The second case is the 'finish' listener ending the connection of a
+     * handler that answered before its request body was read (the body is
+     * still being parsed out of the read that carried the request); Node parses
+     * everything it has read before resOnFinish's destroySoon() runs. */
+    if (reinterpret_cast<uWS::AsyncSocket<SSL>*>(socket)->getBufferedAmount() == 0 && !response->isDeliveringBodyAfterResponse()) {
         return false;
     }
-    /* HttpContext<SSL>::onWritable shuts the socket down once the buffered
-     * response data has flushed and HTTP_CONNECTION_CLOSE is set, so the FIN
-     * is sequenced after the response bytes (like Node's destroySoon). */
+    /* HttpContext<SSL>'s close gates (after the current parse, or from
+     * onWritable once the buffered response data has flushed) shut the socket
+     * down when HTTP_CONNECTION_CLOSE is set, so the FIN is sequenced after the
+     * response bytes and after the body bytes already read (like Node's
+     * destroySoon). */
     auto* httpResponseData = reinterpret_cast<uWS::HttpResponseData<SSL>*>(us_socket_ext(socket));
     httpResponseData->state |= uWS::HttpResponseData<SSL>::HTTP_CONNECTION_CLOSE;
     return true;

src/jsc/bindings/node/JSNodeHTTPServerSocket.h

@@ -105,7 +105,9 @@ class JSNodeHTTPServerSocket : public JSC::JSDestructibleObject {
 
     /* node:http socket.end(): when the in-flight response still has bytes in
      * uWS's send buffer, a shutdown now would put the FIN ahead of them and
-     * truncate the response. Returns true after handing the close to uWS. */
+     * truncate the response; when the finished response's request body is
+     * still being parsed out of the current read, it would drop that body.
+     * Returns true after handing the close to uWS in either case. */
     bool shutdownAfterResponseDrains();
 
     /* Switch the connection into CONNECT-style tunnel mode after an accepted