fetch() and http.request() disagree when a lower-cased proxy env var is empty
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 45/100
- Issue type
- Bug
- Clarity
- Mostly clear
- Activity status
- Active
- Tech stack
- javascript, node.js
- Domain
- backend, networking
Research direction
Start by running the reproduction, then read the proxy selection in lib/internal/http.js and EnvHttpProxyAgent in deps/undici/src/lib/dispatcher/env-http-proxy-agent.js. Add a regression test around the usesProxy('request', env) and usesProxy('fetch', env) cases, and confirm both clients interpret empty proxy variables consistently after the intended behavior is decided.
Written by the indexing model from the issue text.
Description
Version
v27.0.0-pre (main, 976a36a5b6c)
Platform
Linux 6.14.0-37-generic x64
Subsystem
http
What steps will reproduce the bug?
Run the script below with a build that has built-in proxy support.
It starts a proxy and an origin server, then launches a child process that sends one http.request() and one fetch() using the same proxy environment. The script reports whether each request went through the proxy.
'use strict';
const http = require('http');
const { spawn } = require('child_process');
const { once } = require('events');
const seen = [];
const proxy = http.createServer((req, res) => {
seen.push(req.url);
const url = new URL(req.url);
http.request({
hostname: url.hostname,
port: url.port,
path: url.pathname,
})
.on('response', (proxyRes) => proxyRes.pipe(res))
.on('error', () => res.end())
.end();
});
const server = http.createServer((req, res) => res.end('Hello world'));
const child = `
const url = process.env.REQUEST_URL;
require('http').get(url + 'request', (res) => {
res.resume();
res.on('end', () => {
fetch(url + 'fetch')
.then((r) => r.text())
.then(
() => process.exit(0),
() => process.exit(0),
);
});
});
`;
(async () => {
proxy.listen(0);
server.listen(0);
await Promise.all([
once(proxy, 'listening'),
once(server, 'listening'),
]);
const PROXY = `http://localhost:${proxy.address().port}`;
const REQUEST_URL = `http://localhost:${server.address().port}/`;
const cases = [
['HTTP_PROXY only ', {
HTTP_PROXY: PROXY,
}],
["http_proxy='' HTTP_PROXY=proxy", {
http_proxy: '',
HTTP_PROXY: PROXY,
}],
["no_proxy='' NO_PROXY='*' ", {
HTTP_PROXY: PROXY,
no_proxy: '',
NO_PROXY: '*',
}],
];
for (const [name, env] of cases) {
seen.length = 0;
const childEnv = {
...process.env,
NODE_USE_ENV_PROXY: '1',
REQUEST_URL,
};
// Avoid inheriting proxy configuration from the machine running
// the reproduction.
for (const key of [
'http_proxy',
'HTTP_PROXY',
'https_proxy',
'HTTPS_PROXY',
'no_proxy',
'NO_PROXY',
]) {
delete childEnv[key];
}
Object.assign(childEnv, env);
const cp = spawn(process.execPath, ['-e', child], {
env: childEnv,
stdio: 'ignore',
});
await once(cp, 'exit');
const via = (tag) =>
seen.some((url) => url.endsWith('/' + tag)) ? 'proxy ' : 'direct';
console.log(
`${name} http.request: ${via('request')} fetch: ${via('fetch')}`,
);
}
proxy.close();
server.close();
})();
How often does it reproduce? Is there a required condition?
Every run.
It requires NODE_USE_ENV_PROXY=1 (or --use-env-proxy) and a lower-cased proxy variable set to the empty string while its upper-cased counterpart is set.
This does not apply to Windows, where environment variable names are case-insensitive.
What is the expected behavior? Why is that the expected behavior?
fetch() and http.request() should interpret the same proxy environment consistently when built-in proxy support is enabled.
Which interpretation of an empty lower-cased value should be used is a separate question. There are at least two existing behaviors in the ecosystem:
- curl treats an empty lower-cased value as unset and can fall back to the upper-cased variable.
- Python's
urllib.getproxies_environment()treats an empty lower-cased value as an override that removes the upper-cased value.
Node.js currently implements both interpretations at once, depending on which client is used.
The HTTP documentation describes the lower-cased variables as, for example:
Same as
HTTP_PROXY. If both are set,http_proxytakes precedence.
An explicitly present empty string makes the intended behavior here ambiguous.
What do you see instead?
HTTP_PROXY only http.request: proxy fetch: proxy
http_proxy='' HTTP_PROXY=proxy http.request: proxy fetch: direct
no_proxy='' NO_PROXY='*' http.request: direct fetch: proxy
The last two cases disagree, in opposite directions.
Additional information
The built-in HTTP implementation uses || when selecting between lower- and upper-cased variables:
env.http_proxy || env.HTTP_PROXY
env.no_proxy || env.NO_PROXY
Therefore an empty lower-cased value is falsy and the upper-cased value wins.
Undici's EnvHttpProxyAgent, used by fetch() through setupHttpProxy(), uses nullish coalescing instead:
deps/undici/src/lib/dispatcher/env-http-proxy-agent.js#L26:
httpProxy ?? process.env.http_proxy ?? process.env.HTTP_PROXY
and for NO_PROXY:
deps/undici/src/lib/dispatcher/env-http-proxy-agent.js#L171:
process.env.no_proxy ?? process.env.NO_PROXY ?? ''
Therefore an empty lower-cased value wins on the Undici side.
The built-in behavior has been present since 036b1fd66d8 added proxy support to http.request() in v25.0.0. The reproduction above was only run against the stated build of main.
This appears related to #65616, where NO_PROXY matching also differs between fetch() and http.request().
It also relates directly to the still-open item in #57872:
Share code between the fetch and the http(s) builtin implementation (e.g. env var parsing & matching)
A regression test can demonstrate the inconsistency without deciding which interpretation is correct:
const viaRequest = await usesProxy('request', env);
const viaFetch = await usesProxy('fetch', env);
assert.strictEqual(viaFetch, viaRequest);
On current behavior, the empty http_proxy and empty no_proxy cases fail.
I would be happy to send a PR once it is decided which interpretation built-in proxy support should use consistently.
If the built-in HTTP behavior is preferred, the corresponding Undici behavior would need to be aligned upstream. If the Undici behavior is preferred, the || selections in lib/internal/http.js can be changed accordingly. The documentation should then specify how empty values are interpreted.
- Dominant language
- JavaScript
- Stars
- 122k
- Forks
- 37.4k
- Avg merge
- 4d 3h
- Merged PRs (30d)
- 279
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from nodejs/node
-
doc
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
-
build
Difficulty 1/5 Under an hour Newbie friendliness 88/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 84/100
-
Difficulty 1/5 Under an hour Newbie friendliness 90/100
-
feature request
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
Similar issues
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
palladius/rails8-app-on-gcp#145 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
dotenvx/dotenv-vscode#139 ·
-
test-change-proposal
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
web-platform-tests/interop#1455 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
corsairdev/corsair#1764 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100