Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(EnvHttpProxyAgent): prefer lowercase env vars #3152

Merged
merged 1 commit into from
Apr 23, 2024
Merged

fix(EnvHttpProxyAgent): prefer lowercase env vars #3152

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions docs/docs/api/EnvHttpProxyAgent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ Stability: Experimental.

Extends: `undici.Dispatcher`

EnvHttpProxyAgent automatically reads the proxy configuration from the environment variables `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` and sets up the proxy agents accordingly. When `HTTP_PROXY` and `HTTPS_PROXY` are set, `HTTP_PROXY` is used for HTTP requests and `HTTPS_PROXY` is used for HTTPS requests. If only `HTTP_PROXY` is set, `HTTP_PROXY` is used for both HTTP and HTTPS requests. If only `HTTPS_PROXY` is set, it is only used for HTTPS requests.
EnvHttpProxyAgent automatically reads the proxy configuration from the environment variables `http_proxy`, `https_proxy`, and `no_proxy` and sets up the proxy agents accordingly. When `http_proxy` and `https_proxy` are set, `http_proxy` is used for HTTP requests and `https_proxy` is used for HTTPS requests. If only `http_proxy` is set, `http_proxy` is used for both HTTP and HTTPS requests. If only `https_proxy` is set, it is only used for HTTPS requests.

`NO_PROXY` is a comma or space-separated list of hostnames that should not be proxied. The list may contain leading wildcard characters (`*`). If `NO_PROXY` is set, the EnvHttpProxyAgent will bypass the proxy for requests to hosts that match the list. If `NO_PROXY` is set to `"*"`, the EnvHttpProxyAgent will bypass the proxy for all requests.
`no_proxy` is a comma or space-separated list of hostnames that should not be proxied. The list may contain leading wildcard characters (`*`). If `no_proxy` is set, the EnvHttpProxyAgent will bypass the proxy for requests to hosts that match the list. If `no_proxy` is set to `"*"`, the EnvHttpProxyAgent will bypass the proxy for all requests.

Lower case environment variables are also supported: `http_proxy`, `https_proxy`, and `no_proxy`. However, if both the lower case and upper case environment variables are set, the lower case environment variables will be ignored.
Uppercase environment variables are also supported: `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY`. However, if both the lowercase and uppercase environment variables are set, the uppercase environment variables will be ignored.

## `new EnvHttpProxyAgent([options])`

Expand Down
6 changes: 3 additions & 3 deletions lib/dispatcher/env-http-proxy-agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,14 +32,14 @@ class EnvHttpProxyAgent extends DispatcherBase {

this[kNoProxyAgent] = new Agent(agentOpts)

const HTTP_PROXY = httpProxy ?? process.env.HTTP_PROXY ?? process.env.http_proxy
const HTTP_PROXY = httpProxy ?? process.env.http_proxy ?? process.env.HTTP_PROXY
if (HTTP_PROXY) {
this[kHttpProxyAgent] = new ProxyAgent({ ...agentOpts, uri: HTTP_PROXY })
} else {
this[kHttpProxyAgent] = this[kNoProxyAgent]
}

const HTTPS_PROXY = httpsProxy ?? process.env.HTTPS_PROXY ?? process.env.https_proxy
const HTTPS_PROXY = httpsProxy ?? process.env.https_proxy ?? process.env.HTTPS_PROXY
if (HTTPS_PROXY) {
this[kHttpsProxyAgent] = new ProxyAgent({ ...agentOpts, uri: HTTPS_PROXY })
} else {
Expand Down Expand Up @@ -153,7 +153,7 @@ class EnvHttpProxyAgent extends DispatcherBase {
}

get #noProxyEnv () {
return process.env.NO_PROXY ?? process.env.no_proxy ?? ''
return process.env.no_proxy ?? process.env.NO_PROXY ?? ''
}
}

Expand Down
110 changes: 55 additions & 55 deletions test/env-http-proxy-agent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ after(() => {
process.env = { ...env }
})

