From 0413accc6b6e2b81784ab959b400236e4588b123 Mon Sep 17 00:00:00 2001
From: Owen Smith <owen@omsmith.ca>
Date: Tue, 28 Apr 2020 11:42:34 -0400
Subject: [PATCH] http: set default timeout in agent keepSocketAlive
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previous location of setting the timeout would override
behaviour of custom HttpAgents' keepSocketAlive. Moving
it into the default keepSocketAlive allows it to
interoperate with custom agents.

Fixes: https://github.com/nodejs/node/issues/33111

PR-URL: https://github.com/nodejs/node/pull/33127
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
---
 lib/_http_agent.js                       | 10 +++---
 test/parallel/test-http-agent-timeout.js | 40 ++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index 2618c6c3cb8223..3db42174d73004 100644
--- a/lib/_http_agent.js
+++ b/lib/_http_agent.js
@@ -138,11 +138,6 @@ function Agent(options) {
     socket._httpMessage = null;
     this.removeSocket(socket, options);
 
-    const agentTimeout = this.options.timeout || 0;
-    if (socket.timeout !== agentTimeout) {
-      socket.setTimeout(agentTimeout);
-    }
-
     socket.once('error', freeSocketErrorListener);
     freeSockets.push(socket);
   });
@@ -402,6 +397,11 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
   socket.setKeepAlive(true, this.keepAliveMsecs);
   socket.unref();
 
+  const agentTimeout = this.options.timeout || 0;
+  if (socket.timeout !== agentTimeout) {
+    socket.setTimeout(agentTimeout);
+  }
+
   return true;
 };
 
diff --git a/test/parallel/test-http-agent-timeout.js b/test/parallel/test-http-agent-timeout.js
index d8d34414d991a1..07c189e9745330 100644
--- a/test/parallel/test-http-agent-timeout.js
+++ b/test/parallel/test-http-agent-timeout.js
@@ -92,3 +92,43 @@ const http = require('http');
       }));
   }));
 }
+
+{
+  // Ensure custom keepSocketAlive timeout is respected
+
+  const CUSTOM_TIMEOUT = 60;
+  const AGENT_TIMEOUT = 50;
+
+  class CustomAgent extends http.Agent {
+    keepSocketAlive(socket) {
+      if (!super.keepSocketAlive(socket)) {
+        return false;
+      }
+
+      socket.setTimeout(CUSTOM_TIMEOUT);
+      return true;
+    }
+  }
+
+  const agent = new CustomAgent({ keepAlive: true, timeout: AGENT_TIMEOUT });
+
+  const server = http.createServer((req, res) => {
+    res.end();
+  });
+
+  server.listen(0, common.mustCall(() => {
+    http.get({ port: server.address().port, agent })
+      .on('response', common.mustCall((res) => {
+        const socket = res.socket;
+        assert(socket);
+        res.resume();
+        socket.on('free', common.mustCall(() => {
+          socket.on('timeout', common.mustCall(() => {
+            assert.strictEqual(socket.timeout, CUSTOM_TIMEOUT);
+            agent.destroy();
+            server.close();
+          }));
+        }));
+      }));
+  }));
+}