Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/https-proxy/lib/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
})
})
}

Expand Down
21 changes: 20 additions & 1 deletion packages/https-proxy/test/integration/proxy_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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({
Expand Down
16 changes: 16 additions & 0 deletions packages/https-proxy/test/unit/server_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
})
})
})
})