feat(ProxyAgent) improve Curl-y behavior in HTTP->HTTP Proxy connections (#4180) (#4340) (#4445)

* feat(ProxyAgent) improve Curl-y behavior in HTTP->HTTP Proxy connections (#4180) (#4340)

* test: adjust v18 expectations

Author: metcoder95

docs/docs/api/ProxyAgent.md

@@ -24,7 +24,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) - By default, ProxyAgent will request that the Proxy facilitate a tunnel between the endpoint and the agent. Setting `proxyTunnel` to false avoids issuing a CONNECT extension, and includes the endpoint domain and path in each request.
+* **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.
 
 Examples:
 

lib/dispatcher/proxy-agent.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const { kProxy, kClose, kDestroy, kDispatch, kConnector, kInterceptors } = require('../core/symbols')
+const { kProxy, kClose, kDestroy, kDispatch, kInterceptors } = require('../core/symbols')
 const { URL } = require('node:url')
 const Agent = require('./agent')
 const Pool = require('./pool')
@@ -27,61 +27,69 @@ function defaultFactory (origin, opts) {
 
 const noop = () => {}
 
-class ProxyClient extends DispatcherBase {
-  #client = null
-  constructor (origin, opts) {
-    if (typeof origin === 'string') {
-      origin = new URL(origin)
-    }
+function defaultAgentFactory (origin, opts) {
+  if (opts.connections === 1) {
+    return new Client(origin, opts)
+  }
+  return new Pool(origin, opts)
+}
 
-    if (origin.protocol !== 'http:' && origin.protocol !== 'https:') {
-      throw new InvalidArgumentError('ProxyClient only supports http and https protocols')
-    }
+class Http1ProxyWrapper extends DispatcherBase {
+  #client
 
+  constructor (proxyUrl, { headers = {}, connect, factory }) {
     super()
+    if (!proxyUrl) {
+      throw new InvalidArgumentError('Proxy URL is mandatory')
+    }
 
-    this.#client = new Client(origin, opts)
-  }
-
-  async [kClose] () {
-    await this.#client.close()
-  }
-
-  async [kDestroy] () {
-    await this.#client.destroy()
+    this[kProxyHeaders] = headers
+    if (factory) {
+      this.#client = factory(proxyUrl, { connect })
+    } else {
+      this.#client = new Client(proxyUrl, { connect })
+    }
   }
 
-  async [kDispatch] (opts, handler) {
-    const { method, origin } = opts
-    if (method === 'CONNECT') {
-      this.#client[kConnector]({
-        origin,
-        port: opts.port || defaultProtocolPort(opts.protocol),
-        path: opts.host,
-        signal: opts.signal,
-        headers: {
-          ...this[kProxyHeaders],
-          host: opts.host
-        },
-        servername: this[kProxyTls]?.servername || opts.servername
-      },
-      (err, socket) => {
-        if (err) {
-          handler.callback(err)
-        } else {
-          handler.callback(null, { socket, statusCode: 200 })
+  [kDispatch] (opts, handler) {
+    const onHeaders = handler.onHeaders
+    handler.onHeaders = function (statusCode, data, resume) {
+      if (statusCode === 407) {
+        if (typeof handler.onError === 'function') {
+          handler.onError(new InvalidArgumentError('Proxy Authentication Required (407)'))
         }
+        return
       }
-      )
-      return
+      if (onHeaders) onHeaders.call(this, statusCode, data, resume)
     }
-    if (typeof origin === 'string') {
-      opts.origin = new URL(origin)
+
+    // Rewrite request as an HTTP1 Proxy request, without tunneling.
+    const {
+      origin,
+      path = '/',
+      headers = {}
+    } = opts
+
+    opts.path = origin + path
+
+    if (!('host' in headers) && !('Host' in headers)) {
+      const { host } = new URL(origin)
+      headers.host = host
     }
+    opts.headers = { ...this[kProxyHeaders], ...headers }
+
+    return this.#client[kDispatch](opts, handler)
+  }
 
-    return this.#client.dispatch(opts, handler)
+  async [kClose] () {
+    return this.#client.close()
+  }
+
+  async [kDestroy] (err) {
+    return this.#client.destroy(err)
   }
 }
+
 class ProxyAgent extends DispatcherBase {
   constructor (opts) {
     super()
@@ -107,6 +115,7 @@ class ProxyAgent extends DispatcherBase {
     this[kRequestTls] = opts.requestTls
     this[kProxyTls] = opts.proxyTls
     this[kProxyHeaders] = opts.headers || {}
+    this[kTunnelProxy] = proxyTunnel
 
     if (opts.auth && opts.token) {
       throw new InvalidArgumentError('opts.auth cannot be used in combination with opts.token')
@@ -119,21 +128,25 @@ class ProxyAgent extends DispatcherBase {
       this[kProxyHeaders]['proxy-authorization'] = `Basic ${Buffer.from(`${decodeURIComponent(username)}:${decodeURIComponent(password)}`).toString('base64')}`
     }
 
-    const factory = (!proxyTunnel && protocol === 'http:')
-      ? (origin, options) => {
-          if (origin.protocol === 'http:') {
-            return new ProxyClient(origin, options)
-          }
-          return new Client(origin, options)
-        }
-      : undefined
-
     const connect = buildConnector({ ...opts.proxyTls })
     this[kConnectEndpoint] = buildConnector({ ...opts.requestTls })
-    this[kClient] = clientFactory(url, { connect, factory })
-    this[kTunnelProxy] = proxyTunnel
+
+    const agentFactory = opts.factory || defaultAgentFactory
+    const factory = (origin, options) => {
+      const { protocol } = new URL(origin)
+      if (!this[kTunnelProxy] && protocol === 'http:' && this[kProxy].protocol === 'http:') {
+        return new Http1ProxyWrapper(this[kProxy].uri, {
+          headers: this[kProxyHeaders],
+          connect,
+          factory: agentFactory
+        })
+      }
+      return agentFactory(origin, options)
+    }
+    this[kClient] = clientFactory(url, { connect })
     this[kAgent] = new Agent({
       ...opts,
+      factory,
       connect: async (opts, callback) => {
         let requestedPath = opts.host
         if (!opts.port) {
@@ -187,10 +200,6 @@ class ProxyAgent extends DispatcherBase {
       headers.host = host
     }
 
-    if (!this.#shouldConnect(new URL(opts.origin))) {
-      opts.path = opts.origin + opts.path
-    }
-
     return this[kAgent].dispatch(
       {
         ...opts,
@@ -223,19 +232,6 @@ class ProxyAgent extends DispatcherBase {
     await this[kAgent].destroy()
     await this[kClient].destroy()
   }
-
-  #shouldConnect (uri) {
-    if (typeof uri === 'string') {
-      uri = new URL(uri)
-    }
-    if (this[kTunnelProxy]) {
-      return true
-    }
-    if (uri.protocol !== 'http:' || this[kProxy].protocol !== 'http:') {
-      return true
-    }
-    return false
-  }
 }
 
 /**