From 73230cc2c85b4c96eeb3a2a6a252a7b663982dd6 Mon Sep 17 00:00:00 2001 From: Roy Sommer Date: Fri, 21 Dec 2018 20:17:15 +0200 Subject: [PATCH] lib: support overriding http\s.globalAgent Overriding `require('http[s]').globalAgent` is now respected by consequent requests. In order to achieve that, the following changes were made: 1. Implmentation in `http`: `module.exports.globalAgent` is now defined through `Object.defineProperty`. Its getter and setter return \ set `require('_http_agent').globalAgent`. 2. Implementation in `https`: the https `globalAgent` is not the same as `_http_agent`, and is defined in `https` module itself. Therefore, the fix here was to simply use `module.exports.globalAgent` to support mutation. 3. According tests were added for both `http` and `https`, where in both we create a server, set the default agent to a newly created instance and make a request to that server. We then assert that the given instance was actually used by inspecting its sockets property. Fixes: https://github.com/nodejs/node/issues/23281 PR-URL: https://github.com/nodejs/node/pull/25170 Reviewed-By: James M Snell --- lib/http.js | 16 ++++++-- lib/https.js | 2 +- .../test-http-client-override-global-agent.js | 26 +++++++++++++ ...test-https-client-override-global-agent.js | 38 +++++++++++++++++++ 4 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-http-client-override-global-agent.js create mode 100644 test/parallel/test-https-client-override-global-agent.js diff --git a/lib/http.js b/lib/http.js index e3707ffa62b25d..5f2fe6dc68f571 100644 --- a/lib/http.js +++ b/lib/http.js @@ -21,7 +21,7 @@ 'use strict'; -const { Agent, globalAgent } = require('_http_agent'); +const httpAgent = require('_http_agent'); const { ClientRequest } = require('_http_client'); const { methods } = require('_http_common'); const { IncomingMessage } = require('_http_incoming'); @@ -52,9 +52,8 @@ module.exports = { _connectionListener, METHODS: methods.slice().sort(), STATUS_CODES, - Agent, + Agent: httpAgent.Agent, ClientRequest, - globalAgent, IncomingMessage, OutgoingMessage, Server, @@ -76,3 +75,14 @@ Object.defineProperty(module.exports, 'maxHeaderSize', { return maxHeaderSize; } }); + +Object.defineProperty(module.exports, 'globalAgent', { + configurable: true, + enumerable: true, + get() { + return httpAgent.globalAgent; + }, + set(value) { + httpAgent.globalAgent = value; + } +}); diff --git a/lib/https.js b/lib/https.js index 6f31315a559ef9..67a4648e3b70a2 100644 --- a/lib/https.js +++ b/lib/https.js @@ -283,7 +283,7 @@ function request(...args) { options = util._extend(options, args.shift()); } - options._defaultAgent = globalAgent; + options._defaultAgent = module.exports.globalAgent; args.unshift(options); return new ClientRequest(...args); diff --git a/test/parallel/test-http-client-override-global-agent.js b/test/parallel/test-http-client-override-global-agent.js new file mode 100644 index 00000000000000..c36c9d26a8dd63 --- /dev/null +++ b/test/parallel/test-http-client-override-global-agent.js @@ -0,0 +1,26 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const http = require('http'); + +const server = http.Server(common.mustCall((req, res) => { + res.writeHead(200); + res.end('Hello, World!'); +})); + +server.listen(0, common.mustCall(() => { + const agent = new http.Agent(); + const name = agent.getName({ port: server.address().port }); + http.globalAgent = agent; + + makeRequest(); + assert(agent.sockets.hasOwnProperty(name)); // agent has indeed been used +})); + +function makeRequest() { + const req = http.get({ + port: server.address().port + }); + req.on('close', () => + server.close()); +} diff --git a/test/parallel/test-https-client-override-global-agent.js b/test/parallel/test-https-client-override-global-agent.js new file mode 100644 index 00000000000000..12b5ae596bc837 --- /dev/null +++ b/test/parallel/test-https-client-override-global-agent.js @@ -0,0 +1,38 @@ +'use strict'; +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const assert = require('assert'); +const https = require('https'); + +if (!common.hasCrypto) + common.skip('missing crypto'); + +// Disable strict server certificate validation by the client +process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; + +const options = { + key: fixtures.readKey('agent1-key.pem'), + cert: fixtures.readKey('agent1-cert.pem') +}; + +const server = https.Server(options, common.mustCall((req, res) => { + res.writeHead(200); + res.end('Hello, World!'); +})); + +server.listen(0, common.mustCall(() => { + const agent = new https.Agent(); + const name = agent.getName({ port: server.address().port }); + https.globalAgent = agent; + + makeRequest(); + assert(agent.sockets.hasOwnProperty(name)); // agent has indeed been used +})); + +function makeRequest() { + const req = https.get({ + port: server.address().port + }); + req.on('close', () => + server.close()); +}