diff --git a/packages/https-proxy/lib/server.js b/packages/https-proxy/lib/server.js index ccd2a373d2..7115c3075c 100644 --- a/packages/https-proxy/lib/server.js +++ b/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) + } + }) }) } diff --git a/packages/https-proxy/test/integration/proxy_spec.js b/packages/https-proxy/test/integration/proxy_spec.js index f3b7562a20..adda9be22a 100644 --- a/packages/https-proxy/test/integration/proxy_spec.js +++ b/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({ diff --git a/packages/https-proxy/test/unit/server_spec.js b/packages/https-proxy/test/unit/server_spec.js index f9c29f5fbb..8439e422f1 100644 --- a/packages/https-proxy/test/unit/server_spec.js +++ b/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() + }) + }) }) })