Skip to content
This repository has been archived by the owner on May 5, 2023. It is now read-only.

Commit

Permalink
Remove https core module patching logic
Browse files Browse the repository at this point in the history
Instead of patching over `https.request()` and `https.get()` manually
to add the `secureEndpoint` boolean, utilize the stack trace of the
current call logic to determine whether or not the `https.js` core
module is invoking `new ClientRequest()`.

In theory, this is still fragile logic, but should be more future-proof
and cleaner than patching over a core module, which is never really
desirable.

Fixes #29.
Closes #30.
Closes #35.
  • Loading branch information
TooTallNate committed Dec 2, 2019
1 parent 385b939 commit fccc703
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 71 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/package-lock.json
/yarn.lock
/node_modules
/?.js
/?.?s
/dist
16 changes: 14 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import './patch-core';
import net from 'net';
import http from 'http';
import promisify from 'es6-promisify';
Expand All @@ -12,6 +11,12 @@ function isHttpAgent(v: any): v is http.Agent {
return Boolean(v) && typeof v.addRequest === 'function';
}

function isSecureEndpoint(): boolean {
const { stack } = new Error();
if (typeof stack !== 'string') return false;
return stack.split('\n').some(l => l.indexOf('(https.js:') !== -1);
}

function createAgent(opts?: createAgent.AgentOptions): createAgent.Agent;
function createAgent(
callback: createAgent.AgentCallback,
Expand Down Expand Up @@ -105,6 +110,10 @@ namespace createAgent {
this.requests = [];
}

get defaultPort(): number {
return isSecureEndpoint() ? 443 : 80;
}

callback(
req: createAgent.ClientRequest,
opts: createAgent.RequestOptions,
Expand Down Expand Up @@ -136,7 +145,10 @@ namespace createAgent {
* @api public
*/
addRequest(req: ClientRequest, _opts: RequestOptions) {
const ownOpts: RequestOptions = { ..._opts };
const ownOpts: RequestOptions = {
..._opts,
secureEndpoint: isSecureEndpoint()
};

// Set default `host` for HTTP to localhost
if (ownOpts.host == null) {
Expand Down
57 changes: 0 additions & 57 deletions src/patch-core.ts

This file was deleted.

11 changes: 0 additions & 11 deletions test/test-legacy.js
Original file line number Diff line number Diff line change
Expand Up @@ -572,17 +572,6 @@ describe('"https" module', function() {
});
});

it('should not re-patch https.request', () => {
var patchModulePath = '../src/patch-core';
var patchedRequest = https.request;

delete require.cache[require.resolve(patchModulePath)];
require(patchModulePath);

assert.equal(patchedRequest, https.request);
assert.equal(true, https.request.__agent_base_https_request_patched__);
});

describe('PassthroughAgent', function() {
it('should pass through to `https.globalAgent`', function(done) {
// add HTTP server "request" listener
Expand Down

0 comments on commit fccc703

Please sign in to comment.