http: check for handle before running asyncReset()

If an uninitialized or user supplied Socket is in the freeSockets list
of the Agent it would automatically attempt to run
._handle.asyncReset(), but would throw from those not existing. Guard
against that by first checking that they exist.

PR-URL: https://github.com/nodejs/node/pull/14419
Fixes: https://github.com/nodejs/node/issues/13539
Refs: https://github.com/nodejs/node/issues/13352
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>

3 files changed, 61 insertions, 3 deletions

diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index 426cf5b502..ddd36c158e 100644
--- a/lib/_http_agent.js
+++ b/lib/_http_agent.js
@@ -167,9 +167,12 @@ Agent.prototype.addRequest = function addRequest(req, options, port/*legacy*/,
   if (freeLen) {
     // we have a free socket, so use that.
     var socket = this.freeSockets[name].shift();
-    // Assign the handle a new asyncId and run any init() hooks.
-    socket._handle.asyncReset();
-    socket[async_id_symbol] = socket._handle.getAsyncId();
+    // Guard against an uninitialized or user supplied Socket.
+    if (socket._handle && typeof socket._handle.asyncReset === 'function') {
+      // Assign the handle a new asyncId and run any init() hooks.
+      socket._handle.asyncReset();
+      socket[async_id_symbol] = socket._handle.getAsyncId();
+    }
 
     // don't leak
     if (!this.freeSockets[name].length)
diff --git a/test/parallel/test-http-agent-uninitialized-with-handle.js b/test/parallel/test-http-agent-uninitialized-with-handle.js
new file mode 100644
index 0000000000..fab32ade45
--- /dev/null
+++ b/test/parallel/test-http-agent-uninitialized-with-handle.js
@@ -0,0 +1,30 @@
+'use strict';
+
+const common = require('../common');
+const http = require('http');
+const net = require('net');
+
+const agent = new http.Agent({
+  keepAlive: true,
+});
+const socket = new net.Socket();
+// If _handle exists then internals assume a couple methods exist.
+socket._handle = {
+  ref() { },
+  readStart() { },
+};
+const req = new http.ClientRequest(`http://localhost:${common.PORT}/`);
+
+const server = http.createServer(common.mustCall((req, res) => {
+  res.end();
+})).listen(common.PORT, common.mustCall(() => {
+  // Manually add the socket without a _handle.
+  agent.freeSockets[agent.getName(req)] = [socket];
+  // Now force the agent to use the socket and check that _handle exists before
+  // calling asyncReset().
+  agent.addRequest(req, {});
+  req.on('response', common.mustCall(() => {
+    server.close();
+  }));
+  req.end();
+}));
diff --git a/test/parallel/test-http-agent-uninitialized.js b/test/parallel/test-http-agent-uninitialized.js
new file mode 100644
index 0000000000..c522b5fdbd
--- /dev/null
+++ b/test/parallel/test-http-agent-uninitialized.js
@@ -0,0 +1,25 @@
+'use strict';
+
+const common = require('../common');
+const http = require('http');
+const net = require('net');
+
+const agent = new http.Agent({
+  keepAlive: true,
+});
+const socket = new net.Socket();
+const req = new http.ClientRequest(`http://localhost:${common.PORT}/`);
+
+const server = http.createServer(common.mustCall((req, res) => {
+  res.end();
+})).listen(common.PORT, common.mustCall(() => {
+  // Manually add the socket without a _handle.
+  agent.freeSockets[agent.getName(req)] = [socket];
+  // Now force the agent to use the socket and check that _handle exists before
+  // calling asyncReset().
+  agent.addRequest(req, {});
+  req.on('response', common.mustCall(() => {
+    server.close();
+  }));
+  req.end();
+}));