From 93f47b11546b2117b555695b38f505ed2779b5c3 Mon Sep 17 00:00:00 2001 From: Trevor Norris Date: Fri, 21 Jul 2017 17:10:56 -0600 Subject: [PATCH] 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 Reviewed-By: James M Snell Reviewed-By: Refael Ackermann --- lib/_http_agent.js | 9 ++++-- ...st-http-agent-uninitialized-with-handle.js | 30 +++++++++++++++++++ .../parallel/test-http-agent-uninitialized.js | 25 ++++++++++++++++ 3 files changed, 61 insertions(+), 3 deletions(-) create mode 100644 test/parallel/test-http-agent-uninitialized-with-handle.js create mode 100644 test/parallel/test-http-agent-uninitialized.js diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 426cf5b502702a..ddd36c158ec3ca 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 00000000000000..fab32ade45efe4 --- /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 00000000000000..c522b5fdbd43e9 --- /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(); +}));