Do not send requests for SNI server through upstream proxy (#4275)

* add test to ensure that SNI server will never go through proxy

* prevent test from false positive

* ensure that SNI server requests never go through proxy

* e2e test that https-proxy does not pass sni reqs thru upstream

* improve debug logging in https-proxy

* fix using cwd, not workspaceFolder for terminals manager

* remove dead code

* stop the debug proxy after each test


Co-authored-by: Brian Mann <brian.mann86@gmail.com>

.vscode/terminals.json

@@ -20,43 +20,43 @@
       "focus": true,
       "onlySingle": true,
       "execute": false,
-      "cwd": "[workspaceFolder]/packages/server",
+      "cwd": "[cwd]/packages/server",
       "command": "npm run test-watch -- [file]"
     },
     {
       "name": "packages/server test-e2e",
       "focus": true,
       "onlySingle": true,
       "execute": false,
-      "cwd": "[workspaceFolder]/packages/server",
+      "cwd": "[cwd]/packages/server",
       "command": "npm run test-e2e -- --spec name"
     },
     {
       "name": "packages/runner watch",
       "focus": true,
       "onlySingle": true,
-      "cwd": "[workspaceFolder]/packages/runner",
+      "cwd": "[cwd]/packages/runner",
       "command": "npm run watch"
     },
     {
       "name": "packages/driver cypress open",
       "focus": true,
       "onlySingle": true,
-      "cwd": "[workspaceFolder]/packages/driver",
+      "cwd": "[cwd]/packages/driver",
       "command": "npm run cypress:open"
     },
     {
       "name": "packages/desktop-gui cypress open",
       "focus": true,
       "onlySingle": true,
-      "cwd": "[workspaceFolder]/packages/desktop-gui",
+      "cwd": "[cwd]/packages/desktop-gui",
       "command": "npm run cypress:open"
     },
     {
       "name": "packages/desktop-gui watch",
       "focus": true,
       "onlySingle": true,
-      "cwd": "[workspaceFolder]/packages/desktop-gui",
+      "cwd": "[cwd]/packages/desktop-gui",
       "command": "npm run watch"
     }
   ]

packages/https-proxy/lib/server.coffee

@@ -31,7 +31,7 @@ class Server
     ## https://github.com/cypress-io/cypress/issues/3192
     browserSocket.setNoDelay(true)
 
-    debug("Writing browserSocket connection headers %o", { url: req.url })
+    debug("Writing browserSocket connection headers %o", { url: req.url, headLength: _.get(head, 'length'), headers: req.headers })
 
     browserSocket.on "error", (err) =>
       ## TODO: shouldn't we destroy the upstream socket here?
@@ -62,6 +62,8 @@ class Server
       @_onFirstHeadBytes(req, browserSocket, data, options)
 
   _onFirstHeadBytes: (req, browserSocket, head, options) ->
+    debug("Got first head bytes %o", { url: req.url, head: _.chain(head).invoke('toString').slice(0, 64).join('').value() })
+
     browserSocket.pause()
 
     if odc = options.onDirectConnection
@@ -96,8 +98,16 @@ class Server
       res.end()
     .pipe(res)
 
+  _getProxyForUrl: (url) ->
+    if url == "https://localhost:#{@_sniPort}"
+      ## https://github.com/cypress-io/cypress/issues/4257
+      ## this is a tunnel to the SNI server, it should never go through a proxy
+      return undefined
+
+    getProxyForUrl(url)
+
   _makeDirectConnection: (req, browserSocket, head) ->
-    { port, hostname } = url.parse("http://#{req.url}")
+    { port, hostname } = url.parse("https://#{req.url}")
 
     debug("Making connection to #{hostname}:#{port}")
     @_makeConnection(browserSocket, head, port, hostname)
@@ -124,7 +134,7 @@ class Server
 
       browserSocket.resume()
 
