fetch() and http.request() disagree when a lower-cased proxy env var is empty
Nadie ha tomado este issue todavía.
Evaluación
- Dificultad
- 4/5
- Tiempo estimado
- 3-5 días
- Aptitud para principiantes
- 45/100
- Tipo de issue
- Error
- Claridad
- Bastante claro
- Estado de actividad
- Activo
- Stack tecnológico
- javascript, node.js
- Área
- backend, networking
Línea de trabajo
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.
Escrito por el modelo de indexación a partir del texto del issue.
Descripción
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.
- Lenguaje dominante
- JavaScript
- Estrellas
- 122k
- Forks
- 37.4k
- Merge medio
- 4 d 4 h
- PR fusionados (30 d)
- 276
Guía de contribución
Primeros pasos
- Lee el issue completo y luego la guía de contribución del proyecto.
- Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
- Haz un fork del repositorio y trabaja en una rama.
- Abre un pull request que haga referencia al número del issue.
Más de nodejs/node
-
doc
Dificultad 2/5 1-3 horas Aptitud para principiantes 65/100
-
build
Dificultad 1/5 Menos de una hora Aptitud para principiantes 88/100
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 84/100
-
Dificultad 1/5 Menos de una hora Aptitud para principiantes 90/100
-
feature request
Dificultad 2/5 1-3 horas Aptitud para principiantes 68/100
Todos los issues de nodejs/node
Issues similares
-
bug customer-eng Durable Agents Inngest status: needs triage
Dificultad 2/5 1-3 horas Aptitud para principiantes 82/100
-
optimization optimization:agents-md-curator
Dificultad 2/5 1-3 horas Aptitud para principiantes 75/100
githubnext/gh-aw-cao#13475 ·
-
[BUG]: "Clear All" in Settings doesn't clear the saved analysis, old data comes back after reload Abiertobug
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
AOSSIE-Org/OrgExplorer#253 · 1 comentario ·
-
Dificultad 2/5 1-3 horas Aptitud para principiantes 88/100
oxc-project/oxc#26944 ·
-
ai-observability bug team/ai-observability
Dificultad 2/5 1-3 horas Aptitud para principiantes 78/100