http: support HTTP[S]_PROXY environment variables

[joyeecheung]

This is currently a POC. Just trying to open a PR first to see what folks think about this approach - that is, to support HTTP_PROXY etc. environment variables out of the box. The exposure of the lower-level dispatcher and EnvHttpProxyAgent would be a different feature, but I think for many users, having the environment variables supported out of the box is already enough or more important to solve their headaches, while the lower-level utilities are less essential - many other language runtimes/command line tools only support these environment variables out of the box, without necessarily providing more utilities for further customization, and that seems enough for most people who are behind a proxy already.

The idea is that we can first make this handling opt-in via another environment variable (NODE_USE_ENV_PROXY here, or we can use others if there are better names). And in the future we can enable this flag by default, as this is still opt-in under well-known environment variables HTTP_PROXY. I think some investigation is needed to make sure that enabling it by default would not step on the toes of other libraries/tools already recognizing these environment variables and do their own thing.

This requires a patch in undici to make the EnvHttpProxyAgent usable from the Node.js bundle, which I included locally in this PR and a locally built bundle for demonstration purpose. I opened a separate PR to undici to see what's the best way to patch it nodejs/undici#4064

I've only tested it with fetch so far, and left a TODO for handling of http global agents.

Refs: nodejs/undici#1650

[nodejs-github-bot]

Review requested:

• @nodejs/security-wg

[joyeecheung]

cc @mcollina @ronag (maybe there are others that I am forgetting, but you held the floor most of the time in the session in the London summit about this :))

[legendecas]

I believe this will be a notable change.

[github-actions]

The notable-change PRs with changes that should be highlighted in changelogs. label has been added by @legendecas.

Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.

[mcollina]

why is this needed?

[joyeecheung]

I opened a separate PR in undici nodejs/undici#4064 , let's discuss it there.

[joyeecheung]

Depends on #57236

@mcollina why is this semver-major? I think if we make it behind another environment variable first (NODE_USE_HTTP_PROXY in this PR, can change to a different name if a better one comes up), this is still semver-minor, and only another PR that makes it enabled by default and have to be disabled explicitly would be semver-major?

[mcollina]

why is this semver-major? I think if we make it behind another environment variable first (NODE_USE_HTTP_PROXY in this PR, can change to a different name if a better one comes up), this is still semver-minor, and only another PR that makes it enabled by default and have to be disabled explicitly would be semver-major?

The HTTP_PROXY env is used also by other programs, so it's relatively possible that it's set in an environment, making it a behavior change.
Putting it behind a flag/API/Node-specific env variable would make it non-breaking as you proposd.

[jasnell]

Would the intention be to limit this to just undici/fetch? That is, would this also work for require('http').get(...), etc. I'm perfectly fine saying that this should be limited to undici/fetch but want to clarify the intent.

[mcollina]

Given how the current http.Agent work, I'm not entirely sure it's possible to enable it by default without breaking some part of the ecosystem unexpectedly(see the monkeypatching that nock does, as well as observability providers).

[joyeecheung]

I think a probable path forward would be:

1. Make it work with fetch behind NODE_USE_HTTP_PROXY as semver-minor
2. Making it work with http.request etc. behind NODE_USE_HTTP_PROXY as semver-minor
3. Enabling it by default for fetch as semver-major
4. Enabling it by default for http.request as semver-major

3 and 4 can be done independently. In the worst case, we stop at 2, but at least it still allows users being trapped by firewalls and not always having control over the code making requests to opt into a solution that solves their headaches by setting the environment variable, which might be good enough for many already. I think 3 might not be that risky, 4 will need some investigation, but we can use 2 to let users try it out and explore any possible interop problems with existing libraries. So I expect 2 would need a bit more time to fully develop but since it's opt-in as an experimental feature it seems pretty reasonable.