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

By default, Curl does not send a CONNECT request to a non-secure
Proxy with a non-secure endpoint. This can be overridden using the
--proxytunnel or -p 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 behavior 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

Author: caitp

lib/core/request.js

@@ -42,7 +42,8 @@ class Request {
     reset,
     expectContinue,
     servername,
-    throwOnError
+    throwOnError,
+    rawSocket
   }, handler) {
     if (typeof path !== 'string') {
       throw new InvalidArgumentError('path must be a string')
@@ -94,6 +95,12 @@ class Request {
 
     this.abort = null
 
+    if (rawSocket) {
+      assert(body == null, 'rawSocket cannot be used with body')
+      assert(method === 'CONNECT', 'rawSocket can only be used with CONNECT method')
+    }
+    this.rawSocket = rawSocket
+
     if (body == null) {
       this.body = null
     } else if (isStream(body)) {

lib/dispatcher/client.js

@@ -51,7 +51,7 @@ const {
   kOnError,
   kHTTPContext,
   kMaxConcurrentStreams,
-  kResume
+  kResume, kSocket
 } = require('../core/symbols.js')
 const connectH1 = require('./client-h1.js')
 const connectH2 = require('./client-h2.js')
@@ -598,6 +598,12 @@ function _resume (client, sync) {
       return
     }
 
+    if (!request.aborted && request.rawSocket) {
+      assert(request.method === 'CONNECT')
+      request.onUpgrade(200, [], client[kSocket])
+      request.aborted = true
+    }
+
     if (!request.aborted && client[kHTTPContext].write(request)) {
       client[kPendingIdx]++
     } else {

lib/dispatcher/proxy-agent.js

@@ -14,6 +14,7 @@ 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
@@ -36,6 +37,8 @@ class ProxyAgent extends DispatcherBase {
       throw new InvalidArgumentError('Proxy opts.clientFactory must be a function.')
     }
 
+    const { tunnelProxy = true } = opts
+
     super()
 
     const url = this.#getUrl(opts)
@@ -60,13 +63,15 @@ class ProxyAgent extends DispatcherBase {
     const connect = buildConnector({ ...opts.proxyTls })
     this[kConnectEndpoint] = buildConnector({ ...opts.requestTls })
     this[kClient] = clientFactory(url, { connect })
+    this[kTunnelProxy] = tunnelProxy
     this[kAgent] = new Agent({
       ...opts,
       connect: async (opts, callback) => {
         let requestedPath = opts.host
         if (!opts.port) {
           requestedPath += `:${defaultProtocolPort(opts.protocol)}`
         }
+        const shouldConnect = this[kTunnelProxy] || protocol !== 'http:' || opts.protocol !== 'http:'
         try {
           const { socket, statusCode } = await this[kClient].connect({
             origin,
@@ -77,6 +82,7 @@ class ProxyAgent extends DispatcherBase {
               ...this[kProxyHeaders],
               host: opts.host
             },
+            rawSocket: !shouldConnect,
             servername: this[kProxyTls]?.servername || proxyHostname
           })
           if (statusCode !== 200) {
@@ -115,6 +121,12 @@ class ProxyAgent extends DispatcherBase {
       headers.host = host
     }
 
+    // If we have a non-secure, non-tunneling proxy, the server URL is prepended to the
+    // path to ensure that the Proxy can handle the request appropriately.
+    if (!this[kTunnelProxy] && opts.path && opts.origin) {
+      opts.path = `${opts.origin}${opts.path}`
+    }
+
     return this[kAgent].dispatch(
       {
         ...opts,

test/proxy-agent.js

@@ -793,6 +793,53 @@ 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, tunnelProxy: 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 tunnelProxy 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}`,
+    connection: 'keep-alive'
+  })
+
+  server.close()
+  proxy.close()
+  proxyAgent.close()
+})
+
 test('Proxy via HTTPS to HTTP fails on wrong SNI', async (t) => {
   t = tspl(t, { plan: 3 })
   const server = await buildServer()

types/dispatcher.d.ts

@@ -140,6 +140,8 @@ declare namespace Dispatcher {
     redirectionLimitReached?: boolean;
     /** Default: `null` */
     responseHeaders?: 'raw' | null;
+    /** Default: false */
+    rawSocket?: boolean;
   }
   export interface RequestOptions<TOpaque = null> extends DispatchOptions {
     /** Default: `null` */

types/proxy-agent.d.ts

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