Skip to content

Commit

Permalink
lib: support overriding http\s.globalAgent
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
illBeRoy authored and MylesBorins committed May 16, 2019
1 parent f58dbb3 commit 73230cc
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 4 deletions.
16 changes: 13 additions & 3 deletions lib/http.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -52,9 +52,8 @@ module.exports = {
_connectionListener,
METHODS: methods.slice().sort(),
STATUS_CODES,
Agent,
Agent: httpAgent.Agent,
ClientRequest,
globalAgent,
IncomingMessage,
OutgoingMessage,
Server,
Expand All @@ -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;
}
});
2 changes: 1 addition & 1 deletion lib/https.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-http-client-override-global-agent.js
Original file line number Diff line number Diff line change
@@ -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());
}
38 changes: 38 additions & 0 deletions test/parallel/test-https-client-override-global-agent.js
Original file line number Diff line number Diff line change
@@ -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());
}

0 comments on commit 73230cc

Please sign in to comment.