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

fix: Handle privileged commands arg mutation #27282

Merged
merged 4 commits into from
Jul 17, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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).

## 12.17.1
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 @@ -132,6 +132,17 @@ describe('privileged commands', () => {
`)
})

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('passes in test body .then() callback', () => {
cy.then(() => {
cy.exec('echo "hello"')
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