-    if upstreamProxy = getProxyForUrl("https://#{hostname}:#{port}")
+    if upstreamProxy = @_getProxyForUrl("https://#{hostname}:#{port}")
       # todo: as soon as all requests are intercepted, this can go away since this is just for pass-through
       debug("making proxied connection %o", {
         host: "#{hostname}:#{port}",
@@ -149,7 +159,7 @@ class Server
     makeConnection = (port) =>
       debug("Making intercepted connection to %s", port)
 
-      @_makeConnection(browserSocket, head, port)
+      @_makeConnection(browserSocket, head, port, "localhost")
 
     if firstBytes not in SSL_RECORD_TYPES
       ## if this isn't an SSL request then go

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

@@ -5,6 +5,7 @@ process.env.NODE_TLS_REJECT_UNAUTHORIZED = "0"
 _           = require("lodash")
 DebugProxy  = require("@cypress/debugging-proxy")
 net         = require("net")
+network     = require("@packages/network")
 path        = require("path")
 Promise     = require("bluebird")
 proxy       = require("../helpers/proxy")
@@ -241,6 +242,27 @@ describe "Proxy", ->
             expect(socket.destroyed).to.be.true
             resolve()
 
+    ## https://github.com/cypress-io/cypress/issues/4257
+    it "passes through to SNI when it is intercepted and not through proxy", ->
+      createSocket = @sandbox.stub(network.connect, 'createRetryingSocket').callsArgWith(1, new Error('stub'))
+      createProxyConn = @sandbox.spy(network.agent.httpsAgent, 'createUpstreamProxyConnection')
+
+      request({
+        strictSSL: false
+        url: "https://localhost:8443"
+        proxy: "http://localhost:3333"
+        resolveWithFullResponse: true
+        forever: false
+      })
+      .then =>
+        throw new Error('should not succeed')
+      .catch { message: 'Error: socket hang up' }, =>
+        expect(createProxyConn).to.not.be.called
+        expect(createSocket).to.be.calledWith({
+          port: @proxy._sniPort
+          host: 'localhost'
+        })
+
     afterEach ->
       @upstream.stop()
       delete process.env.HTTP_PROXY

packages/https-proxy/test/spec_helper.coffee

@@ -5,6 +5,7 @@ sinonChai     = require("sinon-chai")
 sinonPromise  = require("sinon-as-promised")(Promise)
 
 global.request   = require("request-promise")
+global.sinon     = sinon
 global.supertest = require("supertest")
 
 chai.use(sinonChai)
@@ -15,4 +16,4 @@ beforeEach ->
   @sandbox = sinon.sandbox.create()
 
 afterEach ->
-  @sandbox.restore()
+  @sandbox.restore()

packages/server/__snapshots__/8_network_error_handling_spec.coffee.js

@@ -78,3 +78,65 @@ exports['e2e network error handling Cypress retries HTTPS passthrough behind a p
 
 
 `
+
+exports['e2e network error handling Cypress does not connect to the upstream proxy for the SNI server request 1'] = `
+
+====================================================================================================
+
+  (Run Starting)
+
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ Cypress:    1.2.3                                                                              │
+  │ Browser:    FooBrowser 88                                                                      │
+  │ Specs:      1 found (https_passthru_spec.js)                                                   │
+  │ Searched:   cypress/integration/https_passthru_spec.js                                         │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+
+
+────────────────────────────────────────────────────────────────────────────────────────────────────
+                                                                                                    
+  Running: https_passthru_spec.js...                                                       (1 of 1) 
+
+
+  https passthru retries
+    ✓ retries when visiting a non-test domain
+    ✓ passes through the network error when it cannot connect to the proxy
+
+
+  2 passing
+
+
+  (Results)
+
+  ┌──────────────────────────────────────┐
+  │ Tests:        2                      │
+  │ Passing:      2                      │
+  │ Failing:      0                      │
+  │ Pending:      0                      │
+  │ Skipped:      0                      │
+  │ Screenshots:  0                      │
+  │ Video:        true                   │
+  │ Duration:     X seconds              │
+  │ Spec Ran:     https_passthru_spec.js │
+  └──────────────────────────────────────┘
+
+
+  (Video)
+
+  - Started processing:   Compressing to 32 CRF
+  - Finished processing:  /foo/bar/.projects/e2e/cypress/videos/abc123.mp4 (X seconds)
+
+
+====================================================================================================
+
+  (Run Finished)
+
+
+      Spec                                                Tests  Passing  Failing  Pending  Skipped 
+  ┌────────────────────────────────────────────────────────────────────────────────────────────────┐
+  │ ✔ https_passthru_spec.js                    XX:XX        2        2        -        -        - │
+  └────────────────────────────────────────────────────────────────────────────────────────────────┘
+    All specs passed!                           XX:XX        2        2        -        -        -  
+
+
+`

packages/server/lib/request.coffee

@@ -305,11 +305,6 @@ createRetryingRequestStream = (opts = {}) ->
   emitError = (err) ->
     retryStream.emit("error", err)
 
-    ## TODO: we probably want to destroy
-    ## the stream, but leaving in the error emit
-    ## temporarily until we finish implementation
-    # retryStream.destroy(err)
-
   tryStartStream = ->
     ## if our request has been aborted
     ## in the time that we were waiting to retry

packages/server/lib/util/ensure-url.ts

@@ -1,9 +1,12 @@
-import Bluebird from 'bluebird'
 import _ from 'lodash'
+import Bluebird from 'bluebird'
+import debugModule from 'debug'
 import rp from 'request-promise'
 import * as url from 'url'
 import { agent, connect } from '@packages/network'
 
+const debug = debugModule('cypress:server:ensure-url')
+
 type RetryOptions = {
   retryIntervals: number[]
   onRetry: Function
@@ -15,6 +18,12 @@ export const retryIsListening = (urlStr: string, options: RetryOptions) => {
   const delaysRemaining = _.clone(retryIntervals)
 
   const run = () => {
+    debug('checking that baseUrl is available', {
+      baseUrl: urlStr,
+      delaysRemaining,
+      retryIntervals
+    })
+
     return isListening(urlStr)
     .catch((err) => {
       const delay = delaysRemaining.shift()

packages/server/test/e2e/8_network_error_handling_spec.coffee

@@ -268,6 +268,10 @@ describe "e2e network error handling", ->
       delete process.env.HTTP_PROXY
       delete process.env.NO_PROXY
 
+    afterEach ->
+      if @debugProxy
+        @debugProxy.stop()
+
     it "baseurl check tries 5 times in run mode", ->
       e2e.exec(@, {
         config: {
@@ -359,9 +363,11 @@ describe "e2e network error handling", ->
             ## server as expected
             return true
 
-      new DebugProxy({
+      @debugProxy = new DebugProxy({
         onConnect
       })
+
+      @debugProxy
       .start(PROXY_PORT)
       .then =>
         process.env.HTTP_PROXY = "http://localhost:#{PROXY_PORT}"
@@ -377,3 +383,40 @@ describe "e2e network error handling", ->
 
           expect(connectCounts["localhost:#{HTTPS_PORT}"]).to.be.gte(3)
           expect(connectCounts["localhost:#{ERR_HTTPS_PORT}"]).to.be.gte(4)
+
+    it "does not connect to the upstream proxy for the SNI server request", ->
+      onConnect = sinon.spy ->
+        true
+
+      @debugProxy = new DebugProxy({
+        onConnect
+      })
+
+      @debugProxy
+      .start(PROXY_PORT)
+      .then =>
+        process.env.HTTP_PROXY = "http://localhost:#{PROXY_PORT}"
+        process.env.NO_PROXY = "localhost:13373" ## proxy everything except for the irrelevant test
+
+        e2e.exec(@, {
+          spec: "https_passthru_spec.js"
+          snapshot: true
+          expectedExitCode: 0
+          config: {
+            baseUrl: "https://localhost:#{HTTPS_PORT}"
+          }
+        })
+        .then ->
+          expect(onConnect).to.be.calledTwice
+
+          ## 1st request: verifying base url
+          expect(onConnect.firstCall).to.be.calledWithMatch({
+            host: 'localhost'
+            port: HTTPS_PORT
+          })
+
+          ## 2nd request: <img> load from spec
+          expect(onConnect.secondCall).to.be.calledWithMatch({
+            host: 'localhost'
+            port: HTTPS_PORT
+          })