Commit 8d0c638
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: #23281
PR-URL: #25170
Reviewed-By: James M Snell <jasnell@gmail.com>1 parent ce7bbd2 commit 8d0c638
4 files changed
Lines changed: 78 additions & 4 deletions
File tree
- lib
- test/parallel
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -21,7 +21,7 @@ | |||
| 21 | 21 | ||
| 22 | 22 | 'use strict'; | |
| 23 | 23 | ||
| 24 | - const { Agent, globalAgent } = require('_http_agent'); | ||
| 24 | + const httpAgent = require('_http_agent'); | ||
| 25 | 25 | const { ClientRequest } = require('_http_client'); | |
| 26 | 26 | const { methods } = require('_http_common'); | |
| 27 | 27 | const { IncomingMessage } = require('_http_incoming'); | |
@@ -52,9 +52,8 @@ module.exports = { | |||
| 52 | 52 | _connectionListener, | |
| 53 | 53 | METHODS: methods.slice().sort(), | |
| 54 | 54 | STATUS_CODES, | |
| 55 | - Agent, | ||
| 55 | + Agent: httpAgent.Agent, | ||
| 56 | 56 | ClientRequest, | |
| 57 | - globalAgent, | ||
| 58 | 57 | IncomingMessage, | |
| 59 | 58 | OutgoingMessage, | |
| 60 | 59 | Server, | |
@@ -76,3 +75,14 @@ Object.defineProperty(module.exports, 'maxHeaderSize', { | |||
| 76 | 75 | return maxHeaderSize; | |
| 77 | 76 | } | |
| 78 | 77 | }); | |
| 78 | + | ||
| 79 | + Object.defineProperty(module.exports, 'globalAgent', { | ||
| 80 | + configurable: true, | ||
| 81 | + enumerable: true, | ||
| 82 | + get() { | ||
| 83 | + return httpAgent.globalAgent; | ||
| 84 | + }, | ||
| 85 | + set(value) { | ||
| 86 | + httpAgent.globalAgent = value; | ||
| 87 | + } | ||
| 88 | + }); | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -294,7 +294,7 @@ function request(...args) { | |||
| 294 | 294 | options = util._extend(options, args.shift()); | |
| 295 | 295 | } | |
| 296 | 296 | ||
| 297 | - options._defaultAgent = globalAgent; | ||
| 297 | + options._defaultAgent = module.exports.globalAgent; | ||
| 298 | 298 | args.unshift(options); | |
| 299 | 299 | ||
| 300 | 300 | return new ClientRequest(...args); | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,26 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const common = require('../common'); | ||
| 3 | + const assert = require('assert'); | ||
| 4 | + const http = require('http'); | ||
| 5 | + | ||
| 6 | + const server = http.Server(common.mustCall((req, res) => { | ||
| 7 | + res.writeHead(200); | ||
| 8 | + res.end('Hello, World!'); | ||
| 9 | + })); | ||
| 10 | + | ||
| 11 | + server.listen(0, common.mustCall(() => { | ||
| 12 | + const agent = new http.Agent(); | ||
| 13 | + const name = agent.getName({ port: server.address().port }); | ||
| 14 | + http.globalAgent = agent; | ||
| 15 | + | ||
| 16 | + makeRequest(); | ||
| 17 | + assert(agent.sockets.hasOwnProperty(name)); // agent has indeed been used | ||
| 18 | + })); | ||
| 19 | + | ||
| 20 | + function makeRequest() { | ||
| 21 | + const req = http.get({ | ||
| 22 | + port: server.address().port | ||
| 23 | + }); | ||
| 24 | + req.on('close', () => | ||
| 25 | + server.close()); | ||
| 26 | + } | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -0,0 +1,38 @@ | |||
| 1 | + 'use strict'; | ||
| 2 | + const common = require('../common'); | ||
| 3 | + const fixtures = require('../common/fixtures'); | ||
| 4 | + const assert = require('assert'); | ||
| 5 | + const https = require('https'); | ||
| 6 | + | ||
| 7 | + if (!common.hasCrypto) | ||
| 8 | + common.skip('missing crypto'); | ||
| 9 | + | ||
| 10 | + // Disable strict server certificate validation by the client | ||
| 11 | + process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0'; | ||
| 12 | + | ||
| 13 | + const options = { | ||
| 14 | + key: fixtures.readKey('agent1-key.pem'), | ||
| 15 | + cert: fixtures.readKey('agent1-cert.pem') | ||
| 16 | + }; | ||
| 17 | + | ||
| 18 | + const server = https.Server(options, common.mustCall((req, res) => { | ||
| 19 | + res.writeHead(200); | ||
| 20 | + res.end('Hello, World!'); | ||
| 21 | + })); | ||
| 22 | + | ||
| 23 | + server.listen(0, common.mustCall(() => { | ||
| 24 | + const agent = new https.Agent(); | ||
| 25 | + const name = agent.getName({ port: server.address().port }); | ||
| 26 | + https.globalAgent = agent; | ||
| 27 | + | ||
| 28 | + makeRequest(); | ||
| 29 | + assert(agent.sockets.hasOwnProperty(name)); // agent has indeed been used | ||
| 30 | + })); | ||
| 31 | + | ||
| 32 | + function makeRequest() { | ||
| 33 | + const req = https.get({ | ||
| 34 | + port: server.address().port | ||
| 35 | + }); | ||
| 36 | + req.on('close', () => | ||
| 37 | + server.close()); | ||
| 38 | + } | ||
0 commit comments