Skip to content

Commit ca01320

Browse files
committed
fix(core): respect NO_PROXY in global fetch dispatcher
1 parent 84423e6 commit ca01320

3 files changed

Lines changed: 137 additions & 13 deletions

File tree

packages/core/src/utils/fetch.test.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,16 @@ import * as dnsPromises from 'node:dns/promises';
1010
import type { LookupAddress, LookupAllOptions } from 'node:dns';
1111
import ipaddr from 'ipaddr.js';
1212

13-
const { setGlobalDispatcher, Agent, ProxyAgent } = vi.hoisted(() => ({
13+
const { setGlobalDispatcher, Agent, EnvHttpProxyAgent } = vi.hoisted(() => ({
1414
setGlobalDispatcher: vi.fn(),
1515
Agent: vi.fn(),
16-
ProxyAgent: vi.fn(),
16+
EnvHttpProxyAgent: vi.fn(),
1717
}));
1818

1919
vi.mock('undici', () => ({
2020
setGlobalDispatcher,
2121
Agent,
22-
ProxyAgent,
22+
EnvHttpProxyAgent,
2323
}));
2424

2525
vi.mock('node:dns/promises', () => ({
@@ -54,6 +54,7 @@ describe('fetch utils', () => {
5454
}
5555
return [{ address: '93.184.216.34', family: 4 }];
5656
});
57+
vi.unstubAllEnvs();
5758
});
5859

5960
afterEach(() => {
@@ -237,17 +238,36 @@ describe('fetch utils', () => {
237238
});
238239

239240
describe('setGlobalProxy', () => {
240-
it('should configure ProxyAgent with experiment flag timeout', () => {
241+
it('should configure EnvHttpProxyAgent with experiment flag timeout and noProxy', () => {
241242
const proxyUrl = 'http://proxy.example.com';
243+
const noProxyValue = 'localhost,127.0.0.1';
244+
vi.stubEnv('NO_PROXY', noProxyValue);
245+
242246
updateGlobalFetchTimeouts(45773134);
243247
setGlobalProxy(proxyUrl);
244248

245-
expect(ProxyAgent).toHaveBeenCalledWith({
246-
uri: proxyUrl,
249+
expect(EnvHttpProxyAgent).toHaveBeenCalledWith({
250+
httpProxy: proxyUrl,
251+
httpsProxy: proxyUrl,
252+
noProxy: noProxyValue,
247253
headersTimeout: 45773134,
248254
bodyTimeout: 300000,
249255
});
250256
expect(setGlobalDispatcher).toHaveBeenCalled();
251257
});
258+
259+
it('should fall back to no_proxy if NO_PROXY is not set', () => {
260+
const proxyUrl = 'http://proxy.example.com';
261+
const noProxyValue = 'localhost,127.0.0.1';
262+
vi.stubEnv('no_proxy', noProxyValue);
263+
264+
setGlobalProxy(proxyUrl);
265+
266+
expect(EnvHttpProxyAgent).toHaveBeenCalledWith(
267+
expect.objectContaining({
268+
noProxy: noProxyValue,
269+
}),
270+
);
271+
});
252272
});
253273
});

packages/core/src/utils/fetch.ts

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
import { getErrorMessage, isAbortError } from './errors.js';
88
import { URL } from 'node:url';
9-
import { Agent, ProxyAgent, setGlobalDispatcher } from 'undici';
9+
import { Agent, EnvHttpProxyAgent, setGlobalDispatcher } from 'undici';
1010
import ipaddr from 'ipaddr.js';
1111
import { lookup } from 'node:dns/promises';
1212

@@ -169,11 +169,14 @@ export async function isPrivateIpAsync(url: string): Promise<boolean> {
169169
}
170170

171171
/**
172-
* Creates an undici ProxyAgent that incorporates safe DNS lookup.
172+
* Creates an undici EnvHttpProxyAgent that incorporates safe DNS lookup.
173173
*/
174-
export function createSafeProxyAgent(proxyUrl: string): ProxyAgent {
175-
return new ProxyAgent({
176-
uri: proxyUrl,
174+
export function createSafeProxyAgent(proxyUrl: string): EnvHttpProxyAgent {
175+
const noProxy = process.env['NO_PROXY'] || process.env['no_proxy'];
176+
return new EnvHttpProxyAgent({
177+
httpProxy: proxyUrl,
178+
httpsProxy: proxyUrl,
179+
noProxy,
177180
});
178181
}
179182

@@ -221,9 +224,12 @@ export async function fetchWithTimeout(
221224

222225
export function setGlobalProxy(proxy: string) {
223226
currentProxy = proxy;
227+
const noProxy = process.env['NO_PROXY'] || process.env['no_proxy'];
224228
setGlobalDispatcher(
225-
new ProxyAgent({
226-
uri: proxy,
229+
new EnvHttpProxyAgent({
230+
httpProxy: proxy,
231+
httpsProxy: proxy,
232+
noProxy,
227233
headersTimeout: defaultHeadersTimeout,
228234
bodyTimeout: defaultBodyTimeout,
229235
}),
Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,98 @@
1+
/**
2+
* @license
3+
* Copyright 2025 Google LLC
4+
* SPDX-License-Identifier: Apache-2.0
5+
*/
6+
7+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
8+
import {
9+
EnvHttpProxyAgent,
10+
setGlobalDispatcher,
11+
getGlobalDispatcher,
12+
} from 'undici';
13+
import {
14+
setGlobalProxy,
15+
fetchWithTimeout,
16+
createSafeProxyAgent,
17+
} from './fetch.js';
18+
19+
describe('Proxy Bypass Integration', () => {
20+
const originalDispatcher = getGlobalDispatcher();
21+
22+
beforeEach(() => {
23+
vi.unstubAllEnvs();
24+
});
25+
26+
afterEach(() => {
27+
setGlobalDispatcher(originalDispatcher);
28+
vi.unstubAllEnvs();
29+
});
30+
31+
it('should bypass proxy for localhost when NO_PROXY is set', async () => {
32+
// We use a dummy proxy that will fail if any request is sent to it.
33+
const dummyProxy = 'http://1.2.3.4:5678';
34+
35+
// Set a dummy proxy and NO_PROXY for localhost
36+
vi.stubEnv('NO_PROXY', 'localhost');
37+
38+
// This will set the global dispatcher to a new EnvHttpProxyAgent
39+
setGlobalProxy(dummyProxy);
40+
41+
// We expect fetch to localhost to bypass the proxy.
42+
// Since we don't have a real localhost server running in this unit test,
43+
// we can't easily do a REAL fetch, but we can verify the dispatcher's behavior.
44+
45+
const dispatcher = getGlobalDispatcher();
46+
expect(dispatcher).toBeInstanceOf(EnvHttpProxyAgent);
47+
48+
// To do a real integration test, we would need a local server.
49+
// Let's start a small http server.
50+
51+
const http = await import('node:http');
52+
const server = http.createServer((req, res) => {
53+
res.end('ok');
54+
});
55+
56+
await new Promise<void>((resolve) =>
57+
server.listen(0, '127.0.0.1', () => resolve()),
58+
);
59+
const address = server.address();
60+
if (!address || typeof address === 'string') {
61+
throw new Error('Server address not found');
62+
}
63+
const port = address.port;
64+
const url = `http://127.0.0.1:${port}`;
65+
66+
try {
67+
// We need NO_PROXY to include 127.0.0.1 (or use localhost and fetch localhost)
68+
vi.stubEnv('NO_PROXY', `127.0.0.1,localhost`);
69+
setGlobalProxy(dummyProxy);
70+
71+
// If bypass works, this succeeds. If it tries to use the dummy proxy, it fails/times out.
72+
const response = await fetchWithTimeout(url, 1000);
73+
expect(await response.text()).toBe('ok');
74+
} finally {
75+
server.close();
76+
}
77+
});
78+
79+
it('should USE proxy for other domains when NO_PROXY is set to localhost', async () => {
80+
const dummyProxy = 'http://1.2.3.4:5678';
81+
vi.stubEnv('NO_PROXY', 'localhost');
82+
setGlobalProxy(dummyProxy);
83+
84+
// Fetching a public domain should try to use the proxy and fail (since the proxy is dummy).
85+
// We expect it to time out or fail with a connection error.
86+
await expect(fetchWithTimeout('http://example.com', 500)).rejects.toThrow();
87+
});
88+
89+
it('createSafeProxyAgent should return a proxy agent that respects NO_PROXY', () => {
90+
const proxyUrl = 'http://proxy.example.com';
91+
const noProxyValue = 'localhost,127.0.0.1';
92+
vi.stubEnv('NO_PROXY', noProxyValue);
93+
94+
const agent = createSafeProxyAgent(proxyUrl);
95+
expect(agent).toBeInstanceOf(EnvHttpProxyAgent);
96+
// Since we can't easily inspect internal state, this is mostly a type and presence check.
97+
});
98+
});

0 commit comments

Comments
 (0)