Fix proxying HTTPS requests to IP addresses

packages/https-proxy/lib/server.coffee

@@ -1,6 +1,5 @@
 _            = require("lodash")
-{ agent, connect } = require("@packages/network")
-allowDestroy = require("server-destroy-vvo")
+{ agent, allowDestroy, connect } = require("@packages/network")
 debug        = require("debug")("cypress:https-proxy")
 fs           = require("fs-extra")
 getProxyForUrl = require("proxy-from-env").getProxyForUrl
@@ -23,8 +22,9 @@ SSL_RECORD_TYPES = [
 ]
 
 class Server
-  constructor: (@_ca, @_port) ->
+  constructor: (@_ca, @_port, @_options) ->
     @_onError = null
+    @_ipServers = {}
 
   connect: (req, browserSocket, head, options = {}) ->
     ## don't buffer writes - thanks a lot, Nagle
@@ -217,25 +217,47 @@ class Server
 
   _getPortFor: (hostname) ->
     @_getCertificatePathsFor(hostname)
-
     .catch (err) =>
       @_generateMissingCertificates(hostname)
-
     .then (data = {}) =>
+      if net.isIP(hostname)
+        return @_getServerPortForIp(hostname, data)
+
       @_sniServer.addContext(hostname, data)
 
       return @_sniPort
 
-  listen: (options = {}) ->
+  ## browsers will not do SNI for an IP address, so we need to serve
+  ## 1 HTTPS server per IP
+  ## https://github.com/cypress-io/cypress/issues/771
+  _getServerPortForIp: (ip, data) =>
+    if server = @_ipServers[ip]
+      return server.address().port
+
+    new Promise (resolve) =>
+      @_ipServers[ip] = server = https.createServer(data)
+
+      allowDestroy(server)
+
+      server.on "upgrade", @_onUpgrade.bind(@, @_options.onUpgrade)
+      server.on "request", @_onRequest.bind(@, @_options.onRequest)
+      server.listen 0, '127.0.0.1', =>
+        port = server.address().port
+
+        debug("Created HTTPS Proxy for IP %s on port %s", ip, port)
+
+        resolve(port)
+
+  listen: ->
     new Promise (resolve) =>
-      @_onError = options.onError
+      @_onError = @_options.onError
 
       @_sniServer = https.createServer({})
 
       allowDestroy(@_sniServer)
 
-      @_sniServer.on "upgrade", @_onUpgrade.bind(@, options.onUpgrade)
-      @_sniServer.on "request", @_onRequest.bind(@, options.onRequest)
+      @_sniServer.on "upgrade", @_onUpgrade.bind(@, @_options.onUpgrade)
+      @_sniServer.on "request", @_onRequest.bind(@, @_options.onRequest)
       @_sniServer.listen 0, '127.0.0.1', =>
         ## store the port of our current sniServer
         @_sniPort = @_sniServer.address().port
@@ -246,8 +268,10 @@ class Server
 
   close: ->
     close = =>
-      new Promise (resolve) =>
-        @_sniServer.destroy(resolve)
+      servers = _.values(@_ipServers).concat([@_sniServer])
+      Promise.map servers, (server) ->
+        Promise.fromCallback (cb) ->
+          server.destroy(cb)
 
     close()
     .finally ->
@@ -258,9 +282,9 @@ module.exports = {
     sslServers = {}
 
   create: (ca, port, options = {}) ->
-    srv = new Server(ca, port)
+    srv = new Server(ca, port, options)
 
     srv
-    .listen(options)
+    .listen()
     .return(srv)
 }

packages/https-proxy/package.json

@@ -21,10 +21,9 @@
     "debug": "4.1.1",
     "fs-extra": "8.1.0",
     "lodash": "4.17.15",
-    "node-forge": "0.6.49",
+    "node-forge": "0.8.5",
     "proxy-from-env": "1.0.0",