test('does not create any proxy agents if HTTP_PROXY and HTTPS_PROXY are not set', async (t) => {
test('does not create any proxy agents if http_proxy and https_proxy are not set', async (t) => {
t = tspl(t, { plan: 4 })
const dispatcher = new EnvHttpProxyAgent()
t.ok(dispatcher[kNoProxyAgent] instanceof Agent)
Expand All @@ -28,9 +28,9 @@ test('does not create any proxy agents if HTTP_PROXY and HTTPS_PROXY are not set
return dispatcher.close()
})

test('creates one proxy agent for both http and https when only HTTP_PROXY is defined', async (t) => {
test('creates one proxy agent for both http and https when only http_proxy is defined', async (t) => {
t = tspl(t, { plan: 5 })
process.env.HTTP_PROXY = 'http://example.com:8080'
process.env.http_proxy = 'http://example.com:8080'
const dispatcher = new EnvHttpProxyAgent()
t.ok(dispatcher[kNoProxyAgent] instanceof Agent)
t.ok(!(dispatcher[kNoProxyAgent] instanceof ProxyAgent))
Expand All @@ -40,10 +40,10 @@ test('creates one proxy agent for both http and https when only HTTP_PROXY is de
return dispatcher.close()
})

test('creates separate proxy agent for http and https when HTTP_PROXY and HTTPS_PROXY are set', async (t) => {
test('creates separate proxy agent for http and https when http_proxy and https_proxy are set', async (t) => {
t = tspl(t, { plan: 6 })
process.env.HTTP_PROXY = 'http://example.com:8080'
process.env.HTTPS_PROXY = 'http://example.com:8443'
process.env.http_proxy = 'http://example.com:8080'
process.env.https_proxy = 'http://example.com:8443'
const dispatcher = new EnvHttpProxyAgent()
t.ok(dispatcher[kNoProxyAgent] instanceof Agent)
t.ok(!(dispatcher[kNoProxyAgent] instanceof ProxyAgent))
Expand All @@ -54,10 +54,10 @@ test('creates separate proxy agent for http and https when HTTP_PROXY and HTTPS_
return dispatcher.close()
})

test('handles lowercase http_proxy and https_proxy', async (t) => {
test('handles uppercase HTTP_PROXY and HTTPS_PROXY', async (t) => {
t = tspl(t, { plan: 6 })
process.env.http_proxy = 'http://example.com:8080'
process.env.https_proxy = 'http://example.com:8443'
process.env.HTTP_PROXY = 'http://example.com:8080'
process.env.HTTPS_PROXY = 'http://example.com:8443'
const dispatcher = new EnvHttpProxyAgent()
t.ok(dispatcher[kNoProxyAgent] instanceof Agent)
t.ok(!(dispatcher[kNoProxyAgent] instanceof ProxyAgent))
Expand Down Expand Up @@ -100,34 +100,34 @@ test('prefers options over env vars', async (t) => {
return dispatcher.close()
})

test('prefers uppercase over lower case env vars', async (t) => {
test('prefers lowercase over uppercase env vars', async (t) => {
t = tspl(t, { plan: 2 })
process.env.http_proxy = 'http://lower.example.com:8080'
process.env.https_proxy = 'http://lower.example.com:8443'
process.env.HTTP_PROXY = 'http://upper.example.com:8080'
process.env.HTTPS_PROXY = 'http://upper.example.com:8443'
process.env.http_proxy = 'http://lower.example.com:8080'
process.env.https_proxy = 'http://lower.example.com:8443'
const dispatcher = new EnvHttpProxyAgent()
t.equal(dispatcher[kHttpProxyAgent][kProxy].uri, 'http://upper.example.com:8080/')
t.equal(dispatcher[kHttpsProxyAgent][kProxy].uri, 'http://upper.example.com:8443/')
t.equal(dispatcher[kHttpProxyAgent][kProxy].uri, 'http://lower.example.com:8080/')
t.equal(dispatcher[kHttpsProxyAgent][kProxy].uri, 'http://lower.example.com:8443/')
return dispatcher.close()
})

test('prefers uppercase over lower case env vars even when empty', async (t) => {
test('prefers lowercase over uppercase env vars even when empty', async (t) => {
t = tspl(t, { plan: 2 })
process.env.http_proxy = 'http://lower.example.com:8080'
process.env.https_proxy = 'http://lower.example.com:8443'
process.env.HTTP_PROXY = ''
process.env.HTTPS_PROXY = ''
process.env.HTTP_PROXY = 'http://upper.example.com:8080'
process.env.HTTP_PROXY = 'http://upper.example.com:8443'
process.env.http_proxy = ''
process.env.https_proxy = ''
const dispatcher = new EnvHttpProxyAgent()

t.deepStrictEqual(dispatcher[kHttpProxyAgent], dispatcher[kNoProxyAgent])
t.deepStrictEqual(dispatcher[kHttpsProxyAgent], dispatcher[kNoProxyAgent])
return dispatcher.close()
})

test('creates a proxy agent only for https when only HTTPS_PROXY is set', async (t) => {
test('creates a proxy agent only for https when only https_proxy is set', async (t) => {
t = tspl(t, { plan: 5 })
process.env.HTTPS_PROXY = 'http://example.com:8443'
process.env.https_proxy = 'http://example.com:8443'
const dispatcher = new EnvHttpProxyAgent()
t.ok(dispatcher[kNoProxyAgent] instanceof Agent)
t.ok(!(dispatcher[kNoProxyAgent] instanceof ProxyAgent))
Expand All @@ -139,8 +139,8 @@ test('creates a proxy agent only for https when only HTTPS_PROXY is set', async

test('closes all agents', async (t) => {
t = tspl(t, { plan: 3 })
process.env.HTTP_PROXY = 'http://example.com:8080'
process.env.HTTPS_PROXY = 'http://example.com:8443'
process.env.http_proxy = 'http://example.com:8080'
process.env.https_proxy = 'http://example.com:8443'
const dispatcher = new EnvHttpProxyAgent()
await dispatcher.close()
t.ok(dispatcher[kNoProxyAgent][kClosed])
Expand All @@ -150,8 +150,8 @@ test('closes all agents', async (t) => {

test('destroys all agents', async (t) => {
t = tspl(t, { plan: 3 })
process.env.HTTP_PROXY = 'http://example.com:8080'
process.env.HTTPS_PROXY = 'http://example.com:8443'
process.env.http_proxy = 'http://example.com:8080'
process.env.https_proxy = 'http://example.com:8443'
const dispatcher = new EnvHttpProxyAgent()
await dispatcher.destroy()
t.ok(dispatcher[kNoProxyAgent][kDestroyed])
Expand All @@ -170,8 +170,8 @@ const createEnvHttpProxyAgentWithMocks = (plan = 1, opts = {}) => {
}
return mockPool
}
process.env.HTTP_PROXY = 'http://localhost:8080'
process.env.HTTPS_PROXY = 'http://localhost:8443'
process.env.http_proxy = 'http://localhost:8080'
process.env.https_proxy = 'http://localhost:8443'
const dispatcher = new EnvHttpProxyAgent({ ...opts, factory })
const agentSymbols = [kNoProxyAgent, kHttpProxyAgent, kHttpsProxyAgent]
agentSymbols.forEach((agent) => {
Expand Down Expand Up @@ -201,10 +201,10 @@ test('uses the appropriate proxy for the protocol', async (t) => {
return dispatcher.close()
})

describe('NO_PROXY', () => {
describe('no_proxy', () => {
test('set to *', async (t) => {
t = tspl(t, { plan: 2 })
process.env.NO_PROXY = '*'
process.env.no_proxy = '*'
const { dispatcher, doesNotProxy } = createEnvHttpProxyAgentWithMocks(2)
t.ok(await doesNotProxy('https://example.com'))
t.ok(await doesNotProxy('http://example.com'))
Expand All @@ -213,39 +213,39 @@ describe('NO_PROXY', () => {

test('set but empty', async (t) => {
t = tspl(t, { plan: 1 })
process.env.NO_PROXY = ''
process.env.no_proxy = ''
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks()
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example.com'))
return dispatcher.close()
})

test('no entries (comma)', async (t) => {
t = tspl(t, { plan: 1 })
process.env.NO_PROXY = ','
process.env.no_proxy = ','
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks()
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example.com'))
return dispatcher.close()
})

test('no entries (whitespace)', async (t) => {
t = tspl(t, { plan: 1 })
process.env.NO_PROXY = ' '
process.env.no_proxy = ' '
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks()
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example.com'))
return dispatcher.close()
})

test('no entries (multiple whitespace / commas)', async (t) => {
t = tspl(t, { plan: 1 })
process.env.NO_PROXY = ',\t,,,\n, ,\r'
process.env.no_proxy = ',\t,,,\n, ,\r'
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks()
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example.com'))
return dispatcher.close()
})

test('single host', async (t) => {
t = tspl(t, { plan: 9 })
process.env.NO_PROXY = 'example'
process.env.no_proxy = 'example'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(9)
t.ok(await doesNotProxy('http://example'))
t.ok(await doesNotProxy('http://example:80'))
Expand Down Expand Up @@ -276,7 +276,7 @@ describe('NO_PROXY', () => {

test('subdomain', async (t) => {
t = tspl(t, { plan: 8 })
process.env.NO_PROXY = 'sub.example'
process.env.no_proxy = 'sub.example'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(8)
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example:80'))
Expand All @@ -291,7 +291,7 @@ describe('NO_PROXY', () => {

test('host + port', async (t) => {
t = tspl(t, { plan: 12 })
process.env.NO_PROXY = 'example:80, localhost:3000'
process.env.no_proxy = 'example:80, localhost:3000'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(12)
t.ok(await doesNotProxy('http://example'))
t.ok(await doesNotProxy('http://example:80'))
Expand All @@ -310,7 +310,7 @@ describe('NO_PROXY', () => {

test('host suffix', async (t) => {
t = tspl(t, { plan: 9 })
process.env.NO_PROXY = '.example'
process.env.no_proxy = '.example'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(9)
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example:80'))
Expand All @@ -326,7 +326,7 @@ describe('NO_PROXY', () => {

test('host suffix with *.', async (t) => {
t = tspl(t, { plan: 9 })
process.env.NO_PROXY = '*.example'
process.env.no_proxy = '*.example'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(9)
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example:80'))
Expand All @@ -342,7 +342,7 @@ describe('NO_PROXY', () => {

test('substring suffix', async (t) => {
t = tspl(t, { plan: 10 })
process.env.NO_PROXY = '*example'
process.env.no_proxy = '*example'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(10)
t.ok(await doesNotProxy('http://example'))
t.ok(await doesNotProxy('http://example:80'))
Expand All @@ -359,7 +359,7 @@ describe('NO_PROXY', () => {

test('arbitrary wildcards are NOT supported', async (t) => {
t = tspl(t, { plan: 6 })
process.env.NO_PROXY = '.*example'
process.env.no_proxy = '.*example'
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(6)
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://sub.example'))
Expand All @@ -372,7 +372,7 @@ describe('NO_PROXY', () => {

test('IP addresses', async (t) => {
t = tspl(t, { plan: 12 })
process.env.NO_PROXY = '[::1],[::2]:80,10.0.0.1,10.0.0.2:80'
process.env.no_proxy = '[::1],[::2]:80,10.0.0.1,10.0.0.2:80'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(12)
t.ok(await doesNotProxy('http://[::1]/'))
t.ok(await doesNotProxy('http://[::1]:80/'))
Expand All @@ -391,7 +391,7 @@ describe('NO_PROXY', () => {

test('CIDR is NOT supported', async (t) => {
t = tspl(t, { plan: 2 })
process.env.NO_PROXY = '127.0.0.1/32'
process.env.no_proxy = '127.0.0.1/32'
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(2)
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://127.0.0.1'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://127.0.0.1/32'))
Expand All @@ -400,7 +400,7 @@ describe('NO_PROXY', () => {

test('127.0.0.1 does NOT match localhost', async (t) => {
t = tspl(t, { plan: 2 })
process.env.NO_PROXY = '127.0.0.1'
process.env.no_proxy = '127.0.0.1'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(2)
t.ok(await doesNotProxy('http://127.0.0.1'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://localhost'))
Expand All @@ -409,7 +409,7 @@ describe('NO_PROXY', () => {

test('protocols that have a default port', async (t) => {
t = tspl(t, { plan: 6 })
process.env.NO_PROXY = 'xxx:21,xxx:70,xxx:80,xxx:443'
process.env.no_proxy = 'xxx:21,xxx:70,xxx:80,xxx:443'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(6)
t.ok(await doesNotProxy('http://xxx'))
t.ok(await doesNotProxy('http://xxx:80'))
Expand All @@ -420,9 +420,9 @@ describe('NO_PROXY', () => {
return dispatcher.close()
})

test('should not be case-sensitive', async (t) => {
test('should not be case sensitive', async (t) => {
t = tspl(t, { plan: 6 })
process.env.no_proxy = 'XXX YYY ZzZ'
process.env.NO_PROXY = 'XXX YYY ZzZ'
const { dispatcher, doesNotProxy } = createEnvHttpProxyAgentWithMocks(6)
t.ok(await doesNotProxy('http://xxx'))
t.ok(await doesNotProxy('http://XXX'))
Expand All @@ -433,32 +433,32 @@ describe('NO_PROXY', () => {
return dispatcher.close()
})

test('prefers uppercase over lower case', async (t) => {
test('prefers lowercase over uppercase', async (t) => {
t = tspl(t, { plan: 2 })
process.env.no_proxy = 'sub.example.com'
process.env.NO_PROXY = 'example.com'
process.env.NO_PROXY = 'sub.example.com'
process.env.no_proxy = 'example.com'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(6)
t.ok(await doesNotProxy('http://example.com'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://sub.example.com'))
return dispatcher.close()
})

test('prefers uppercase over lower case even when it is empty', async (t) => {
test('prefers lowercase over uppercase even when it is empty', async (t) => {
t = tspl(t, { plan: 1 })
process.env.no_proxy = 'example.com'
process.env.NO_PROXY = ''
process.env.NO_PROXY = 'example.com'
process.env.no_proxy = ''
const { dispatcher, usesProxyAgent } = createEnvHttpProxyAgentWithMocks()
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example.com'))
return dispatcher.close()
})

test('handles env var changes', async (t) => {
t = tspl(t, { plan: 4 })
process.env.NO_PROXY = 'example.com'
process.env.no_proxy = 'example.com'
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(4)
t.ok(await doesNotProxy('http://example.com'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://sub.example.com'))
process.env.NO_PROXY = 'sub.example.com'
process.env.no_proxy = 'sub.example.com'
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://example.com'))
t.ok(await doesNotProxy('http://sub.example.com'))
return dispatcher.close()
Expand All @@ -469,7 +469,7 @@ describe('NO_PROXY', () => {
const { dispatcher, doesNotProxy, usesProxyAgent } = createEnvHttpProxyAgentWithMocks(4, { noProxy: 'example.com' })
t.ok(await doesNotProxy('http://example.com'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://sub.example.com'))
process.env.NO_PROXY = 'sub.example.com'
process.env.no_proxy = 'sub.example.com'
t.ok(await doesNotProxy('http://example.com'))
t.ok(await usesProxyAgent(kHttpProxyAgent, 'http://sub.example.com'))
return dispatcher.close()
Expand Down