fix: Do not crash when a ill formed URL request is proxied (#19274)

Author: ryanthemanuel

packages/https-proxy/lib/server.js

@@ -34,6 +34,16 @@ class Server {
   }
 
   connect (req, browserSocket, head, options = {}) {
+    // the SNI server requires a hostname, so if the hostname is blank,
+    // destroy the socket and fail fast
+    const { hostname } = url.parse(`https://${req.url}`)
+
+    if (!hostname) {
+      browserSocket.destroy()
+
+      return debug(`Invalid hostname for request url ${req.url}`)
+    }
+
     // don't buffer writes - thanks a lot, Nagle
     // https://github.com/cypress-io/cypress/issues/3192
     browserSocket.setNoDelay(true)
@@ -199,6 +209,17 @@ class Server {
 
         return makeConnection(port)
       })
+      .catch((err) => {
+        debug('Error making connection %o', { err })
+
+        browserSocket.destroy(err)
+
+        leave()
+
+        if (this._onError) {
+          return this._onError(err, browserSocket, head)
+        }
+      })
     })
   }
 

packages/https-proxy/test/integration/proxy_spec.js

@@ -215,6 +215,25 @@ describe('Proxy', () => {
         expect(this.proxy._generateMissingCertificates).to.be.calledTwice
       })
     })
+
+    // https://github.com/cypress-io/cypress/issues/9220
+    it('handles errors with addContext', async function () {
+      this.sandbox.spy(this.proxy, 'connect')
+      this.sandbox.stub(this.proxy._sniServer, 'addContext').throws(new Error('error adding context'))
+
+      return request({
+        strictSSL: false,
+        url: 'https://localhost:8443/',
+        proxy: 'http://localhost:3333',
+      }).catch(() => {
+        // This scenario will cause an error but we should clean
+        // ensure the outgoing socket created for this connection was destroyed
+        expect(this.proxy.connect).calledOnce
+        const socket = this.proxy.connect.getCalls()[0].args[1]
+
+        expect(socket.destroyed).to.be.true
+      })
+    })
   })
 
   context('closing', () => {
@@ -229,7 +248,7 @@ describe('Proxy', () => {
       }).then(() => {
         return proxy.start(3333)
       }).then(() => {
-      // force this to reject if its called
+        // force this to reject if its called
         this.sandbox.stub(this.proxy, '_generateMissingCertificates').rejects(new Error('should not call'))
 
         return request({

packages/https-proxy/test/unit/server_spec.js

@@ -92,5 +92,21 @@ describe('lib/server', () => {
         srv._makeConnection(socket, head, '443', '%7Balgolia_application_id%7D-dsn.algolia.net')
       })
     })
+
+    // https://github.com/cypress-io/cypress/issues/9220
+    it('does not crash when a blank URL is parsed and instead only destroys the socket', function (done) {
+      const socket = new EE()
+
+      socket.destroy = this.sandbox.stub()
+      const head = {}
+
+      this.setup()
+      .then((srv) => {
+        srv.connect({ url: '%20:443' }, socket, head)
+        expect(socket.destroy).to.be.calledOnce
+
+        done()
+      })
+    })
   })
 })