Skip to content

Commit

Permalink
http: use objects with null prototype in Agent
Browse files Browse the repository at this point in the history
Fixes: #36364

PR-URL: #36409
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
targos committed Dec 13, 2020
1 parent c6c8337 commit 2ef9a76
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 19 deletions.
12 changes: 12 additions & 0 deletions doc/api/http.md
Expand Up @@ -262,6 +262,10 @@ terminates them.
### `agent.freeSockets`
<!-- YAML
added: v0.11.4
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36409
description: The property now has a `null` prototype.
-->

* {Object}
Expand Down Expand Up @@ -328,6 +332,10 @@ can have open. Unlike `maxSockets`, this parameter applies across all origins.
### `agent.requests`
<!-- YAML
added: v0.5.9
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36409
description: The property now has a `null` prototype.
-->

* {Object}
Expand All @@ -338,6 +346,10 @@ sockets. Do not modify.
### `agent.sockets`
<!-- YAML
added: v0.3.6
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/36409
description: The property now has a `null` prototype.
-->

* {Object}
Expand Down
16 changes: 9 additions & 7 deletions lib/_http_agent.js
Expand Up @@ -23,6 +23,7 @@

const {
NumberIsNaN,
ObjectCreate,
ObjectKeys,
ObjectSetPrototypeOf,
ObjectValues,
Expand Down Expand Up @@ -83,13 +84,13 @@ function Agent(options) {
this.defaultPort = 80;
this.protocol = 'http:';

this.options = { ...options };
this.options = { __proto__: null, ...options };

// Don't confuse net and make it think that we're connecting to a pipe
this.options.path = null;
this.requests = {};
this.sockets = {};
this.freeSockets = {};
this.requests = ObjectCreate(null);
this.sockets = ObjectCreate(null);
this.freeSockets = ObjectCreate(null);
this.keepAliveMsecs = this.options.keepAliveMsecs || 1000;
this.keepAlive = this.options.keepAlive || false;
this.maxSockets = this.options.maxSockets || Agent.defaultMaxSockets;
Expand Down Expand Up @@ -227,13 +228,14 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */,
// Legacy API: addRequest(req, host, port, localAddress)
if (typeof options === 'string') {
options = {
__proto__: null,
host: options,
port,
localAddress
};
}

options = { ...options, ...this.options };
options = { __proto__: null, ...options, ...this.options };
if (options.socketPath)
options.path = options.socketPath;

Expand Down Expand Up @@ -294,7 +296,7 @@ Agent.prototype.addRequest = function addRequest(req, options, port/* legacy */,
};

Agent.prototype.createSocket = function createSocket(req, options, cb) {
options = { ...options, ...this.options };
options = { __proto__: null, ...options, ...this.options };
if (options.socketPath)
options.path = options.socketPath;

Expand Down Expand Up @@ -435,7 +437,7 @@ Agent.prototype.removeSocket = function removeSocket(s, options) {
// There might be older requests in a different origin, but
// if the origin which releases the socket has pending requests
// that will be prioritized.
for (const prop in this.requests) {
for (const prop of ObjectKeys(this.requests)) {
// Check whether this specific origin is already at maxSockets
if (this.sockets[prop] && this.sockets[prop].length) break;
debug('removeSocket, have a request with different origin,' +
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-client-agent.js
Expand Up @@ -46,8 +46,8 @@ server.listen(0, common.mustCall(() => {
}));

const countdown = new Countdown(max, () => {
assert(!http.globalAgent.sockets.hasOwnProperty(name));
assert(!http.globalAgent.requests.hasOwnProperty(name));
assert(!(name in http.globalAgent.sockets));
assert(!(name in http.globalAgent.requests));
server.close();
});

Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-http-client-override-global-agent.js
Expand Up @@ -14,7 +14,7 @@ server.listen(0, common.mustCall(() => {
http.globalAgent = agent;

makeRequest();
assert(agent.sockets.hasOwnProperty(name)); // Agent has indeed been used
assert(name in agent.sockets); // Agent has indeed been used
}));

function makeRequest() {
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-connect-req-res.js
Expand Up @@ -43,8 +43,8 @@ server.listen(0, common.mustCall(function() {

// Make sure this request got removed from the pool.
const name = `localhost:${server.address().port}`;
assert(!http.globalAgent.sockets.hasOwnProperty(name));
assert(!http.globalAgent.requests.hasOwnProperty(name));
assert(!(name in http.globalAgent.sockets));
assert(!(name in http.globalAgent.requests));

// Make sure this socket has detached.
assert(!socket.ondata);
Expand Down
4 changes: 2 additions & 2 deletions test/parallel/test-http-connect.js
Expand Up @@ -70,8 +70,8 @@ server.listen(0, common.mustCall(() => {
req.on('connect', common.mustCall((res, socket, firstBodyChunk) => {
// Make sure this request got removed from the pool.
const name = `localhost:${server.address().port}`;
assert(!http.globalAgent.sockets.hasOwnProperty(name));
assert(!http.globalAgent.requests.hasOwnProperty(name));
assert(!(name in http.globalAgent.sockets));
assert(!(name in http.globalAgent.requests));

// Make sure this socket has detached.
assert(!socket.ondata);
Expand Down
6 changes: 3 additions & 3 deletions test/parallel/test-http-keep-alive.js
Expand Up @@ -59,14 +59,14 @@ server.listen(0, common.mustCall(function() {
}, common.mustCall((response) => {
response.on('end', common.mustCall(() => {
assert.strictEqual(agent.sockets[name].length, 1);
assert(!agent.requests.hasOwnProperty(name));
assert(!(name in agent.requests));
server.close();
}));
response.resume();
}));
}));

process.on('exit', () => {
assert(!agent.sockets.hasOwnProperty(name));
assert(!agent.requests.hasOwnProperty(name));
assert(!(name in agent.sockets));
assert(!(name in agent.requests));
});
2 changes: 1 addition & 1 deletion test/parallel/test-http-upgrade-agent.js
Expand Up @@ -78,7 +78,7 @@ server.listen(0, '127.0.0.1', common.mustCall(function() {
assert.deepStrictEqual(expectedHeaders, res.headers);

// Make sure this request got removed from the pool.
assert(!http.globalAgent.sockets.hasOwnProperty(name));
assert(!(name in http.globalAgent.sockets));

req.on('close', common.mustCall(function() {
socket.end();
Expand Down
2 changes: 1 addition & 1 deletion test/parallel/test-https-client-override-global-agent.js
Expand Up @@ -25,7 +25,7 @@ server.listen(0, common.mustCall(() => {
https.globalAgent = agent;

makeRequest();
assert(agent.sockets.hasOwnProperty(name)); // Agent has indeed been used
assert(name in agent.sockets); // Agent has indeed been used
}));

function makeRequest() {
Expand Down

0 comments on commit 2ef9a76

Please sign in to comment.