From d6a90b74cd174fc97118dc4e95966af7f5b10e53 Mon Sep 17 00:00:00 2001
From: Robert Nagy <ronagy@icloud.com>
Date: Fri, 24 Apr 2026 20:28:49 +0200
Subject: [PATCH] http: emit 'drain' on OutgoingMessage only after buffers
 drain

Previously, socketOnDrain could be invoked synchronously from
_flushOutput (via _onPendingData -> updateOutgoingData) while the bytes
just handed to the socket were still buffered and while outputSize had
not yet been reset on the OutgoingMessage. The 'drain' event fired even
though res.writableLength was non-zero, breaking the invariant a user
would reasonably expect after `while (!res.write(...));`.

Gate the emission in socketOnDrain on msg.writableLength === 0 (which
also covers outputSize + chunked buffer + socket.writableLength), and
apply the same check in OutgoingMessage._flush so that 'drain' is only
emitted when the response is genuinely drained. The socket's own
'drain' event will otherwise propagate through socketOnDrain when the
socket buffer actually empties.

Signed-off-by: Robert Nagy <ronagy@icloud.com>
Assisted-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 lib/_http_outgoing.js                         |  4 +-
 lib/_http_server.js                           |  8 ++-
 ...est-http-outgoing-drain-writable-length.js | 58 +++++++++++++++++++
 3 files changed, 67 insertions(+), 3 deletions(-)
 create mode 100644 test/parallel/test-http-outgoing-drain-writable-length.js

diff --git a/lib/_http_outgoing.js b/lib/_http_outgoing.js
index 24aae1caca69d3..fa9e38ae7481e9 100644
--- a/lib/_http_outgoing.js
+++ b/lib/_http_outgoing.js
@@ -1139,12 +1139,12 @@ OutgoingMessage.prototype._flush = function _flush() {
 
   if (socket?.writable) {
     // There might be remaining data in this.output; write it out
-    const ret = this._flushOutput(socket);
+    this._flushOutput(socket);
 
     if (this.finished) {
       // This is a queue to the server or client to bring in the next this.
       this._finish();
-    } else if (ret && this[kNeedDrain]) {
+    } else if (this[kNeedDrain] && this.writableLength === 0) {
       this[kNeedDrain] = false;
       this.emit('drain');
     }
diff --git a/lib/_http_server.js b/lib/_http_server.js
index 4da10dd1edf133..24ca21606316ad 100644
--- a/lib/_http_server.js
+++ b/lib/_http_server.js
@@ -817,7 +817,13 @@ function socketOnDrain(socket, state) {
   }
 
   const msg = socket._httpMessage;
-  if (msg && !msg.finished && msg[kNeedDrain]) {
+  // Only emit 'drain' once the message has no data pending anywhere, so that
+  // msg.writableLength === 0 when the event fires. socketOnDrain is called
+  // synchronously from updateOutgoingData during _flushOutput, at which point
+  // the bytes we just handed to the socket (or the stale outputSize) mean
+  // the message is not actually drained yet - we wait for the socket's
+  // own 'drain' event instead.
+  if (msg && !msg.finished && msg[kNeedDrain] && msg.writableLength === 0) {
     msg[kNeedDrain] = false;
     msg.emit('drain');
   }
diff --git a/test/parallel/test-http-outgoing-drain-writable-length.js b/test/parallel/test-http-outgoing-drain-writable-length.js
new file mode 100644
index 00000000000000..39bb0a9447676d
--- /dev/null
+++ b/test/parallel/test-http-outgoing-drain-writable-length.js
@@ -0,0 +1,58 @@
+'use strict';
+// Regression test: when a pipelined ServerResponse (whose writes were
+// buffered in outputData while the socket belonged to a previous response)
+// is finally assigned its socket and flushed, 'drain' must not be emitted
+// until the socket's own buffer has actually drained. Previously,
+// socketOnDrain was called synchronously from _flushOutput via _onPendingData
+// and emitted 'drain' even though the bytes we just wrote were still sitting
+// in the socket's writable buffer, so res.writableLength was non-zero.
+
+const common = require('../common');
+const http = require('http');
+const net = require('net');
+const assert = require('assert');
+
+let step = 0;
+
+const server = http.createServer(common.mustCall((req, res) => {
+  step++;
+
+  if (step === 1) {
+    // Keep the first response open briefly so the second is queued with
+    // res.socket === null.
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    setTimeout(() => res.end('ok'), 50);
+    return;
+  }
+
+  // Second (pipelined) response - queued in state.outgoing, no socket yet.
+  assert.strictEqual(res.socket, null);
+
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+
+  // Write past the response's highWaterMark so res.write() returns false
+  // and kNeedDrain is set. Data is buffered in outputData.
+  const chunk = Buffer.alloc(16 * 1024, 'x');
+  while (res.write(chunk));
+  assert.strictEqual(res.writableNeedDrain, true);
+
+  res.on('drain', common.mustCall(() => {
+    assert.strictEqual(
+      res.writableLength, 0,
+      `'drain' fired with writableLength=${res.writableLength}`,
+    );
+    res.end();
+    server.close();
+  }));
+}, 2));
+
+server.listen(0, common.mustCall(function() {
+  const port = this.address().port;
+  const client = net.connect(port);
+  client.write(
+    `GET /1 HTTP/1.1\r\nHost: localhost:${port}\r\n\r\n` +
+    `GET /2 HTTP/1.1\r\nHost: localhost:${port}\r\n\r\n`,
+  );
+  client.resume();
+  client.on('error', () => {});
+}));