From 2ef9a76ece1e403d1dd7019fceb8f258607e7a69 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Zasso?= Date: Sun, 6 Dec 2020 14:52:08 +0100 Subject: [PATCH] http: use objects with null prototype in Agent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/nodejs/node/issues/36364 PR-URL: https://github.com/nodejs/node/pull/36409 Reviewed-By: Gerhard Stöbich Reviewed-By: Matteo Collina Reviewed-By: Rich Trott --- doc/api/http.md | 12 ++++++++++++ lib/_http_agent.js | 16 +++++++++------- test/parallel/test-http-client-agent.js | 4 ++-- .../test-http-client-override-global-agent.js | 2 +- test/parallel/test-http-connect-req-res.js | 4 ++-- test/parallel/test-http-connect.js | 4 ++-- test/parallel/test-http-keep-alive.js | 6 +++--- test/parallel/test-http-upgrade-agent.js | 2 +- .../test-https-client-override-global-agent.js | 2 +- 9 files changed, 33 insertions(+), 19 deletions(-) diff --git a/doc/api/http.md b/doc/api/http.md index 71203056b359f3..8851ae8d6e4893 100644 --- a/doc/api/http.md +++ b/doc/api/http.md @@ -262,6 +262,10 @@ terminates them. ### `agent.freeSockets` * {Object} @@ -328,6 +332,10 @@ can have open. Unlike `maxSockets`, this parameter applies across all origins. ### `agent.requests` * {Object} @@ -338,6 +346,10 @@ sockets. Do not modify. ### `agent.sockets` * {Object} diff --git a/lib/_http_agent.js b/lib/_http_agent.js index 6f6a0920fd8c04..10638fe69ed124 100644 --- a/lib/_http_agent.js +++ b/lib/_http_agent.js @@ -23,6 +23,7 @@ const { NumberIsNaN, + ObjectCreate, ObjectKeys, ObjectSetPrototypeOf, ObjectValues, @@ -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; @@ -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; @@ -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; @@ -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,' + diff --git a/test/parallel/test-http-client-agent.js b/test/parallel/test-http-client-agent.js index 3ae906f09c92ae..77616edd99d2fb 100644 --- a/test/parallel/test-http-client-agent.js +++ b/test/parallel/test-http-client-agent.js @@ -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(); }); diff --git a/test/parallel/test-http-client-override-global-agent.js b/test/parallel/test-http-client-override-global-agent.js index 0efaded5e419b0..f8b2928940e188 100644 --- a/test/parallel/test-http-client-override-global-agent.js +++ b/test/parallel/test-http-client-override-global-agent.js @@ -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() { diff --git a/test/parallel/test-http-connect-req-res.js b/test/parallel/test-http-connect-req-res.js index cd6e55e0898c66..dfb83a41aa6236 100644 --- a/test/parallel/test-http-connect-req-res.js +++ b/test/parallel/test-http-connect-req-res.js @@ -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); diff --git a/test/parallel/test-http-connect.js b/test/parallel/test-http-connect.js index 47fd7316d0cf1d..b6053ba9d45187 100644 --- a/test/parallel/test-http-connect.js +++ b/test/parallel/test-http-connect.js @@ -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); diff --git a/test/parallel/test-http-keep-alive.js b/test/parallel/test-http-keep-alive.js index 42d534e8e6b459..bd075230d16db1 100644 --- a/test/parallel/test-http-keep-alive.js +++ b/test/parallel/test-http-keep-alive.js @@ -59,7 +59,7 @@ 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(); @@ -67,6 +67,6 @@ server.listen(0, common.mustCall(function() { })); process.on('exit', () => { - assert(!agent.sockets.hasOwnProperty(name)); - assert(!agent.requests.hasOwnProperty(name)); + assert(!(name in agent.sockets)); + assert(!(name in agent.requests)); }); diff --git a/test/parallel/test-http-upgrade-agent.js b/test/parallel/test-http-upgrade-agent.js index cb33edc32e0139..28498819490b4f 100644 --- a/test/parallel/test-http-upgrade-agent.js +++ b/test/parallel/test-http-upgrade-agent.js @@ -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(); diff --git a/test/parallel/test-https-client-override-global-agent.js b/test/parallel/test-https-client-override-global-agent.js index a4bb2732756b2b..8774e77bb95d37 100644 --- a/test/parallel/test-https-client-override-global-agent.js +++ b/test/parallel/test-https-client-override-global-agent.js @@ -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() {