Tracking issue: HTTP_PROXY/HTTPS_PROXY/NO_PROXY support in Node.js

[joyeecheung]

• Support https/http proxies via HTTP_PROXY/HTTPS_PROXY/NO_PROXY in fetch() (behind NODE_USE_ENV_PROXY)
  • Initial support http: support HTTP[S]_PROXY environment variables in fetch #57165
  • align with curl: do not tunnel for http proxy ProxyAgent has different behavior from curl -x undici#4083
  • align with curl: support keep-alive Support proxy keep-alive undici#4086
• Support https/http proxies via HTTP_PROXY/HTTPS_PROXY/NO_PROXY in http(s) builtins (behind NODE_USE_ENV_PROXY)
  • Initial support for http(s).request and agent: http,https: add built-in proxy support in http/https.request and Agent #58980
  • Check that it allows other popular user-land agents (for proxy or not) to operate with/without NODE_USE_ENV_PROXY
  • Check that it works transparently with popular npm packages that uses http/https.request
• Socks 5 proxy support
  • Socks5 proxy support in undici: ProxyAgent: support socks5 protocol undici#2224
  • Socks5 proxy support in http/https builtins
• Investigate enabling NODE_USE_ENV_PROXY by default
  • In fetch() (less risky since fetch is patched less in the wild)
  • In http(s) builtins (might be too risky, need to investigate)
• Share code between the fetch and the http(s) builtin implementation (e.g. env var parsing & matching)
• Add a CLI flag that allows enabling proxies: Add --use-env-proxy to NODE_OPTIONS #59100

Old issues: #8381 #15620

[joyeecheung]

cc @nodejs/undici @nodejs/http

I have been looking into support for the http(s) builtins but it seems a bit too convoluted if this is hooked into the existing socket pooling of the agent and we need to perform tunnelling over the socket, as the lifecycle of the socket is exposed through the agent interface as well, So I am planning to take a different approach and just bypass the pooling when proxies are used for tunneling (that seems to be what most existing solutions on npm are already doing in their custom agents, anyway, just in a more convoluted way to combat with the Node.js internal invariants)

[joyeecheung]

So I did comment that pooling seems difficult to support, but I ended up finishing an implementation that works with pooling and it's passing tests of basic use cases and failure cases: https://github.com/joyeecheung/node/tree/http-agent-proxy-4/ - still need another pass to check that I am not leaking anything with the pooling and finish more tests before I open a PR though.

[joyeecheung]

Still moving along with the implementation and was trying to figure out how to implement the timeouts in tunnel establishment. From what I gathered the most intuitive handling is probably - follow the timeout in per-request option bag if there is one, then fallback to the agent timeout if there is one, otherwise just make it infinite like the normal default request options. There might be appetite for a separate timeout value for the tunnel, but then it seems niche (most npm packages I've seen either just uses default and goes infinite or inherit from request/agent options) so we could defer that to later if anyone actually wants it.

[joyeecheung]

Almost finished the initial implementation, it currently has 36 proxy specific tests (most of the diff is tests). Ran all the other existing http/https tests through a minimal local proxy server to check that the proxy behavior is transparent:

NODE_USE_ENV_PROXY=1 HTTPS_PROXY=http://127.0.0.1:8000 HTTP_PROXY=http://127.0.0.1:8000 tools/test.py --timeout=20 "test/parallel/test-http*"

[03:36|% 100|+ 622|-  79]: Done

Most of the failures are caused by the testing proxy server itself not being very transparent since I didn't put a lot of thought into it, or that the tests are expecting specific things (e.g. events, errors) that have to come from a server in the same process. Will do another pass to check the failures to make sure that it's at least not worse than user-land implementations, before I open a PR soon-ish - I think the agent API surface might still need some discussion in the PR, but the functionality is unlikely to change since it's pretty conventional.

[bnoordhuis]

NODE_USE_ENV_PROXY=1

The rule/guideline is "no new environment variables," right? --use-env-proxy that's accepted in NODE_OPTIONS?

(node already has way too many switches IMO but that's a separate discussion.)

[joyeecheung]

It's a bit too late: we already have shipped for the support in fetch. https://nodejs.org/api/cli.html#node_use_env_proxy1

Although in practice NODE_OPTIONS may also be somewhat tricky to concatenate or inherit especially when dealing with workers (#41103), and the worker is not necessarily controlled by those who need to enable the feature (e.g. a worker spawned by a library forget to or could not properly concatenate the CLI options, and that worker sends an unproxied request in an environment that needs a proxy, env vars tend to be handled more correctly since it's a flat object)

[richardlau]

Since this is a tracking issue, it looks like the tests introduced in #57165 trigger an assertion failure in lib/internal/process/pre_execution.js when Node.js is built with an external Undici (./configure --shared-builtin-undici/undici-path ...): #58865

[joyeecheung]

PR for initial implementation of proxy support in http/https.request and Agent #58980