Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http: use objects with null prototype in Agent #36409

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions doc/api/http.md
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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