-    "semaphore": "1.1.0",
-    "server-destroy-vvo": "1.0.1"
+    "semaphore": "1.1.0"
   },
   "devDependencies": {
     "@cypress/debugging-proxy": "2.0.1",

packages/https-proxy/test/helpers/https_server.coffee

@@ -1,6 +1,6 @@
 https = require("https")
 Promise = require("bluebird")
-allowDestroy = require("server-destroy-vvo")
+{ allowDestroy } = require("@packages/network")
 certs = require("./certs")
 
 defaultOnRequest = (req, res) ->

packages/https-proxy/test/helpers/proxy.coffee

@@ -1,3 +1,4 @@
+{ allowDestroy } = require("@packages/network")
 http       = require("http")
 path       = require("path")
 httpsProxy = require("../../lib/proxy")
@@ -28,6 +29,8 @@ module.exports = {
   start: (port) ->
     prx = http.createServer()
 
+    allowDestroy(prx)
+
     dir = path.join(process.cwd(), "ca")
 
     httpsProxy.create(dir, port, {
@@ -61,7 +64,7 @@ module.exports = {
 
   stop: ->
     new Promise (resolve) ->
-      prx.close(resolve)
+      prx.destroy(resolve)
     .then ->
       prx.proxy.close()
 }

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

@@ -4,6 +4,8 @@ process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"
 
 _           = require("lodash")
 DebugProxy  = require("@cypress/debugging-proxy")
+fs          = require("fs-extra")
+https       = require("https")
 net         = require("net")
 network     = require("@packages/network")
 path        = require("path")
@@ -152,6 +154,37 @@ describe "Proxy", ->
           proxy: "http://localhost:3333"
         })
 
+    ## https://github.com/cypress-io/cypress/issues/771
+    it "generates certs and can proxy requests for HTTPS requests to IPs", ->
+      @sandbox.spy(@proxy, "_generateMissingCertificates")
+      @sandbox.spy(@proxy, "_getServerPortForIp")
+
+      remove = (folder, file) =>
+        fs.removeAsync(path.join(folder, file))
+
+      Promise.all([
+        httpsServer.start(8445),
+        remove(@proxy._ca.certsFolder, '127.0.0.1.pem'),
+        remove(@proxy._ca.keysFolder, '127.0.0.1.key'),
+        remove(@proxy._ca.keysFolder, '127.0.0.1.public.key')
+      ])
+      .then =>
+        request({
+          strictSSL: false
+          url: "https://127.0.0.1:8445/"
+          proxy: "http://localhost:3333"
+        })
+      .then =>
+        ## this should not stand up its own https server
+        request({
+          strictSSL: false
+          url: "https://localhost:8443/"
+          proxy: "http://localhost:3333"
+        })
+      .then =>
+        expect(@proxy._ipServers["127.0.0.1"]).to.be.an.instanceOf(https.Server)
+        expect(@proxy._getServerPortForIp).to.be.calledWith('127.0.0.1', sinon.match.any)
+
   context "closing", ->
     it "resets sslServers and can reopen", ->
       request({
@@ -256,7 +289,7 @@ describe "Proxy", ->
       })
       .then =>
         throw new Error('should not succeed')
-      .catch { message: 'Error: socket hang up' }, =>
+      .catch { message: "Error: socket hang up" }, =>
         expect(createProxyConn).to.not.be.called
         expect(createSocket).to.be.calledWith({
           port: @proxy._sniPort

packages/network/lib/allow-destroy.ts

@@ -0,0 +1,31 @@
+import net from 'net'
+
+/**
+ * `allowDestroy` adds a `destroy` method to a `net.Server`. `destroy(cb)`
+ * will kill all open connections and call `cb` when the server is closed.
+ *
+ * Note: `server-destroy` NPM package cannot be used - it does not track
+ * `secureConnection` events.
+ */
+export function allowDestroy (server: net.Server) {
+  let connections: net.Socket[] = []
+
+  function trackConn (conn) {
+    connections.push(conn)
+
+    conn.on('close', () => {
+      connections = connections.filter((connection) => connection !== conn)
+    })
+  }
+
+  server.on('connection', trackConn)
+  server.on('secureConnection', trackConn)
+
+  // @ts-ignore Property 'destroy' does not exist on type 'Server'.
+  server.destroy = function (cb) {
+    server.close(cb)
+    connections.map((connection) => connection.destroy())
+  }
+
+  return server
+}

packages/network/lib/index.ts

@@ -1,6 +1,9 @@
 import agent from './agent'
 import * as connect from './connect'
+import { allowDestroy } from './allow-destroy'
 
-export { agent }
-
-export { connect }
+export {
+  agent,
+  allowDestroy,
+  connect,
+}

packages/network/test/support/servers.ts

@@ -4,36 +4,14 @@ import http from 'http'
 import https from 'https'
 import Io from '@packages/socket'
 import Promise from 'bluebird'
+import { allowDestroy } from '../../lib/allow-destroy'
 
 export interface AsyncServer {
   closeAsync: () => Promise<void>
   destroyAsync: () => Promise<void>
   listenAsync: (port) => Promise<void>
 }
 
-function addDestroy (server: http.Server | https.Server) {
-  let connections = []
-
-  function trackConn (conn) {
-    connections.push(conn)
-
-    conn.on('close', () => {
-      connections = connections.filter((connection) => connection !== conn)
-    })
-  }
-
-  server.on('connection', trackConn)
-  server.on('secureConnection', trackConn)
-
-  // @ts-ignore Property 'destroy' does not exist on type 'Server'.
-  server.destroy = function (cb) {
-    server.close(cb)
-    connections.map((connection) => connection.destroy())
-  }
-
-  return server
-}
-
 function createExpressApp () {
   const app: express.Application = express()
 
@@ -72,14 +50,14 @@ export class Servers {
     )
     .spread((app: Express.Application, [cert, key]: string[]) => {
       this.httpServer = Promise.promisifyAll(
-        addDestroy(http.createServer(app))
+        allowDestroy(http.createServer(app))
       ) as http.Server & AsyncServer
 
       this.wsServer = Io.server(this.httpServer)
 
       this.https = { cert, key }
       this.httpsServer = Promise.promisifyAll(
-        addDestroy(https.createServer(this.https, <http.RequestListener>app))
+        allowDestroy(https.createServer(this.https, <http.RequestListener>app))
       ) as https.Server & AsyncServer
 
       this.wssServer = Io.server(this.httpsServer)

packages/network/test/unit/agent_spec.ts

@@ -15,6 +15,7 @@ import {
   regenerateRequestHead, CombinedAgent,
 } from '../../lib/agent'
 import { AsyncServer, Servers } from '../support/servers'
+import { allowDestroy } from '../../lib/allow-destroy'
 
 const expect = chai.expect
 
@@ -273,9 +274,11 @@ describe('lib/agent', function () {
 
       it('#createUpstreamProxyConnection throws when connection is accepted then closed', function () {
         const proxy = Bluebird.promisifyAll(
-          net.createServer((socket) => {
-            socket.end()
-          })
+          allowDestroy(
+            net.createServer((socket) => {
+              socket.end()
+            })
+          )
         ) as net.Server & AsyncServer
 
         const proxyPort = PROXY_PORT + 2

packages/server/lib/server.coffee

@@ -701,6 +701,7 @@ class Server
       @_fileServer?.close()
       @_httpsProxy?.close()
     )
+    .catch { message: "Not running" }, _.noop
     .then =>
       ## reset any middleware
       @_middleware = null

packages/server/lib/util/server_destroy.js

@@ -1,12 +1,5 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Sanity-check the conversion and remove this comment.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const Promise = require('bluebird')
-const allowDestroy = require('server-destroy')
+const { allowDestroy } = require('@packages/network')
 
 module.exports = function (server) {
   allowDestroy(server)
@@ -16,4 +9,4 @@ module.exports = function (server) {
     .catch(() => {})
   }
 }
-//# dont catch any errors
+// dont catch any errors

packages/server/package.json

@@ -118,7 +118,6 @@
     "sanitize-filename": "1.6.1",
     "semver": "6.3.0",
     "send": "0.17.0",
-    "server-destroy": "1.0.1",
     "shell-env": "3.0.0",
     "signal-exit": "3.0.2",
     "sinon": "5.1.1",