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
cypress-io · Mar 11, 2020 · 26bee79 · 26bee79
1 parent c656bfa
commit 26bee79
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 43 deletions.
diff --git a/cli/__snapshots__/spawn_spec.js b/cli/__snapshots__/spawn_spec.js
@@ -1,18 +1,18 @@
-exports['lib/exec/spawn .start detects kill signal exits with error on SIGKILL 1'] = `
-The Test Runner unexpectedly exited via a [36mexit[39m event with signal [36mSIGKILL[39m
+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"
+}
 
-Please search Cypress documentation for possible solutions:
-
-[34mhttps://on.cypress.io[39m
-
-Check if there is a GitHub issue describing this crash:
-
-[34mhttps://github.com/cypress-io/cypress/issues[39m
-
-Consider opening a new issue.
-
-----------
-
-Platform: darwin (Foo-OsVersion)
-Cypress Version: 0.0.0
-`
+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"
+}
diff --git a/cli/lib/exec/spawn.js b/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')
 

diff --git a/cli/lib/util.js b/cli/lib/util.js
@@ -235,7 +235,7 @@ const util = {
     return isCi
   },
 
-  getEnvOverrides () {
+  getEnvOverrides (options = {}) {
     return _
     .chain({})
     .extend(util.getEnvColors())
@@ -244,9 +244,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 when --dev is passed, so this won't work if Node is too old
+      logger.warn('(dev-mode warning only) NODE_OPTIONS=--max-http-header-size could not be set. See https://github.com/cypress-io/cypress/pull/5452')
+
+      return
+    }
+
+    // https://github.com/cypress-io/cypress/issues/5431
+    const NODE_OPTIONS = `--max-http-header-size=${1024 * 1024}`
+
+    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),

diff --git a/cli/test/lib/exec/spawn_spec.js b/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)
       })
     })
 

diff --git a/cli/test/lib/util_spec.js b/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}`
+
 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
     })
   })
 

diff --git a/packages/electron/lib/electron.coffee b/packages/electron/lib/electron.coffee
@@ -75,10 +75,6 @@ module.exports = {
         if opts.inspectBrk
           argv.unshift("--inspect-brk=5566")
 
-      ## max HTTP header size 8kb -> 1mb
-      ## https://github.com/cypress-io/cypress/issues/76
-      argv.unshift("--max-http-header-size=#{1024*1024}")
-
       debug("spawning %s with args", execPath, argv)
 
       if debug.enabled

diff --git a/packages/server/index.js b/packages/server/index.js
@@ -1,3 +1,11 @@
+// if running in production mode (CYPRESS_ENV)
+// all transpile should have been done already
+// and these calls should do nothing
+require('@packages/ts/register')
+require('@packages/coffee/register')
+
+require('./lib/util/reset_node_options').reset()
+
 // override tty if we're being forced to
 require('./lib/util/tty').override()
 
@@ -9,11 +17,6 @@ if (process.env.CY_NET_PROFILE && process.env.CYPRESS_ENV) {
 
 process.env.UV_THREADPOOL_SIZE = 128
 require('graceful-fs').gracefulify(require('fs'))
-// if running in production mode (CYPRESS_ENV)
-// all transpile should have been done already
-// and these calls should do nothing
-require('@packages/ts/register')
-require('@packages/coffee/register')
 
 require && require.extensions && delete require.extensions['.litcoffee']
 require && require.extensions && delete require.extensions['.coffee.md']

diff --git a/packages/server/lib/util/reset_node_options.ts b/packages/server/lib/util/reset_node_options.ts
@@ -0,0 +1,17 @@
+/**
+ * Once the Electron process is launched, restore the user's original NODE_OPTIONS
+ * environment variables from before the CLI added extra NODE_OPTIONS.
+ *
+ * This way, any `node` processes launched by Cypress will retain the user's
+ * `NODE_OPTIONS` without unexpected modificiations that could cause issues with
+ * user code.
+ */
+
+export function reset () {
+  // @ts-ignore
+  if (process.versions.electron && typeof process.env.ORIGINAL_NODE_OPTIONS === 'string') {
+    process.env.NODE_OPTIONS = process.env.ORIGINAL_NODE_OPTIONS
+
+    delete process.env.ORIGINAL_NODE_OPTIONS
+  }
+}
diff --git a/packages/server/test/e2e/4_xhr_spec.coffee b/packages/server/test/e2e/4_xhr_spec.coffee
@@ -30,4 +30,5 @@ describe "e2e xhr", ->
     spec: "xhr_spec.coffee"
     snapshot: true
     expectedExitCode: 0
+    useCli: true
   }