Skip to content

Commit b2c65f3

Browse files
authored
fix(devtools-proxy-support): remove ability to specify per-request CA (#449)
This is currently not supported by `https-proxy-agent`. Instead of trying to work around this, let's just keep it simple for now and always stick to the CA option provided to the agent (i.e. typically on the application level).
1 parent c532e58 commit b2c65f3

File tree

2 files changed

+12
-18
lines changed

2 files changed

+12
-18
lines changed

packages/devtools-proxy-support/src/agent.spec.ts

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ describe('createAgent', function () {
2121
new URL(url).protocol === 'https:' ? httpsGet : httpGet;
2222
const options = {
2323
agent,
24-
ca: setup.tlsOptions.ca,
2524
checkServerIdentity: () => undefined, // allow hostname mismatches
2625
...customOptions,
2726
};
@@ -142,7 +141,10 @@ describe('createAgent', function () {
142141
it('can connect to an https proxy without auth', async function () {
143142
const res = await get(
144143
'https://example.com/hello',
145-
createAgent({ proxy: `http://127.0.0.1:${setup.httpsProxyPort}` })
144+
createAgent({
145+
proxy: `http://127.0.0.1:${setup.httpsProxyPort}`,
146+
ca: setup.tlsOptions.ca,
147+
})
146148
);
147149
expect(res.body).to.equal('OK /hello');
148150
expect(setup.getRequestedUrls()).to.deep.equal([
@@ -157,6 +159,7 @@ describe('createAgent', function () {
157159
'https://example.com/hello',
158160
createAgent({
159161
proxy: `http://foo:[email protected]:${setup.httpsProxyPort}`,
162+
ca: setup.tlsOptions.ca,
160163
})
161164
);
162165
expect(res.body).to.equal('OK /hello');
@@ -173,6 +176,7 @@ describe('createAgent', function () {
173176
'https://example.com/hello',
174177
createAgent({
175178
proxy: `http://foo:[email protected]:${setup.httpsProxyPort}`,
179+
ca: setup.tlsOptions.ca,
176180
})
177181
);
178182
expect(res.statusCode).to.equal(407);
@@ -186,17 +190,6 @@ describe('createAgent', function () {
186190
resetSystemCACache();
187191
});
188192

189-
it('can connect using CA as part of the request options', async function () {
190-
// get() helpfully sets the CA for us
191-
const res = await get(
192-
'https://example.com/hello',
193-
createAgent({ proxy: `http://127.0.0.1:${setup.httpsProxyPort}` })
194-
);
195-
expect(res.body).to.equal('OK /hello');
196-
expect(setup.getRequestedUrls()).to.deep.equal([
197-
'http://example.com/hello',
198-
]);
199-
});
200193
it('can connect using CA as part of the agent options (no explicit CA set)', async function () {
201194
const res = await get(
202195
'https://example.com/hello',

packages/devtools-proxy-support/src/agent.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,14 @@ class DevtoolsProxyAgent extends ProxyAgent implements AgentWithInitialize {
4949
private _reqLockResolve: (() => void) | undefined;
5050

5151
constructor(proxyOptions: DevtoolsProxyOptions, logger: ProxyLogEmitter) {
52-
// We remove .ca because the Node.js HTTP agent implementation overrides
53-
// request options with agent options, but we want to merge them instead
52+
// NB: The Node.js HTTP agent implementation overrides request options
53+
// with agent options. Ideally, we'd want to merge them, but it seems like
54+
// there is little we can do about it at this point.
55+
// None of our products need the ability to specify per-request CA options
56+
// currently anyway.
5457
// https://github.com/nodejs/node/blob/014dad5953a632f44e668f9527f546c6e1bb8b86/lib/_http_agent.js#L239
55-
const { ca, ...proxyOptionsWithoutCA } = proxyOptions;
56-
void ca;
5758
super({
58-
...proxyOptionsWithoutCA,
59+
...proxyOptions,
5960
getProxyForUrl: (url: string) => this._getProxyForUrl(url),
6061
});
6162

0 commit comments

Comments
 (0)