Skip to content

Commit

Permalink
fix: Handle privileged commands arg mutation (#27282)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisbreiding authored Jul 17, 2023
1 parent 8fc59f4 commit ddec92d
Show file tree
Hide file tree
Showing 16 changed files with 49 additions and 114 deletions.
1 change: 1 addition & 0 deletions cli/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ _Released 07/18/2023 (PENDING)_

**Bugfixes:**

- Fixed an issue where commands would fail with the error `must only be invoked from the spec file or support file` if their arguments were mutated. Fixes [#27200](https://github.com/cypress-io/cypress/issues/27200).
- Fixed an issue where `cy.writeFile()` would erroneously fail with the error `cy.writeFile() must only be invoked from the spec file or support file`. Fixes [#27097](https://github.com/cypress-io/cypress/issues/27097).
- Fixed an issue where web workers could not be created within a spec. Fixes [#27298](https://github.com/cypress-io/cypress/issues/27298).

Expand Down
4 changes: 2 additions & 2 deletions packages/driver/cypress/e2e/commands/exec.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ describe('src/cy/commands/exec', () => {

cy.exec('ls').then(() => {
expect(Cypress.backend).to.be.calledWith('run:privileged', {
args: ['8374177128052794'],
commandName: 'exec',
userArgs: ['8374177128052794'],
options: {
cmd: 'ls',
timeout: 2500,
Expand All @@ -33,8 +33,8 @@ describe('src/cy/commands/exec', () => {

cy.exec('ls', { env: { FOO: 'foo' } }).then(() => {
expect(Cypress.backend).to.be.calledWith('run:privileged', {
args: ['8374177128052794', '6419589148408857'],
commandName: 'exec',
userArgs: ['8374177128052794', '6419589148408857'],
options: {
cmd: 'ls',
timeout: 2500,
Expand Down
16 changes: 8 additions & 8 deletions packages/driver/cypress/e2e/commands/files.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['6998637248317671'],
commandName: 'readFile',
userArgs: ['6998637248317671'],
options: {
file: 'foo.json',
encoding: 'utf8',
Expand All @@ -39,8 +39,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['6998637248317671', '2573904513237804'],
commandName: 'readFile',
userArgs: ['6998637248317671', '2573904513237804'],
options: {
file: 'foo.json',
encoding: 'ascii',
Expand All @@ -61,8 +61,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['6998637248317671', '6158203196586298'],
commandName: 'readFile',
userArgs: ['6998637248317671', '6158203196586298'],
options: {
file: 'foo.json',
encoding: null,
Expand Down Expand Up @@ -451,8 +451,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['2916834115813688', '4891975990226114'],
commandName: 'writeFile',
userArgs: ['2916834115813688', '4891975990226114'],
options: {
fileName: 'foo.txt',
contents: 'contents',
Expand All @@ -471,8 +471,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['2916834115813688', '4891975990226114', '2573904513237804'],
commandName: 'writeFile',
userArgs: ['2916834115813688', '4891975990226114', '2573904513237804'],
options: {
fileName: 'foo.txt',
contents: 'contents',
Expand All @@ -494,8 +494,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['2916834115813688', '6309890104324788', '6158203196586298'],
commandName: 'writeFile',
userArgs: ['2916834115813688', '6309890104324788', '6158203196586298'],
options: {
fileName: 'foo.txt',
contents: buffer,
Expand All @@ -514,8 +514,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['2916834115813688', '4891975990226114', '4694939291947123'],
commandName: 'writeFile',
userArgs: ['2916834115813688', '4891975990226114', '4694939291947123'],
options: {
fileName: 'foo.txt',
contents: 'contents',
Expand Down Expand Up @@ -569,8 +569,8 @@ describe('src/cy/commands/files', () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['2916834115813688', '4891975990226114', '2343101193011749'],
commandName: 'writeFile',
userArgs: ['2916834115813688', '4891975990226114', '2343101193011749'],
options: {
fileName: 'foo.txt',
contents: 'contents',
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/cypress/e2e/commands/task.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ describe('src/cy/commands/task', () => {

cy.task('foo').then(() => {
expect(Cypress.backend).to.be.calledWith('run:privileged', {
args: ['338657716278786'],
commandName: 'task',
userArgs: ['338657716278786'],
options: {
task: 'foo',
timeout: 2500,
Expand All @@ -30,8 +30,8 @@ describe('src/cy/commands/task', () => {

cy.task('foo', { foo: 'foo' }).then(() => {
expect(Cypress.backend).to.be.calledWith('run:privileged', {
args: ['338657716278786', '4940328425038888'],
commandName: 'task',
userArgs: ['338657716278786', '4940328425038888'],
options: {
task: 'foo',
timeout: 2500,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ context('cy.origin files', { browser: '!webkit' }, () => {
expect(Cypress.backend).to.be.calledWith(
'run:privileged',
{
args: ['6998637248317671', '4581875909943693'],
commandName: 'writeFile',
userArgs: ['6998637248317671', '4581875909943693'],
options: {
fileName: 'foo.json',
contents,
Expand Down
11 changes: 11 additions & 0 deletions packages/driver/cypress/e2e/e2e/privileged_commands.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,17 @@ describe('privileged commands', () => {
cy.get('#basic').selectFile(Uint8Array.from([98, 97, 122]))
})

it('handles args being mutated', () => {
const obj = { foo: 'bar' }

cy.wait(10).then(() => {
obj.foo = 'baz'
})

cy.task('return:arg', obj)
cy.writeFile('cypress/_test-output/written.json', obj)
})

it('handles evaled code', () => {
window.eval(`
cy.task('return:arg', 'eval arg')
Expand Down
14 changes: 4 additions & 10 deletions packages/driver/src/cy/commands/actions/selectFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export default (Commands, Cypress, cy, state, config) => {
}
}

const readFiles = async (filePaths, options, userArgs) => {
const readFiles = async (filePaths, options) => {
if (!filePaths.length) return []

// This reads the file with privileged access in the same manner as
Expand All @@ -177,7 +177,6 @@ export default (Commands, Cypress, cy, state, config) => {
options: {
files: filePaths,
},
userArgs,
})
.then((results) => {
return results.map((result) => {
Expand Down Expand Up @@ -268,11 +267,11 @@ export default (Commands, Cypress, cy, state, config) => {
}
}

async function collectFiles (files, options, userArgs) {
async function collectFiles (files, options) {
const filesCollection = ([] as (Cypress.FileReference | FilePathObject)[]).concat(files).map(parseFile(options))
// if there are any file paths, read them from the server in one go
const filePaths = filesCollection.filter((file) => (file as FilePathObject).isFilePath)
const filePathResults = await readFiles(filePaths, options, userArgs)
const filePathResults = await readFiles(filePaths, options)

// stitch them back into the collection
filePathResults.forEach((filePathResult) => {
Expand All @@ -284,11 +283,6 @@ export default (Commands, Cypress, cy, state, config) => {

Commands.addAll({ prevSubject: 'element' }, {
async selectFile (subject: JQuery<any>, files: Cypress.FileReference | Cypress.FileReference[], options: Partial<InternalSelectFileOptions>, ...extras: never[]): Promise<JQuery> {
// privileged commands need to send any and all args, even if not part
// of their API, so they can be compared to the args collected when the
// command is invoked
const userArgs = [files, _.isObject(options) ? { ...options } : undefined, ...extras]

options = _.defaults({}, options, {
action: 'select',
log: true,
Expand Down Expand Up @@ -351,7 +345,7 @@ export default (Commands, Cypress, cy, state, config) => {
}

// Make sure files is an array even if the user only passed in one
const filesArray = await collectFiles(files, options, userArgs)
const filesArray = await collectFiles(files, options)
const subjectChain = cy.subjectChain()

// We verify actionability on the subject, rather than the eventTarget,
Expand Down
6 changes: 0 additions & 6 deletions packages/driver/src/cy/commands/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,6 @@ interface InternalExecOptions extends Partial<Cypress.ExecOptions> {
export default (Commands, Cypress, cy) => {
Commands.addAll({
exec (cmd: string, userOptions: Partial<Cypress.ExecOptions>, ...extras: never[]) {
// privileged commands need to send any and all args, even if not part
// of their API, so they can be compared to the args collected when the
// command is invoked
const userArgs = [cmd, userOptions, ...extras]

userOptions = userOptions || {}

const options: InternalExecOptions = _.defaults({}, userOptions, {
Expand Down Expand Up @@ -60,7 +55,6 @@ export default (Commands, Cypress, cy) => {
cy,
Cypress: (Cypress as unknown) as InternalCypress.Cypress,
options: _.pick(options, 'cmd', 'timeout', 'env'),
userArgs,
})
.timeout(options.timeout)
.then((result) => {
Expand Down
12 changes: 0 additions & 12 deletions packages/driver/src/cy/commands/files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ type WriteFileOptions = Partial<Cypress.WriteFileOptions & Cypress.Timeoutable>
export default (Commands, Cypress, cy, state) => {
Commands.addAll({
readFile (file: string, encoding: Cypress.Encodings | ReadFileOptions | undefined, userOptions?: ReadFileOptions, ...extras: never[]) {
// privileged commands need to send any and all args, even if not part
// of their API, so they can be compared to the args collected when the
// command is invoked
const userArgs = [file, encoding, _.isObject(userOptions) ? { ...userOptions } : undefined, ...extras]

if (_.isObject(encoding)) {
userOptions = encoding
encoding = undefined
Expand Down Expand Up @@ -77,7 +72,6 @@ export default (Commands, Cypress, cy, state) => {
file,
encoding: options.encoding,
},
userArgs,
})
.timeout(options.timeout)
.catch((err) => {
Expand Down Expand Up @@ -146,11 +140,6 @@ export default (Commands, Cypress, cy, state) => {
},

writeFile (fileName: string, contents: string, encoding: Cypress.Encodings | WriteFileOptions | undefined, userOptions: WriteFileOptions, ...extras: never[]) {
// privileged commands need to send any and all args, even if not part
// of their API, so they can be compared to the args collected when the
// command is invoked
const userArgs = [fileName, contents, encoding, _.isObject(userOptions) ? { ...userOptions } : undefined, ...extras]

if (_.isObject(encoding)) {
userOptions = encoding
encoding = undefined
Expand Down Expand Up @@ -214,7 +203,6 @@ export default (Commands, Cypress, cy, state) => {
encoding: options.encoding,
flag: options.flag,
},
userArgs,
})
.timeout(options.timeout)
.then(({ filePath, contents }) => {
Expand Down
15 changes: 0 additions & 15 deletions packages/driver/src/cy/commands/origin/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,6 @@ const normalizeOrigin = (urlOrDomain) => {
type OptionsOrFn<T> = { args: T } | (() => {})
type Fn<T> = (args?: T) => {}

function getUserArgs<T> (urlOrDomain: string, optionsOrFn: OptionsOrFn<T>, extras: never[], fn?: Fn<T>) {
return [
urlOrDomain,
fn && _.isObject(optionsOrFn) ? { ...optionsOrFn } : optionsOrFn,
fn ? fn : undefined,
...extras,
]
}

export default (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy, state: StateFunc, config: Cypress.InternalConfig) => {
const communicator = Cypress.primaryOriginCommunicator

Expand All @@ -45,11 +36,6 @@ export default (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy, state: State
return $errUtils.throwErrByPath('webkit.origin')
}

// privileged commands need to send any and all args, even if not part
// of their API, so they can be compared to the args collected when the
// command is invoked
const userArgs = getUserArgs<T>(urlOrDomain, optionsOrFn, extras, fn)

const userInvocationStack = state('current').get('userInvocationStack')

// store the invocation stack in the case that `cy.origin` errors
Expand Down Expand Up @@ -213,7 +199,6 @@ export default (Commands, Cypress: Cypress.Cypress, cy: Cypress.cy, state: State
options: {
specBridgeOrigin,
},
userArgs,
})

// once the secondary origin page loads, send along the
Expand Down
6 changes: 0 additions & 6 deletions packages/driver/src/cy/commands/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,6 @@ interface InternalTaskOptions extends Partial<Cypress.Loggable & Cypress.Timeout
export default (Commands, Cypress, cy) => {
Commands.addAll({
task (task, arg, userOptions: Partial<Cypress.Loggable & Cypress.Timeoutable>, ...extras: never[]) {
// privileged commands need to send any and all args, even if not part
// of their API, so they can be compared to the args collected when the
// command is invoked
const userArgs = [task, arg, _.isObject(userOptions) ? { ...userOptions } : undefined, ...extras]

userOptions = userOptions || {}

const options: InternalTaskOptions = _.defaults({}, userOptions, {
Expand Down Expand Up @@ -65,7 +60,6 @@ export default (Commands, Cypress, cy) => {
commandName: 'task',
cy,
Cypress: (Cypress as unknown) as InternalCypress.Cypress,
userArgs,
options: {
task,
arg,
Expand Down
4 changes: 2 additions & 2 deletions packages/driver/src/cypress/chainer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,15 @@ export class $Chainer {

static add (key, fn) {
$Chainer.prototype[key] = function (...args) {
const verificationPromise = Cypress.emitMap('command:invocation', { name: key, args })
const privilegeVerification = Cypress.emitMap('command:invocation', { name: key, args })

const userInvocationStack = $stackUtils.normalizedUserInvocationStack(
(new this.specWindow.Error('command invocation stack')).stack,
)

// call back the original function with our new args
// pass args an as array and not a destructured invocation
fn(this, userInvocationStack, args, verificationPromise)
fn(this, userInvocationStack, args, privilegeVerification)

// return the chainer so additional calls
// are slurped up by the chainer instead of cy
Expand Down
8 changes: 4 additions & 4 deletions packages/driver/src/cypress/cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -683,7 +683,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
const cyFn = wrap(true)
const chainerFn = wrap(false)

const callback = (chainer, userInvocationStack, args, verificationPromise, firstCall = false) => {
const callback = (chainer, userInvocationStack, args, privilegeVerification, firstCall = false) => {
// dont enqueue / inject any new commands if
// onInjectCommand returns false
const onInjectCommand = cy.state('onInjectCommand')
Expand All @@ -699,7 +699,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
chainerId: chainer.chainerId,
userInvocationStack,
fn: firstCall ? cyFn : chainerFn,
verificationPromise,
privilegeVerification,
}))
}

Expand All @@ -715,7 +715,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert
// websocket message for running the command to ensure prevent a race
// condition where running the command happens before the command is
// verified
const verificationPromise = Cypress.emitMap('command:invocation', { name, args })
const privilegeVerification = Cypress.emitMap('command:invocation', { name, args })

// this is the first call on cypress
// so create a new chainer instance
Expand All @@ -727,7 +727,7 @@ export class $Cy extends EventEmitter2 implements ITimeouts, IStability, IAssert

const userInvocationStack = $stackUtils.captureUserInvocationStack(cy.specWindow.Error)

callback(chainer, userInvocationStack, args, verificationPromise, true)
callback(chainer, userInvocationStack, args, privilegeVerification, true)

// if we are in the middle of a command
// and its return value is a promise
Expand Down
Loading

4 comments on commit ddec92d

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ddec92d Jul 17, 2023

Choose a reason for hiding this comment

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

Circle has built the linux x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.17.2/linux-x64/develop-ddec92d8da9fac92a7556867cb0258a4dbe8f1ec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ddec92d Jul 17, 2023

Choose a reason for hiding this comment

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

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.17.2/darwin-x64/develop-ddec92d8da9fac92a7556867cb0258a4dbe8f1ec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ddec92d Jul 17, 2023

Choose a reason for hiding this comment

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

Circle has built the darwin x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.17.2/darwin-x64/develop-ddec92d8da9fac92a7556867cb0258a4dbe8f1ec/cypress.tgz

@cypress-bot
Copy link
Contributor

@cypress-bot cypress-bot bot commented on ddec92d Jul 17, 2023

Choose a reason for hiding this comment

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

Circle has built the win32 x64 version of the Test Runner.

Learn more about this pre-release build at https://on.cypress.io/advanced-installation#Install-pre-release-version

Run this command to install the pre-release locally:

npm install https://cdn.cypress.io/beta/npm/12.17.2/win32-x64/develop-ddec92d8da9fac92a7556867cb0258a4dbe8f1ec/cypress.tgz

Please sign in to comment.