Skip to content

Commit 978d97e

Browse files
flotwigjennifer-shehane
flotwig
authored and
jennifer-shehane
committed
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
1 parent 3a747ab commit 978d97e

File tree

12 files changed

+182
-52
lines changed

12 files changed

+182
-52
lines changed

‎cli/__snapshots__/spawn_spec.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
exports['lib/exec/spawn .start forces colors and streams when supported 1'] = {
2+
"FORCE_COLOR": "1",
3+
"DEBUG_COLORS": "1",
4+
"MOCHA_COLORS": "1",
5+
"FORCE_STDIN_TTY": "1",
6+
"FORCE_STDOUT_TTY": "1",
7+
"FORCE_STDERR_TTY": "1",
8+
"NODE_OPTIONS": "--max-http-header-size=1048576"
9+
}
10+
11+
exports['lib/exec/spawn .start does not force colors and streams when not supported 1'] = {
12+
"FORCE_COLOR": "0",
13+
"DEBUG_COLORS": "0",
14+
"FORCE_STDIN_TTY": "0",
15+
"FORCE_STDOUT_TTY": "0",
16+
"FORCE_STDERR_TTY": "0",
17+
"NODE_OPTIONS": "--max-http-header-size=1048576"
18+
}

‎cli/lib/exec/spawn.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ module.exports = {
102102
}
103103

104104
const { onStderrData, electronLogging } = overrides
105-
const envOverrides = util.getEnvOverrides()
105+
const envOverrides = util.getEnvOverrides(options)
106106
const electronArgs = _.clone(args)
107107
const node11WindowsFix = isPlatform('win32')
108108

‎cli/lib/util.js‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ const util = {
184184
return isCi
185185
},
186186

