fix: auto-detect HTTP proxy tunneling

docs/docs/api/ProxyAgent.md

@@ -27,7 +27,7 @@ For detailed information on the parsing process and potential validation errors,
 * **clientFactory** `(origin: URL, opts: Object) => Dispatcher` (optional) - Default: `(origin, opts) => new Pool(origin, opts)`
 * **requestTls** `BuildOptions` (optional) - Options object passed when creating the underlying socket via the connector builder for the request. It extends from [`Client#ConnectOptions`](/docs/docs/api/Client.md#parameter-connectoptions).
 * **proxyTls** `BuildOptions` (optional) - Options object passed when creating the underlying socket via the connector builder for the proxy server. It extends from [`Client#ConnectOptions`](/docs/docs/api/Client.md#parameter-connectoptions).
-* **proxyTunnel** `boolean` (optional) - For connections involving secure protocols, Undici will always establish a tunnel via the HTTP2  CONNECT extension. If proxyTunnel is set to true, this will occur for unsecured proxy/endpoint connections as well. Currently, there is no way to facilitate HTTP1 IP tunneling as described in https://www.rfc-editor.org/rfc/rfc9484.html#name-http-11-request. If proxyTunnel is set to false (the default), ProxyAgent connections where both the Proxy and Endpoint are unsecured will issue all requests to the Proxy, and prefix the endpoint request path with the endpoint origin address.
+* **proxyTunnel** `boolean` (optional) - Undici automatically detects when proxy tunneling is required. If either the proxy or the target endpoint uses a secure protocol, Undici will establish a tunnel via CONNECT. For plain HTTP proxy to plain HTTP endpoint connections, Undici will forward requests directly to the proxy and prefix the request path with the target origin. Set `proxyTunnel` to `true` to force tunneling for those unsecured HTTP-to-HTTP connections as well. Currently, there is no way to facilitate HTTP1 IP tunneling as described in https://www.rfc-editor.org/rfc/rfc9484.html#name-http-11-request.
 
 Examples:
 

lib/dispatcher/proxy-agent.js

@@ -36,6 +36,10 @@ function defaultAgentFactory (origin, opts) {
   return new Pool(origin, opts)
 }
 
+function shouldProxyTunnel (proxyProtocol, requestProtocol, proxyTunnel) {
+  return proxyTunnel === true || proxyProtocol !== 'http:' || requestProtocol !== 'http:'
+}
+
 class Http1ProxyWrapper extends DispatcherBase {
   #client
 
@@ -104,7 +108,7 @@ class ProxyAgent extends DispatcherBase {
       throw new InvalidArgumentError('Proxy opts.clientFactory must be a function.')
     }
 
-    const { proxyTunnel = true, connectTimeout } = opts
+    const { proxyTunnel, connectTimeout } = opts
 
     super()
 
@@ -150,7 +154,7 @@ class ProxyAgent extends DispatcherBase {
         })
       }
 
-      if (!this[kTunnelProxy] && protocol === 'http:' && this[kProxy].protocol === 'http:') {
+      if (!shouldProxyTunnel(this[kProxy].protocol, protocol, this[kTunnelProxy])) {
         return new Http1ProxyWrapper(this[kProxy].uri, {
           headers: this[kProxyHeaders],
           connect,

test/env-http-proxy-agent-nodejs-bundle.js

@@ -20,7 +20,7 @@ describe('EnvHttpProxyAgent and setGlobalDispatcher', () => {
     process.env = { ...env }
   })
 
-  test('should work with undici fetch from index-fetch', async (t) => {
+  test('should work with undici fetch from index-fetch with tunneling enabled', async (t) => {
     const { strictEqual } = tspl(t, { plan: 3 })
 
     // Instead of using mocks, start a real server and a minimal proxy server
@@ -73,7 +73,7 @@ describe('EnvHttpProxyAgent and setGlobalDispatcher', () => {
     const proxyAddress = `http://localhost:${proxy.address().port}`
     const serverAddress = `http://localhost:${server.address().port}`
     process.env.http_proxy = proxyAddress
-    setGlobalDispatcher(new EnvHttpProxyAgent())
+    setGlobalDispatcher(new EnvHttpProxyAgent({ proxyTunnel: true }))
 
     const res = await undiciFetch(serverAddress)
     strictEqual(await res.text(), 'Hello world')

test/env-http-proxy-agent.js

@@ -4,6 +4,8 @@ const { tspl } = require('@matteo.collina/tspl')
 const { test, describe, after, beforeEach } = require('node:test')
 const { EnvHttpProxyAgent, ProxyAgent, Agent, fetch, MockAgent } = require('..')
 const { kNoProxyAgent, kHttpProxyAgent, kHttpsProxyAgent, kClosed, kDestroyed, kProxy } = require('../lib/core/symbols')
+const { createServer } = require('node:http')
+const { createProxy } = require('proxy')
 
 const env = { ...process.env }
 
@@ -158,6 +160,54 @@ test('destroys all agents', async (t) => {
   t.ok(dispatcher[kHttpsProxyAgent][kDestroyed])
 })
 
+test('defaults to non-tunneled HTTP proxying for HTTP endpoints - #5093', async (t) => {
+  t = tspl(t, { plan: 3 })
+
+  const server = await buildServer()
+  const proxy = await buildProxy()
+
+  process.env.http_proxy = `http://localhost:${proxy.address().port}`
+
+  const dispatcher = new EnvHttpProxyAgent()
+  const serverUrl = `http://localhost:${server.address().port}`
+
+  try {
+    proxy.on('connect', () => {
+      t.fail('should not tunnel plain HTTP over an HTTP proxy by default')
+    })
+
+    proxy.on('request', (req) => {
+      t.strictEqual(req.url, `${serverUrl}/`)
+    })
+
+    server.on('request', (req, res) => {
+      t.strictEqual(req.url, '/')
+      res.end('ok')
+    })
+
+    const response = await fetch(serverUrl, { dispatcher })
+    t.strictEqual(await response.text(), 'ok')
+  } finally {
+    await new Promise((resolve) => proxy.close(resolve))
+    await new Promise((resolve) => server.close(resolve))
+    await dispatcher.close()
+  }
+})
+
+function buildServer () {
+  return new Promise((resolve) => {
+    const server = createServer({ joinDuplicateHeaders: true })
+    server.listen(0, () => resolve(server))
+  })
+}
+
+function buildProxy () {
+  return new Promise((resolve) => {
+    const server = createProxy(createServer({ joinDuplicateHeaders: true }))
+    server.listen(0, () => resolve(server))
+  })
+}
+
 const createEnvHttpProxyAgentWithMocks = (plan = 1, opts = {}) => {
   const factory = (origin) => {
     const mockAgent = new MockAgent()
@@ -171,7 +221,7 @@ const createEnvHttpProxyAgentWithMocks = (plan = 1, opts = {}) => {
   }
   process.env.http_proxy = 'http://localhost:8080'
   process.env.https_proxy = 'http://localhost:8443'
-  const dispatcher = new EnvHttpProxyAgent({ ...opts, factory })
+  const dispatcher = new EnvHttpProxyAgent({ proxyTunnel: true, ...opts, factory })
   const agentSymbols = [kNoProxyAgent, kHttpProxyAgent, kHttpsProxyAgent]
   agentSymbols.forEach((agentSymbol) => {
     const originalDispatch = dispatcher[agentSymbol].dispatch

test/proxy-agent.js

@@ -900,7 +900,7 @@ test('use proxy-agent with setGlobalDispatcher', async (t) => {
 
   const serverUrl = `http://localhost:${server.address().port}`
   const proxyUrl = `http://localhost:${proxy.address().port}`
-  const proxyAgent = new ProxyAgent({ uri: proxyUrl, proxyTunnel: false })
+  const proxyAgent = new ProxyAgent({ uri: proxyUrl })
   const parsedOrigin = new URL(serverUrl)
   setGlobalDispatcher(proxyAgent)
 
@@ -984,7 +984,7 @@ test('ProxyAgent correctly sends headers when using fetch - #1355, #1623', async
   const serverUrl = `http://localhost:${server.address().port}`
   const proxyUrl = `http://localhost:${proxy.address().port}`
 
-  const proxyAgent = new ProxyAgent({ uri: proxyUrl, proxyTunnel: false })
+  const proxyAgent = new ProxyAgent({ uri: proxyUrl })
   setGlobalDispatcher(proxyAgent)
 
   after(() => setGlobalDispatcher(defaultDispatcher))
@@ -1094,7 +1094,7 @@ test('should throw when proxy does not return 200', async (t) => {
     return false
   }
 
-  const proxyAgent = new ProxyAgent({ uri: proxyUrl, proxyTunnel: false })
+  const proxyAgent = new ProxyAgent({ uri: proxyUrl })
   try {
     await request(serverUrl, { dispatcher: proxyAgent })
     t.fail()

types/proxy-agent.d.ts

@@ -24,6 +24,11 @@ declare namespace ProxyAgent {
     requestTls?: buildConnector.BuildOptions;
     proxyTls?: buildConnector.BuildOptions;
     clientFactory?(origin: URL, opts: object): Dispatcher;
+    /**
+     * Undici automatically tunnels when either the proxy or the target endpoint
+     * uses a secure protocol. Set to true to force tunneling for plain HTTP
+     * proxy to plain HTTP endpoint connections as well.
+     */
     proxyTunnel?: boolean;
   }
 }