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

This refactors the way the legacy unsecured behaviour is implemented, by wrapping the Proxy client in a wrapper which rewrites requests, and handles errors. This will also insert authentication headers in each request.

Author: caitp

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) - 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,13 +1,12 @@
 'use strict'
 
-const { kProxy, kClose, kDestroy, kDispatch, kConnector } = require('../core/symbols')
+const { kProxy, kClose, kDestroy, kDispatch } = require('../core/symbols')
 const { URL } = require('node:url')
 const Agent = require('./agent')
 const Pool = require('./pool')
 const DispatcherBase = require('./dispatcher-base')
 const { InvalidArgumentError, RequestAbortedError, SecureProxyConnectionError } = require('../core/errors')
 const buildConnector = require('../core/connect')
-const Client = require('./client')
 
 const kAgent = Symbol('proxy agent')
 const kClient = Symbol('proxy client')
@@ -26,62 +25,74 @@ function defaultFactory (origin, opts) {
 }
 
 const noop = () => {}
+const Client = require('./client')
+class Http1ProxyWrapper extends DispatcherBase {
+  #client
+  constructor ({ client, headers = {} }) {
+    super()
+    if (!client) throw new InvalidArgumentError('Proxy client is mandatory')
 
-class ProxyClient extends DispatcherBase {
-  #client = null
-  constructor (origin, opts) {
-    if (typeof origin === 'string') {
-      origin = new URL(origin)
-    }
+    this[kProxyHeaders] = headers
+    this.#client = client
+  }
 
-    if (origin.protocol !== 'http:' && origin.protocol !== 'https:') {
-      throw new InvalidArgumentError('ProxyClient only supports http and https protocols')
+  [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
+      }
+      if (onHeaders) onHeaders.call(this, statusCode, data, resume)
     }
 
-    super()
+    const {
+      origin,
+      path = '/',
+      headers = {}
+    } = opts
+    opts.path = origin + path
 
-    this.#client = new Client(origin, opts)
+    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)
   }
 
   async [kClose] () {
-    await this.#client.close()
+    // FIXME: remove wrapper from the pool
   }
 
   async [kDestroy] () {
-    await this.#client.destroy()
+    // FIXME: remove wrapper from the pool
   }
 
-  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 })
-        }
-      }
-      )
-      return
-    }
-    if (typeof origin === 'string') {
-      opts.origin = new URL(origin)
-    }
+  close () {
+    // FIXME: remove wrapper from the pool
+  }
+
+  destroy () {
+    // FIXME: remove wrapper from the pool
+  }
 
-    return this.#client.dispatch(opts, handler)
+  // Optionally forward other methods if needed
+  get pending () {
+    return this.#client.pending
+  }
+
+  get running () {
+    return this.#client.running
+  }
+
+  get size () {
+    return this.#client.size
   }
 }
+
 class ProxyAgent extends DispatcherBase {
   constructor (opts) {
     if (!opts || (typeof opts === 'object' && !(opts instanceof URL) && !opts.uri)) {
@@ -93,7 +104,7 @@ class ProxyAgent extends DispatcherBase {
       throw new InvalidArgumentError('Proxy opts.clientFactory must be a function.')
     }
 
-    const { proxyTunnel = true } = opts
+    const { proxyTunnel = false } = opts
 
     super()
 
@@ -116,21 +127,20 @@ 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 })
+    const factory = (url, opts) => {
+      const { protocol } = typeof url === 'string' ? new URL(url) : url
+      if (!this[kTunnelProxy] && protocol === 'http:' && this[kProxy].protocol === 'http:') {
+        return new Http1ProxyWrapper({ client: this[kClient], headers: this[kProxyHeaders] })
+      }
+      return new Client(url, opts)
+    }
+    this[kClient] = clientFactory(url, { connect })
     this[kTunnelProxy] = proxyTunnel
     this[kAgent] = new Agent({
       ...opts,
+      factory,
       connect: async (opts, callback) => {
         let requestedPath = opts.host
         if (!opts.port) {
@@ -185,10 +195,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,
@@ -221,19 +227,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
-  }
 }
 
 /**