Fix "Parse Error" when performing HTTP requests (#5988)

* Detect if NODE_OPTIONS are present in binary; if not, respawn

* Always reset NODE_OPTIONS, even if no ORIGINAL_

Co-authored-by: Andrew Smith <andrew@andrew.codes>

* Exit with correct code # from stub process

* Clean up based on Brian's feedback

* how process.versions is null, i have no idea, but it is

* add repro for invalid header char

* Always pass NODE_OPTIONS with max-http-header-size (#5452)

* cli: set NODE_OPTIONS=--max-http-header-size=1024*1024 on spawn

* electron: remove redundant max-http-header-size

* server: add useCli option to make e2e tests go thru cli

* server: add test for XHR with body > 100kb via CLI

* clean up conditional

* cli: don't pass --max-http-header-size in dev w node < 11.10

* add original_node_options to restore o.g. user node_options

* force no color

* Revert "Use websockets to stub large XHR response bodies instead of hea… (#5525)"

This reverts commit 249db45.

* fix yarn.lock

* update 4_xhr_spec snapshot

* make 6_visit_spec reproduce invalid header char error

* pass --http-parser=legacy

* still set headers if an ERR_INVALID_CHAR is raised

* add --http-parser=legacy in some more places

* update http_requests_spec

* readd spawn_spec

* improve debug logging

* remove unnecessary changes

* cleanup

* revert yarn.lock to develop

* use cp.spawn, not cp.fork

to work around the Electron patch: https://github.com/electron/electron/blob/39baf6879011c0fe8cc975c7585567c7ed0aeed8/patches/node/refactor_alter_child_process_fork_to_use_execute_script_with.patch

Co-authored-by: Andrew Smith <andrew@andrew.codes>

cli/__snapshots__/spawn_spec.js

@@ -1,3 +1,22 @@
+exports['lib/exec/spawn .start forces colors and streams when supported 1'] = {
+  "FORCE_COLOR": "1",
+  "DEBUG_COLORS": "1",
+  "MOCHA_COLORS": "1",
+  "FORCE_STDIN_TTY": "1",
+  "FORCE_STDOUT_TTY": "1",
+  "FORCE_STDERR_TTY": "1",
+  "NODE_OPTIONS": "--max-http-header-size=1048576 --http-parser=legacy"
+}
+
+exports['lib/exec/spawn .start does not force colors and streams when not supported 1'] = {
+  "FORCE_COLOR": "0",
+  "DEBUG_COLORS": "0",
+  "FORCE_STDIN_TTY": "0",
+  "FORCE_STDOUT_TTY": "0",
+  "FORCE_STDERR_TTY": "0",
+  "NODE_OPTIONS": "--max-http-header-size=1048576 --http-parser=legacy"
+}
+
 exports['lib/exec/spawn .start detects kill signal exits with error on SIGKILL 1'] = `
 The Test Runner unexpectedly exited via a exit event with signal SIGKILL
 

cli/lib/exec/spawn.js

@@ -109,7 +109,7 @@ module.exports = {
         }
 
         const { onStderrData, electronLogging } = overrides
-        const envOverrides = util.getEnvOverrides()
+        const envOverrides = util.getEnvOverrides(options)
         const electronArgs = _.clone(args)
         const node11WindowsFix = isPlatform('win32')
 

cli/lib/util.js

@@ -268,7 +268,7 @@ const util = {
     return isCi
   },
 
-  getEnvOverrides () {
+  getEnvOverrides (options = {}) {
     return _
     .chain({})
     .extend(util.getEnvColors())
@@ -277,9 +277,35 @@ const util = {
     .mapValues((value) => { // stringify to 1 or 0
       return value ? '1' : '0'
     })
+    .extend(util.getNodeOptions(options))
     .value()
   },
 
+  getNodeOptions (options, nodeVersion) {
+    if (!nodeVersion) {
+      nodeVersion = Number(process.versions.node.split('.')[0])
+    }
+
+    if (options.dev && nodeVersion < 12) {
+      // `node` is used instead of Electron when --dev is passed, so this won't work if Node is too old
+      debug('NODE_OPTIONS=--max-http-header-size could not be set because we\'re in dev mode and Node is < 12.0.0')
+
+      return
+    }
+
+    // https://github.com/cypress-io/cypress/issues/5431
+    const NODE_OPTIONS = `--max-http-header-size=${1024 ** 2} --http-parser=legacy`
+
+    if (_.isString(process.env.NODE_OPTIONS)) {
+      return {
+        NODE_OPTIONS: `${NODE_OPTIONS} ${process.env.NODE_OPTIONS}`,
+        ORIGINAL_NODE_OPTIONS: process.env.NODE_OPTIONS || '',
+      }
+    }
+
+    return { NODE_OPTIONS }
+  },
+
   getForceTty () {
     return {
       FORCE_STDIN_TTY: util.isTty(process.stdin.fd),

cli/test/lib/exec/spawn_spec.js

@@ -329,14 +329,7 @@ describe('lib/exec/spawn', function () {
 
       return spawn.start([], { env: {} })
       .then(() => {
-        expect(cp.spawn.firstCall.args[2].env).to.deep.eq({
-          FORCE_COLOR: '1',
-          DEBUG_COLORS: '1',
-          MOCHA_COLORS: '1',
-          FORCE_STDERR_TTY: '1',
-          FORCE_STDIN_TTY: '1',
-          FORCE_STDOUT_TTY: '1',
-        })
+        snapshot(cp.spawn.firstCall.args[2].env)
       })
     })
 
@@ -368,13 +361,7 @@ describe('lib/exec/spawn', function () {
 
       return spawn.start([], { env: {} })
       .then(() => {
-        expect(cp.spawn.firstCall.args[2].env).to.deep.eq({
-          FORCE_COLOR: '0',
-          DEBUG_COLORS: '0',
-          FORCE_STDERR_TTY: '0',
-          FORCE_STDIN_TTY: '0',
-          FORCE_STDOUT_TTY: '0',
-        })
+        snapshot(cp.spawn.firstCall.args[2].env)
       })
     })
 

cli/test/lib/util_spec.js

@@ -3,6 +3,7 @@ require('../spec_helper')
 const os = require('os')
 const tty = require('tty')
 const snapshot = require('../support/snapshot')
+const mockedEnv = require('mocked-env')
 const supportsColor = require('supports-color')
 const proxyquire = require('proxyquire')
 const hasha = require('hasha')
@@ -11,6 +12,9 @@ const la = require('lazy-ass')
 const util = require(`${lib}/util`)
 const logger = require(`${lib}/logger`)
 
+// https://github.com/cypress-io/cypress/issues/5431
+const expectedNodeOptions = `--max-http-header-size=${1024 * 1024} --http-parser=legacy`
+
 describe('util', () => {
   beforeEach(() => {
     sinon.stub(process, 'exit')
@@ -213,6 +217,7 @@ describe('util', () => {
         FORCE_COLOR: '1',
         DEBUG_COLORS: '1',
         MOCHA_COLORS: '1',
+        NODE_OPTIONS: expectedNodeOptions,
       })
 
       util.supportsColor.returns(false)
@@ -224,7 +229,46 @@ describe('util', () => {
         FORCE_STDERR_TTY: '0',
         FORCE_COLOR: '0',
         DEBUG_COLORS: '0',
+        NODE_OPTIONS: expectedNodeOptions,
+      })
+    })
+  })
+
+  context('.getNodeOptions', () => {
+    let restoreEnv
+
+    afterEach(() => {
+      if (restoreEnv) {
+        restoreEnv()
+        restoreEnv = null
+      }
+    })
+
+    it('adds required NODE_OPTIONS', () => {
+      restoreEnv = mockedEnv({
+        NODE_OPTIONS: undefined,
       })
+
+      expect(util.getNodeOptions({})).to.deep.eq({
+        NODE_OPTIONS: expectedNodeOptions,
+      })
+    })
+
+    it('includes existing NODE_OPTIONS', () => {
+      restoreEnv = mockedEnv({
+        NODE_OPTIONS: '--foo --bar',
+      })
+
+      expect(util.getNodeOptions({})).to.deep.eq({
+        NODE_OPTIONS: `${expectedNodeOptions} --foo --bar`,
+        ORIGINAL_NODE_OPTIONS: '--foo --bar',
+      })
+    })
+
+    it('does not return if dev is set and version < 12', () => {
+      expect(util.getNodeOptions({
+        dev: true,
+      }, 11)).to.be.undefined
     })
   })
 

packages/driver/src/cy/commands/navigation.coffee

@@ -86,7 +86,7 @@ cannotVisitDifferentOrigin = (origin, previousUrlVisited, remoteUrl, existingUrl
       previousUrl: previousUrlVisited
       attemptedUrl: origin
     }
-  } 
+  }
 
   $errUtils.throwErrByPath("visit.cannot_visit_different_origin", errOpts)
 
@@ -294,6 +294,10 @@ normalizeTimeoutOptions = (options) ->
 module.exports = (Commands, Cypress, cy, state, config) ->
   reset()
 
+  Cypress.on "test:before:run:async", ->
+    ## reset any state on the backend
+    Cypress.backend('reset:server:state')
+
   Cypress.on("test:before:run", reset)
 
   Cypress.on "stability:changed", (bool, event) ->

packages/driver/src/cy/commands/xhr.coffee

@@ -88,9 +88,6 @@ startXhrServer = (cy, state, config) ->
     xhrUrl: config("xhrUrl")
     stripOrigin: stripOrigin
 
-    emitIncoming: (id, route) ->
-      Cypress.backend('incoming:xhr', id, route)
-
     ## shouldnt these stubs be called routes?
     ## rename everything related to stubs => routes
     onSend: (xhr, stack, route) =>
@@ -252,10 +249,6 @@ module.exports = (Commands, Cypress, cy, state, config) ->
   ## correctly
   Cypress.on("window:unload", cancelPendingXhrs)
 
-  Cypress.on "test:before:run:async", ->
-    ## reset any state on the backend
-    Cypress.backend('reset:server:state')
-
   Cypress.on "test:before:run", ->
     ## reset the existing server
     reset()
@@ -266,7 +259,7 @@ module.exports = (Commands, Cypress, cy, state, config) ->
     ## window such as if the last test ended
     ## with a cross origin window
     try
-      server = startXhrServer(cy, state, config, Cypress)
+      server = startXhrServer(cy, state, config)
     catch err
       ## in this case, just don't bind to the server
       server = null

packages/driver/src/cypress/server.coffee

@@ -11,8 +11,11 @@ props                    = "onreadystatechange onload onerror".split(" ")
 
 restoreFn = null
 
-setHeader = (xhr, key, val) ->
+setHeader = (xhr, key, val, transformer) ->
   if val?
+    if transformer
+      val = transformer(val)
+
     key = "X-Cypress-" + _.capitalize(key)
     xhr.setRequestHeader(key, encodeURI(val))
 
@@ -174,30 +177,18 @@ create = (options = {}) ->
       hasEnabledStubs and route and route.response?
 
     applyStubProperties: (xhr, route) ->
-      responseToString = =>
-        if not _.isString(route.response)
-          return JSON.stringify(route.response)
-
-        route.response
-
-      response = responseToString()
-
-      headers = {
-        "id": xhr.id
-        "status": route.status
-        "matched": route.url + ""
-        "delay": route.delay
-        "headers": transformHeaders(route.headers)
-      }
-
-      if response.length > 4096
-        options.emitIncoming(xhr.id, response)
-        headers.responseDeferred = true
-      else
-        headers.response = response
-
-      _.map headers, (v, k) =>
-        setHeader(xhr, k, v)
+      responser = if _.isObject(route.response) then JSON.stringify else null
+
+      ## add header properties for the xhr's id
+      ## and the testId
+      setHeader(xhr, "id", xhr.id)
+      # setHeader(xhr, "testId", options.testId)
+
+      setHeader(xhr, "status",   route.status)
+      setHeader(xhr, "response", route.response, responser)
+      setHeader(xhr, "matched",  route.url + "")
+      setHeader(xhr, "delay",    route.delay)
+      setHeader(xhr, "headers",  route.headers, transformHeaders)
 
     route: (attrs = {}) ->
       warnOnStubDeprecation(attrs, "route")

packages/electron/lib/electron.js

@@ -85,7 +85,7 @@ module.exports = {
 
       // max HTTP header size 8kb -> 1mb
       // https://github.com/cypress-io/cypress/issues/76
-      argv.unshift(`--max-http-header-size=${1024 * 1024}`)
+      argv.unshift(`--max-http-header-size=${1024 * 1024} --http-parser=legacy`)
 
       debug('spawning %s with args', execPath, argv)
 

packages/proxy/lib/http/response-middleware.ts

@@ -119,16 +119,70 @@ const LogResponse: ResponseMiddleware = function () {
 }
 
 const PatchExpressSetHeader: ResponseMiddleware = function () {
+  const { incomingRes } = this
   const originalSetHeader = this.res.setHeader
 
-  // express.Response.setHeader does all kinds of silly/nasty stuff to the content-type...
-  // but we don't want to change it at all!
-  this.res.setHeader = (k, v) => {
-    if (k === 'content-type') {
-      v = this.incomingRes.headers['content-type'] || v
+  // Node uses their own Symbol object, so use this to get the internal kOutHeaders
+  // symbol - Symbol.for('kOutHeaders') will not work
+  const getKOutHeadersSymbol = () => {
+    const findKOutHeadersSymbol = (): symbol => {
+      return _.find(Object.getOwnPropertySymbols(this.res), (sym) => {
+        return sym.toString() === 'Symbol(kOutHeaders)'
+      })!
     }
 
-    return originalSetHeader.call(this.res, k, v)
+    let sym = findKOutHeadersSymbol()
+
+    if (sym) {
+      return sym
+    }
+
+    // force creation of a new header field so the kOutHeaders key is available
+    this.res.setHeader('X-Cypress-HTTP-Response', 'X')
+    this.res.removeHeader('X-Cypress-HTTP-Response')
+
+    sym = findKOutHeadersSymbol()
+
+    if (!sym) {
+      throw new Error('unable to find kOutHeaders symbol')
+    }
+
+    return sym
+  }
+
+  let kOutHeaders
+
+  this.res.setHeader = function (name, value) {
+    // express.Response.setHeader does all kinds of silly/nasty stuff to the content-type...
+    // but we don't want to change it at all!
+    if (name === 'content-type') {
+      value = incomingRes.headers['content-type'] || value
+    }
+
+    // run the original function - if an "invalid header char" error is raised,
+    // set the header manually. this way we can retain Node's original error behavior
+    try {
+      return originalSetHeader.call(this, name, value)
+    } catch (err) {
+      if (err.code !== 'ERR_INVALID_CHAR') {
+        throw err
+      }
+
+      debug('setHeader error ignored %o', { name, value, code: err.code, err })
+
+      if (!kOutHeaders) {
+        kOutHeaders = getKOutHeadersSymbol()
+      }
+
+      // https://github.com/nodejs/node/blob/42cce5a9d0fd905bf4ad7a2528c36572dfb8b5ad/lib/_http_outgoing.js#L483-L495
+      let headers = this[kOutHeaders]
+
+      if (!headers) {
+        this[kOutHeaders] = headers = Object.create(null)
+      }
+
+      headers[name.toLowerCase()] = [name, value]
+    }
   }
 
   this.next()