fetch() and http.request() disagree when a lower-cased proxy env var is empty
Ninguém assumiu esta issue ainda.
Avaliação
- Dificuldade
- 4/5
- Tempo estimado
- 3-5 dias
- Facilidade para iniciantes
- 45/100
- Tipo de issue
- Bug
- Clareza
- Razoavelmente clara
- Status de atividade
- Ativa
- Stack de tecnologia
- javascript, node.js
- Domínio
- backend, networking
Direção de pesquisa
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.
Escrita pelo modelo de indexação a partir do texto da issue.
Descrição
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.
- Linguagem predominante
- JavaScript
- Estrelas
- 122k
- Forks
- 37.4k
- Merge médio
- 4d 2h
- PRs com merge (30d)
- 277
Guia de contribuição
Primeiros passos
- Leia a issue inteira e depois o guia de contribuição do projeto.
- Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
- Faça um fork do repositório e trabalhe em uma branch.
- Abra um pull request que referencie o número da issue.
Mais de nodejs/node
-
doc
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 65/100
-
build
Dificuldade 1/5 Menos de uma hora Facilidade para iniciantes 88/100
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 84/100
-
Dificuldade 1/5 Menos de uma hora Facilidade para iniciantes 90/100
-
feature request
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 68/100
Todas as issues de nodejs/node
Issues semelhantes
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 70/100
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 75/100
mksglu/context-mode#1200 ·
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 75/100
neondatabase/website#5944 ·
-
module: core
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 75/100
bigbluebutton/bigbluebutton#25849 ·
-
Dificuldade 2/5 1-3 horas Facilidade para iniciantes 75/100
jaegertracing/jaeger-ui#4506 ·