187-
getEnvOverrides () {
187+
getEnvOverrides (options = {}) {
188188
return _
189189
.chain({})
190190
.extend(util.getEnvColors())
@@ -193,9 +193,35 @@ const util = {
193193
.mapValues((value) => { // stringify to 1 or 0
194194
return value ? '1' : '0'
195195
})
196+
.extend(util.getNodeOptions(options))
196197
.value()
197198
},
198199

200+
getNodeOptions (options, nodeVersion) {
201+
if (!nodeVersion) {
202+
nodeVersion = Number(process.versions.node.split('.')[0])
203+
}
204+
205+
if (options.dev && nodeVersion < 12) {
206+
// `node` is used when --dev is passed, so this won't work if Node is too old
207+
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')
208+
209+
return
210+
}
211+
212+
// https://github.com/cypress-io/cypress/issues/5431
213+
const NODE_OPTIONS = `--max-http-header-size=${1024 * 1024}`
214+
215+
if (_.isString(process.env.NODE_OPTIONS)) {
216+
return {
217+
NODE_OPTIONS: `${NODE_OPTIONS} ${process.env.NODE_OPTIONS}`,
218+
ORIGINAL_NODE_OPTIONS: process.env.NODE_OPTIONS || '',
219+
}
220+
}
221+
222+
return { NODE_OPTIONS }
223+
},
224+
199225
getForceTty () {
200226
return {
201227
FORCE_STDIN_TTY: util.isTty(process.stdin.fd),

‎cli/test/lib/exec/spawn_spec.js‎

Lines changed: 3 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ require('../../spec_helper')
33
const _ = require('lodash')
44
const cp = require('child_process')
55
const os = require('os')
6+
const snapshot = require('snap-shot-it')
67
const tty = require('tty')
78
const path = require('path')
89
const EE = require('events')
@@ -287,14 +288,7 @@ describe('lib/exec/spawn', function () {
287288

288289
return spawn.start([], { env: {} })
289290
.then(() => {
290-
expect(cp.spawn.firstCall.args[2].env).to.deep.eq({
291-
FORCE_COLOR: '1',
292-
DEBUG_COLORS: '1',
293-
MOCHA_COLORS: '1',
294-
FORCE_STDERR_TTY: '1',
295-
FORCE_STDIN_TTY: '1',
296-
FORCE_STDOUT_TTY: '1',
297-
})
291+
snapshot(cp.spawn.firstCall.args[2].env)
298292
})
299293
})
300294

@@ -326,13 +320,7 @@ describe('lib/exec/spawn', function () {
326320

327321
return spawn.start([], { env: {} })
328322
.then(() => {
329-
expect(cp.spawn.firstCall.args[2].env).to.deep.eq({
330-
FORCE_COLOR: '0',
331-
DEBUG_COLORS: '0',
332-
FORCE_STDERR_TTY: '0',
333-
FORCE_STDIN_TTY: '0',
334-
FORCE_STDOUT_TTY: '0',
335-
})
323+
snapshot(cp.spawn.firstCall.args[2].env)
336324
})
337325
})
338326

‎cli/test/lib/util_spec.js‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ require('../spec_helper')
33
const os = require('os')
44
const tty = require('tty')
55
const snapshot = require('../support/snapshot')
6+
const mockedEnv = require('mocked-env')
67
const supportsColor = require('supports-color')
78
const proxyquire = require('proxyquire')
89
const hasha = require('hasha')
@@ -11,6 +12,9 @@ const la = require('lazy-ass')
1112
const util = require(`${lib}/util`)
1213
const logger = require(`${lib}/logger`)
1314

15+
// https://github.com/cypress-io/cypress/issues/5431
16+
const expectedNodeOptions = `--max-http-header-size=${1024 * 1024}`
17+
1418
describe('util', () => {
1519
beforeEach(() => {
1620
sinon.stub(process, 'exit')
@@ -213,6 +217,7 @@ describe('util', () => {
213217
FORCE_COLOR: '1',
214218
DEBUG_COLORS: '1',
215219
MOCHA_COLORS: '1',
220+
NODE_OPTIONS: expectedNodeOptions,
216221
})
217222

218223
util.supportsColor.returns(false)
@@ -224,7 +229,46 @@ describe('util', () => {
224229
FORCE_STDERR_TTY: '0',
225230
FORCE_COLOR: '0',
226231
DEBUG_COLORS: '0',
232+
NODE_OPTIONS: expectedNodeOptions,
233+
})
234+
})
235+
})
236+
237+
context('.getNodeOptions', () => {
238+
let restoreEnv
239+
240+
afterEach(() => {
241+
if (restoreEnv) {
242+
restoreEnv()
243+
restoreEnv = null
244+
}
245+
})
246+
247+
it('adds required NODE_OPTIONS', () => {
248+
restoreEnv = mockedEnv({
249+
NODE_OPTIONS: undefined,
227250
})
251+
252+
expect(util.getNodeOptions({})).to.deep.eq({
253+
NODE_OPTIONS: expectedNodeOptions,
254+
})
255+
})
256+
257+
it('includes existing NODE_OPTIONS', () => {
258+
restoreEnv = mockedEnv({
259+
NODE_OPTIONS: '--foo --bar',
260+
})
261+
262+
expect(util.getNodeOptions({})).to.deep.eq({
263+
NODE_OPTIONS: `${expectedNodeOptions} --foo --bar`,
264+
ORIGINAL_NODE_OPTIONS: '--foo --bar',
265+
})
266+
})
267+
268+
it('does not return if dev is set and version < 12', () => {
269+
expect(util.getNodeOptions({
270+
dev: true,
271+
}, 11)).to.be.undefined
228272
})
229273
})
230274

‎packages/electron/lib/electron.coffee‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -75,10 +75,6 @@ module.exports = {
7575
if opts.inspectBrk
7676
argv.unshift("--inspect-brk=5566")
7777

78-
## max HTTP header size 8kb -> 1mb
79-
## https://github.com/cypress-io/cypress/issues/76
80-
argv.unshift("--max-http-header-size=#{1024*1024}")
81-
8278
debug("spawning %s with args", execPath, argv)
8379

8480
if debug.enabled

‎packages/server/__snapshots__/4_xhr_spec.coffee.js‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,20 +23,21 @@ exports['e2e xhr / passes'] = `
2323
✓ does not inject into json's contents from http server even requesting text/html
2424
✓ does not inject into json's contents from file server even requesting text/html
2525
✓ works prior to visit
26+
✓ can stub a 100kb response
2627
server with 1 visit
2728
✓ response body
2829
✓ request body
2930
✓ aborts
3031
3132
32-
8 passing
33+
9 passing
3334
3435
3536
(Results)
3637
3738
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
38-
│ Tests: 8 │
39-
│ Passing: 8 │
39+
│ Tests: 9 │
40+
│ Passing: 9 │
4041
│ Failing: 0 │
4142
│ Pending: 0 │
4243
│ Skipped: 0 │
@@ -60,9 +61,9 @@ exports['e2e xhr / passes'] = `
6061
6162
Spec Tests Passing Failing Pending Skipped
6263
┌────────────────────────────────────────────────────────────────────────────────────────────────┐
63-
│ ✔ xhr_spec.coffee XX:XX 8 8 - - - │
64+
│ ✔ xhr_spec.coffee XX:XX 9 9 - - - │
6465
└────────────────────────────────────────────────────────────────────────────────────────────────┘
65-
✔ All specs passed! XX:XX 8 8 - - -
66+
✔ All specs passed! XX:XX 9 9 - - -
6667
6768
6869
`

‎packages/server/index.js‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,11 @@
1+
// if running in production mode (CYPRESS_ENV)
2+
// all transpile should have been done already
3+
// and these calls should do nothing
4+
require('@packages/ts/register')
5+
require('@packages/coffee/register')
6+
7+
require('./lib/util/reset_node_options').reset()
8+
19
// override tty if we're being forced to
210
require('./lib/util/tty').override()
311

@@ -9,11 +17,6 @@ if (process.env.CY_NET_PROFILE && process.env.CYPRESS_ENV) {
917

1018
process.env.UV_THREADPOOL_SIZE = 128
1119
require('graceful-fs').gracefulify(require('fs'))
12-
// if running in production mode (CYPRESS_ENV)
13-
// all transpile should have been done already
14-
// and these calls should do nothing
15-
require('@packages/ts/register')
16-
require('@packages/coffee/register')
1720

1821
require && require.extensions && delete require.extensions['.litcoffee']
1922
require && require.extensions && delete require.extensions['.coffee.md']

‎packages/server/lib/util/reset_node_options.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
/**
2+
* Once the Electron process is launched, restore the user's original NODE_OPTIONS
3+
* environment variables from before the CLI added extra NODE_OPTIONS.
4+
*
5+
* This way, any `node` processes launched by Cypress will retain the user's
6+
* `NODE_OPTIONS` without unexpected modificiations that could cause issues with
7+
* user code.
8+
*/
9+
10+
export function reset () {
11+
// @ts-ignore
12+
if (process.versions.electron && typeof process.env.ORIGINAL_NODE_OPTIONS === 'string') {
13+
process.env.NODE_OPTIONS = process.env.ORIGINAL_NODE_OPTIONS
14+
15+
delete process.env.ORIGINAL_NODE_OPTIONS
16+
}
17+
}

‎packages/server/test/e2e/4_xhr_spec.coffee‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,5 @@ describe "e2e xhr", ->
3030
spec: "xhr_spec.coffee"
3131
snapshot: true
3232
expectedExitCode: 0
33+
useCli: true
3334
}

0 commit comments

Comments
 (0)