fix: auto-detect HTTP proxy tunneling

[mcollina]

This relates to...

Fixes #5093

Rationale

EnvHttpProxyAgent currently inherits ProxyAgent's tunneling behavior. For plain HTTP targets reached through an HTTP proxy, defaulting to CONNECT can break proxies that do not implement tunneling and can lead to the looping behavior reported in #5093.

Per RFC 9112 §3.2.2, the tunnel-vs-forward decision depends only on the request protocol, not the proxy protocol. This matches Node's built-in http.request / https.request.

Behavior

Request	Proxy	Action
HTTP	HTTP	Forward, absolute-form URI
HTTP	HTTPS	TLS to proxy, then forward, absolute-form URI
HTTPS	HTTP	CONNECT tunnel, TLS to target through it
HTTPS	HTTPS	TLS to proxy, CONNECT tunnel, TLS to target

proxyTunnel: true still forces CONNECT for plain HTTP requests.

Changes

• shouldProxyTunnel keys on the request protocol only
• Http1ProxyWrapper accepts a proxyServername to pin SNI to the proxy (otherwise the inner Client derives it from the rewritten Host header, which points at the target)
• Forward path through an HTTPS proxy wraps ERR_TLS_CERT_ALTNAME_INVALID as SecureProxyConnectionError, matching the tunneling path
• Regression coverage for EnvHttpProxyAgent using http_proxy (Use of HTTP_PROXY & NODE_USE_ENV_PROXY fails with an infinite loop #5093) and for the new HTTPS-proxy-to-HTTP-target forwarding case
• Docs and TypeScript comment updated

Bug Fixes

• fix default proxy behavior for plain HTTP over HTTP proxy connections
• fix HTTP-over-HTTPS-proxy to forward (absolute-form) instead of tunneling, per RFC 9112 §3.2.2
• fix the EnvHttpProxyAgent case behind Use of HTTP_PROXY & NODE_USE_ENV_PROXY fails with an infinite loop #5093

Breaking Changes and Deprecations

HTTP request through an HTTPS proxy now forwards instead of tunneling by default. Matches Node's built-in behavior and the RFC. Pass proxyTunnel: true to restore the previous behavior.

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[trivikr]

Should proxyTunnel be set to true on line 76 in test/env-http-proxy-agent-nodejs-bundle.js?

-    setGlobalDispatcher(new EnvHttpProxyAgent())
+    setGlobalDispatcher(new EnvHttpProxyAgent({ proxyTunnel: true }))

[mvolz]

Thanks for submitting this!

Do we have a sense of when this might make it into to a published branch? The reason I ask is that we're trying to replace request with fetch, but our outbound proxy rejects CONNECT requests for https so this is a blocker... trying to decide whether to revert all of it and go back to request we're in a deployable state, or if it's reasonable to wait for this change.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.20%. Comparing base (d863782) to head (19807ee).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5116      +/-   ##
==========================================
- Coverage   93.22%   93.20%   -0.03%     
==========================================
  Files         110      110              
  Lines       36603    36653      +50     
==========================================
+ Hits        34123    34162      +39     
- Misses       2480     2491      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[mcollina]

I investigated the remaining CI failures after the rebase.

All regular undici jobs are passing. The only failures left are the --shared-builtin-undici/undici-path jobs for Node 24 and 26.

What is happening there:

• this PR changes fetch() / EnvHttpProxyAgent so plain http -> http proxy -> http target traffic is non-tunneled by default
• in other words, the proxy now receives a regular proxied GET http://host/path request instead of a CONNECT
• the failing shared-builtin jobs run Node's own test/client-proxy/* suite against the externalized copy of undici
• those Node fetch tests still expect the old CONNECT behavior for the HTTP-over-HTTP-proxy case

I checked ../node locally and Node's http.request() tests already expect the curl-style non-tunneled behavior for the same setup:

• test/client-proxy/test-http-proxy-request.mjs
• test/client-proxy/test-http-set-global-proxy-from-env-http-request.mjs
• test/client-proxy/test-use-env-proxy-cli-http.mjs

That last file is especially telling because it currently expects:

• http.request() => proxied GET
• fetch() => tunneled CONNECT

So the failing shared-builtin jobs are not showing an undici regression in the main test matrix; they are showing that Node's fetch-side proxy tests need to be updated to match the new behavior.

I think the follow-up should be a nodejs/node patch adjusting those test/client-proxy fetch expectations for plain HTTP over HTTP proxy connections.

[joyeecheung]

I think the rules mostly depend on request protocol, not the proxy protocol? In the case of "http request over https proxy", 9112 3.2.2 says

When making a request to a proxy, other than a CONNECT or server-wide OPTIONS request (as detailed below), a client MUST send the target URI in "absolute-form" as the request-target.

(Basically the same as 7230 5.3.2) so e.g. a GET HTTP request over HTTPS proxy should be sent as

1. TLS handshake to proxy server
2. Sent the rewritten HTTP request directly to the proxy server

And according to 9110 9.3.6

Tunnels are commonly used to create an end-to-end virtual connection, through one or more proxies, which can then be secured using TLS (Transport Layer Security)

A HTTPS request over HTTP proxy should be sent as

1. Establish tunnel to proxy server (without TLS handshake to the proxy server)
2. TLS handshake to the target server through the tunnel
3. Sent the original HTTP request to the target server.

This is also what the builtin https/http.request implements.

(Note: I don't think socks5 count as a proxy defined in https://datatracker.ietf.org/doc/html/rfc9110, and socks5 is always effectively tunneled because the stuff in https://datatracker.ietf.org/doc/html/rfc1928 sits in a lower layer)

[joyeecheung]

The failing Node.js tests have this comment exactly for what this is fixing:

// FIXME(undici:4083): undici currently always tunnels the request over
// CONNECT if proxyTunnel is not explicitly set to false.

(which just means the tests are only documenting what undici currently does, and needs to be updated once undici fixes it, so failures here is to be expected).