Repository: nodejs/node
Pull request: http: fix http client leaky with double response

Fixed: https://github.com/nodejs/node/issues/60025

When a request corresponds to multiple responses, ensure that only the first one is processed and skip all subsequent invalid responses (or emit a `response` event then emit an `error` event on request object ?).

- [x] `make -j4 test` (UNIX), or `vcbuild test` (Windows) passes
- [x] tests and/or benchmarks are included
- [x] documentation is changed or added
- [x] commit message follows [commit guidelines](https://github.com/nodejs/node/blob/master/doc/guides/contributing/pull-requests.md#commit-message-guidelines)


Changed files:
filename: lib/_http_client.js
patch: @@ -47,6 +47,7 @@ const {
   HTTPParser,
   isLenient,
   prepareError,
+  kSkipPendingData,
 } = require('_http_common');
 const {
   kUniqueHeaders,
@@ -692,7 +693,14 @@ function parserOnIncomingClient(res, shouldKeepAlive) {
     // We already have a response object, this means the server
     // sent a double response.
     socket.destroy();
-    return 0;  // No special treatment.
+    if (socket.parser) {
+      // https://github.com/nodejs/node/issues/60025
+      // Now, parser.incoming is pointed to the new IncomingMessage,
+      // we need to rewrite it to the first one and skip all the pending IncomingMessage
+      socket.parser.incoming = req.res;
+      socket.parser.incoming[kSkipPendingData] = true;
+    }
+    return 0;
   }
   req.res = res;
 

filename: lib/_http_common.js
patch: @@ -41,6 +41,7 @@ const {
 } = incoming;
 
 const kIncomingMessage = Symbol('IncomingMessage');
+const kSkipPendingData = Symbol('SkipPendingData');
 const kOnMessageBegin = HTTPParser.kOnMessageBegin | 0;
 const kOnHeaders = HTTPParser.kOnHeaders | 0;
 const kOnHeadersComplete = HTTPParser.kOnHeadersComplete | 0;
@@ -126,7 +127,7 @@ function parserOnBody(b) {
   const stream = this.incoming;
 
   // If the stream has already been removed, then drop it.
-  if (stream === null)
+  if (stream === null || stream[kSkipPendingData])
     return;
 
   // Pretend this was the result of a stream._read call.
@@ -141,7 +142,7 @@ function parserOnMessageComplete() {
   const parser = this;
   const stream = parser.incoming;
 
-  if (stream !== null) {
+  if (stream !== null && !stream[kSkipPendingData]) {
     stream.complete = true;
     // Emit any trailing headers.
     const headers = parser._headers;
@@ -310,4 +311,5 @@ module.exports = {
   HTTPParser,
   isLenient,
   prepareError,
+  kSkipPendingData,
 };

filename: test/parallel/test-http-client-leaky-with-double-response.js
patch: @@ -0,0 +1,44 @@
+'use strict';
+// Flags: --expose-gc
+const common = require('../common');
+const http = require('http');
+const assert = require('assert');
+const { onGC } = require('../common/gc');
+
+function createServer() {
+  const server = http.createServer(common.mustCall((req, res) => {
+    res.setHeader('Content-Type', 'application/json');
+    res.end(JSON.stringify({ hello: 'world' }));
+    req.socket.write('HTTP/1.1 400 Bad Request\r\n\r\n');
+  }));
+
+  return new Promise((resolve) => {
+    server.listen(0, common.mustCall(() => {
+      resolve(server);
+    }));
+  });
+}
+
+async function main() {
+  const server = await createServer();
+  const req = http.get({
+    port: server.address().port,
+  }, common.mustCall((res) => {
+    const chunks = [];
+    res.on('data', common.mustCallAtLeast((c) => chunks.push(c), 1));
+    res.on('end', common.mustCall(() => {
+      const body = Buffer.concat(chunks).toString('utf8');
+      const data = JSON.parse(body);
+      assert.strictEqual(data.hello, 'world');
+    }));
+  }));
+  const timer = setInterval(global.gc, 300);
+  onGC(req, {
+    ongc: common.mustCall(() => {
+      clearInterval(timer);
+      server.close();
+    })
+  });
+}
+
+main();