From f60c692499c488d4ab59b67b258eea88dc826287 Mon Sep 17 00:00:00 2001
From: Luigi Pinca <luigipinca@gmail.com>
Date: Fri, 3 Nov 2017 18:13:48 +0100
Subject: [PATCH] http: use 'connect' event only if socket is connecting

Fixes a bug that prevented `ClientRequest.prototype.setTimeout()` from
working properly when the socket was reused for multiple requests.

Fixes: https://github.com/nodejs/node/issues/16716
Refs: https://github.com/nodejs/node/pull/8895
PR-URL: https://github.com/nodejs/node/pull/16725
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
---
 lib/_http_client.js                           |  8 +++-
 ...st-http-client-timeout-connect-listener.js | 42 +++++++++++++++++++
 2 files changed, 48 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-http-client-timeout-connect-listener.js

diff --git a/lib/_http_client.js b/lib/_http_client.js
index fedadedcc2e657..df99daa9d54043 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -728,9 +728,13 @@ ClientRequest.prototype.setTimeout = function setTimeout(msecs, callback) {
   }
 
   this.once('socket', function(sock) {
-    sock.once('connect', function() {
+    if (sock.connecting) {
+      sock.once('connect', function() {
+        sock.setTimeout(msecs, emitTimeout);
+      });
+    } else {
       sock.setTimeout(msecs, emitTimeout);
-    });
+    }
   });
 
   return this;
diff --git a/test/parallel/test-http-client-timeout-connect-listener.js b/test/parallel/test-http-client-timeout-connect-listener.js
new file mode 100644
index 00000000000000..ea09aff718d557
--- /dev/null
+++ b/test/parallel/test-http-client-timeout-connect-listener.js
@@ -0,0 +1,42 @@
+'use strict';
+const common = require('../common');
+
+// This test ensures that `ClientRequest.prototype.setTimeout()` does
+// not add a listener for the `'connect'` event to the socket if the
+// socket is already connected.
+
+const assert = require('assert');
+const http = require('http');
+
+// Maximum allowed value for timeouts.
+const timeout = 2 ** 31 - 1;
+
+const server = http.createServer((req, res) => {
+  res.end();
+});
+
+server.listen(0, common.mustCall(() => {
+  const agent = new http.Agent({ keepAlive: true, maxSockets: 1 });
+  const options = { port: server.address().port, agent: agent };
+
+  doRequest(options, common.mustCall(() => {
+    const req = doRequest(options, common.mustCall(() => {
+      agent.destroy();
+      server.close();
+    }));
+
+    req.on('socket', common.mustCall((socket) => {
+      assert.strictEqual(socket.listenerCount('connect'), 0);
+    }));
+  }));
+}));
+
+function doRequest(options, callback) {
+  const req = http.get(options, (res) => {
+    res.on('end', callback);
+    res.resume();
+  });
+
+  req.setTimeout(timeout);
+  return req;
+}