Skip to content

Commit

Permalink
http: check for handle before running asyncReset()
Browse files Browse the repository at this point in the history
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: nodejs#14419
Fixes: nodejs#13539
Refs: nodejs#13352
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
  • Loading branch information
trevnorris authored and refack committed Jul 25, 2017
1 parent b0a8a7c commit 93f47b1
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 3 deletions.
9 changes: 6 additions & 3 deletions lib/_http_agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
30 changes: 30 additions & 0 deletions test/parallel/test-http-agent-uninitialized-with-handle.js
Original file line number Diff line number Diff line change
@@ -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();
}));
25 changes: 25 additions & 0 deletions test/parallel/test-http-agent-uninitialized.js
Original file line number Diff line number Diff line change
@@ -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();
}));

0 comments on commit 93f47b1

Please sign in to comment.