Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always pass NODE_OPTIONS with max-http-header-size #5452

Merged
merged 11 commits into from
Oct 25, 2019
18 changes: 18 additions & 0 deletions cli/__snapshots__/spawn_spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
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"
}

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"
}
2 changes: 1 addition & 1 deletion cli/lib/exec/spawn.js
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ module.exports = {
}

const { onStderrData, electronLogging } = overrides
const envOverrides = util.getEnvOverrides()
const envOverrides = util.getEnvOverrides(options)
const electronArgs = _.clone(args)
const node11WindowsFix = isPlatform('win32')

Expand Down
28 changes: 27 additions & 1 deletion cli/lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const util = {
return isCi
},

getEnvOverrides () {
getEnvOverrides (options = {}) {
return _
.chain({})
.extend(util.getEnvColors())
Expand All @@ -193,9 +193,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')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will the new max-header log in DEBUG logs for all of our users? Cause I'd like any new options to be logged in their DEBUG logs for future debugging.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not logged anywhere :/ But, it should always be passed, as this is the only condition where it wouldn't be.

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),
Expand Down
18 changes: 3 additions & 15 deletions cli/test/lib/exec/spawn_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ require('../../spec_helper')
const _ = require('lodash')
const cp = require('child_process')
const os = require('os')
const snapshot = require('snap-shot-it')
const tty = require('tty')
const path = require('path')
const EE = require('events')
Expand Down Expand Up @@ -287,14 +288,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)
})
})

Expand Down Expand Up @@ -326,13 +320,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)
})
})

Expand Down
44 changes: 44 additions & 0 deletions cli/test/lib/util_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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')
Expand Down Expand Up @@ -213,6 +217,7 @@ describe('util', () => {
FORCE_COLOR: '1',
DEBUG_COLORS: '1',
MOCHA_COLORS: '1',
NODE_OPTIONS: expectedNodeOptions,
})

util.supportsColor.returns(false)
Expand All @@ -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
})
})

Expand Down
4 changes: 0 additions & 4 deletions packages/electron/lib/electron.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 6 additions & 5 deletions packages/server/__snapshots__/4_xhr_spec.coffee.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,20 +23,21 @@ exports['e2e xhr / passes'] = `
✓ does not inject into json's contents from http server even requesting text/html
✓ does not inject into json's contents from file server even requesting text/html
✓ works prior to visit
✓ can stub a 100kb response
server with 1 visit
✓ response body
✓ request body
✓ aborts


8 passing
9 passing


(Results)

┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ Tests: 8
│ Passing: 8
│ Tests: 9
│ Passing: 9
│ Failing: 0 │
│ Pending: 0 │
│ Skipped: 0 │
Expand All @@ -60,9 +61,9 @@ exports['e2e xhr / passes'] = `

Spec Tests Passing Failing Pending Skipped
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
│ ✔ xhr_spec.coffee XX:XX 8 8 - - - │
│ ✔ xhr_spec.coffee XX:XX 9 9 - - - │
└────────────────────────────────────────────────────────────────────────────────────────────────┘
✔ All specs passed! XX:XX 8 8 - - -
✔ All specs passed! XX:XX 9 9 - - -


`
13 changes: 8 additions & 5 deletions packages/server/index.js
Original file line number Diff line number Diff line change
@@ -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()

Expand All @@ -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']
Expand Down
17 changes: 17 additions & 0 deletions packages/server/lib/util/reset_node_options.ts
Original file line number Diff line number Diff line change
@@ -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
}
}
1 change: 1 addition & 0 deletions packages/server/test/e2e/4_xhr_spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -30,4 +30,5 @@ describe "e2e xhr", ->
spec: "xhr_spec.coffee"
snapshot: true
expectedExitCode: 0
useCli: true
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,33 @@ describe "xhrs", ->
it "works prior to visit", ->
cy.server()

## https://github.com/cypress-io/cypress/issues/5431
it "can stub a 100kb response", (done) ->
body = 'X'.repeat(100 * 1024)

cy.server()
cy.route({
method: 'POST'
url: '/foo'
response: {
'bar': body
}
})

cy.visit("/index.html")
.then (win) ->
xhr = new win.XMLHttpRequest
xhr.open("POST", "/foo")
xhr.send()

finish = ->
expect(xhr.status).to.eq(200)
expect(xhr.responseText).to.include(body)
done()

xhr.onload = finish
xhr.onerror = finish

describe "server with 1 visit", ->
before ->
cy.visit("/xhr.html")
Expand Down