feat(ProxyAgent): match Curl behavior in HTTP->HTTP Proxy connections (#4180) (#4433)

* feat(ProxyAgent): match Curl behavior in HTTP->HTTP Proxy connections (#4180)

* feat(ProxyAgent): match Curl behaviour for http-http Proxy connections

Curl does not send a CONNECT request for to a Proxy server, by default,
for cleartext communications to an endpoint, via a cleartext connection
to a Proxy. It permits forcing a CONNECT request to be sent via the
--tunnelproxy parameter.

This change modifies ProxyAgent's constructor to accept a `tunnelProxy`
option, sends a CONNECT if either `tunnelProxy` is true, or either the
Proxy or endpoint use a non-http: protocol.

Disabling tunneling for HTTP->HTTP by default would be a breaking change, so
currently, the tunneling behaviour requires an opt-out. This may change depending
on feedback during code review.

This adds a new test case which explicitly disables tunneling for an HTTP->HTTP
connection, and asserts that no CONNECT message is sent to the server or proxy,
and that the expected HTTP request is sent to the proxy.

Closes #4083

* Part 2

This version tries to expose less sketchiness -- it's not particularly well organized yet, and
I'm sure it could be cleaned up a lot.

Instead of adding the "rawSocket" stuff to RequestOptions, there's a new wrapper ProxyClient added,
which intercepts the CONNECT message and prevents it from being dispatched.

Unfortunately the wrapper client isn't quite written in a way to manage all of the client-ness,
so ProxyAgent is still responsible for updating the PATH of HTTP->HTTP Proxy requests to include
the endpoint domain.

It is messy though, admittedly.

* remove rawSocket from Dispatcher type definition

* Add some docs

* rename to proxyTunnel to match CURL

* Rename  to  in the docs, too

* Try to clarify the docs a bit

initially just wanted to fix a typo, but thought maybe the original explanation wasn't great.

(cherry picked from commit 95fd9d3)

* fix: test

---------

Co-authored-by: ⭐caitp⭐ <[email protected]>

Author: metcoder95

docs/docs/api/ProxyAgent.md

@@ -22,8 +22,9 @@ For detailed information on the parsing process and potential validation errors,
 * **token** `string` (optional) - It can be passed by a string of token for authentication.
 * **auth** `string` (**deprecated**) - Use token.
 * **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. See [TLS](https://nodejs.org/api/tls.html#tlsconnectoptions-callback).
-* **proxyTls** `BuildOptions` (optional) - Options object passed when creating the underlying socket via the connector builder for the proxy server. See [TLS](https://nodejs.org/api/tls.html#tlsconnectoptions-callback).
+* **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.
 
 Examples:
 

lib/dispatcher/proxy-agent.js

@@ -1,19 +1,21 @@
 'use strict'
 
-const { kProxy, kClose, kDestroy, kInterceptors } = require('../core/symbols')
+const { kProxy, kClose, kDestroy, kDispatch, kConnector, kInterceptors } = 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')
 const kProxyHeaders = Symbol('proxy headers')
 const kRequestTls = Symbol('request tls settings')
 const kProxyTls = Symbol('proxy tls settings')
 const kConnectEndpoint = Symbol('connect endpoint function')
+const kTunnelProxy = Symbol('tunnel proxy')
 
 function defaultProtocolPort (protocol) {
   return protocol === 'https:' ? 443 : 80
@@ -25,6 +27,61 @@ function defaultFactory (origin, opts) {
 
 const noop = () => {}
 
+class ProxyClient extends DispatcherBase {
+  #client = null
+  constructor (origin, opts) {
+    if (typeof origin === 'string') {
+      origin = new URL(origin)
+    }
+
+    if (origin.protocol !== 'http:' && origin.protocol !== 'https:') {
+      throw new InvalidArgumentError('ProxyClient only supports http and https protocols')
+    }
+
+    super()
+
+    this.#client = new Client(origin, opts)
+  }
+
+  async [kClose] () {
+    await this.#client.close()
+  }
+
+  async [kDestroy] () {
+    await this.#client.destroy()
+  }
+
+  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)
+    }
+
+    return this.#client.dispatch(opts, handler)
+  }
+}
 class ProxyAgent extends DispatcherBase {
   constructor (opts) {
     super()
@@ -38,6 +95,8 @@ class ProxyAgent extends DispatcherBase {
       throw new InvalidArgumentError('Proxy opts.clientFactory must be a function.')
     }
 
+    const { proxyTunnel = true } = opts
+
     const url = this.#getUrl(opts)
     const { href, origin, port, protocol, username, password, hostname: proxyHostname } = url
 
@@ -60,9 +119,19 @@ 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 })
+    this[kClient] = clientFactory(url, { connect, factory })
+    this[kTunnelProxy] = proxyTunnel
     this[kAgent] = new Agent({
       ...opts,
       connect: async (opts, callback) => {
@@ -118,6 +187,10 @@ 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,
@@ -150,6 +223,19 @@ 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
+  }
 }
 
 /**

test/proxy-agent.js

@@ -9,6 +9,7 @@ const Pool = require('../lib/dispatcher/pool')
 const { createServer } = require('node:http')
 const https = require('node:https')
 const { createProxy } = require('proxy')
+const nodeMajorVersion = Number(process.versions.node.split('.')[0])
 
 const certs = (() => {
   const forge = require('node-forge')
@@ -793,6 +794,54 @@ test('Proxy via HTTP to HTTP endpoint', async (t) => {
   proxyAgent.close()
 })
 
+test('Proxy via HTTP to HTTP endpoint with tunneling disabled', async (t) => {
+  t = tspl(t, { plan: 3 })
+  const server = await buildServer()
+  const proxy = await buildProxy()
+
+  const serverUrl = `http://localhost:${server.address().port}`
+  const proxyUrl = `http://localhost:${proxy.address().port}`
+  const proxyAgent = new ProxyAgent({ uri: proxyUrl, proxyTunnel: false })
+
+  server.on('request', function (req, res) {
+    t.ok(!req.connection.encrypted)
+    const headers = { host: req.headers.host, connection: req.headers.connection }
+    res.end(JSON.stringify(headers))
+  })
+
+  server.on('secureConnection', () => {
+    t.fail('server is http')
+  })
+
+  proxy.on('secureConnection', () => {
+    t.fail('proxy is http')
+  })
+
+  proxy.on('connect', () => {
+    t.fail(true, 'connect to proxy should unreachable if proxyTunnel is false')
+  })
+
+  proxy.on('request', function (req) {
+    const bits = { method: req.method, url: req.url }
+    t.deepStrictEqual(bits, {
+      method: 'GET',
+      url: `${serverUrl}/`
+    })
+  })
+
+  const data = await request(serverUrl, { dispatcher: proxyAgent })
+  const json = await data.body.json()
+  t.deepStrictEqual(json, {
+    host: `localhost:${server.address().port}`,
+    // Mismatch on behaviour between 18 and upwards
+    connection: nodeMajorVersion > 18 ? 'keep-alive' : 'close'
+  })
+
+  server.close()
+  proxy.close()
+  proxyAgent.close()
+})
+
 test('Proxy via HTTPS to HTTP fails on wrong SNI', async (t) => {
   t = tspl(t, { plan: 2 })
   const server = await buildServer()

types/proxy-agent.d.ts

@@ -24,5 +24,6 @@ declare namespace ProxyAgent {
     requestTls?: buildConnector.BuildOptions;
     proxyTls?: buildConnector.BuildOptions;
     clientFactory?(origin: URL, opts: object): Dispatcher;
+    proxyTunnel?: boolean;
   }
 }