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

RegExp for modifyObstructiveCode replaces var names started with top with self when it shouldn't #4794

Closed
indooorsman opened this issue Jul 24, 2019 · 6 comments · Fixed by #4831
Assignees
Labels
pkg/server This is due to an issue in the packages/server directory topic: framebusting type: bug

Comments

@indooorsman
Copy link

const topOrParentEqualityBeforeRe = /((?:window|self)(?:\.|\[['"](?:top|self)['"]\])?\s*[!=]==?\s*(?:(?:window|self)(?:\.|\[['"]))?)(top|parent)/g

One of my file contains the below code:

if (topTarget.window === topTarget) { ... }

in Cypress, It's replaced with

if (topTarget.window === selfTarget) { ... }

and it caused error.

Please fix it as soon as possible, many thanks <3

@indooorsman indooorsman changed the title The Proxy in Server package has a critical bug when replace top to self The Proxy in Server package has a critical bug when replace top with self Jul 24, 2019
@jennifer-shehane
Copy link
Member

Hey @indooorsman, this is intended behavior in Cypress in order to have sites tested in Cypress work correctly - as they are run from within an iframe - and some sites have framebusting code.

You can turn off this behavior by setting modifyObstructiveCode: false in your configuration.

I'd like to more fully understand the error you are getting and the unintended effect that Cypress seems to have caused in your application though. Can you post the error and explain the unexpected behavior more fully? Thanks.

@jennifer-shehane jennifer-shehane added the stage: needs information Not enough info to reproduce the issue label Jul 24, 2019
@indooorsman
Copy link
Author

indooorsman commented Jul 25, 2019

@jennifer-shehane Thanks for your reply, the error is selfTarget is not defined.
The original code:

funcA(topTarget) {
  if (topTarget.window === topTarget) { ... }
}

In cypress it's replaced with:

funcA(topTarget) {
  if (topTarget.window === selfTarget) { ... }
}

Of course, selfTarget is not defined.

And I think the regexp topOrParentEqualityBeforeRe has bugs.

About setting modifyObstructiveCode in configuration, it's better to have a default value of false in my opinion.

@jennifer-shehane
Copy link
Member

@indooorsman Thanks. Yeah, indeed - I agree this RegExp is too lenient in this situation and is definitely a problem. Thanks for clearing this up - we need to fix this.

@cypress-bot cypress-bot bot added stage: ready for work The issue is reproducible and in scope and removed stage: needs information Not enough info to reproduce the issue labels Jul 26, 2019
@cypress-bot cypress-bot bot added stage: work in progress and removed stage: ready for work The issue is reproducible and in scope labels Jul 26, 2019
@jennifer-shehane jennifer-shehane changed the title The Proxy in Server package has a critical bug when replace top with self RegExp for modifyObstructiveCode replaces var names started with top with self when it shouldn't Jul 26, 2019
@jennifer-shehane jennifer-shehane self-assigned this Jul 26, 2019
@jennifer-shehane jennifer-shehane added the pkg/server This is due to an issue in the packages/server directory label Jul 26, 2019
@cypress-bot cypress-bot bot added stage: needs review The PR code is done & tested, needs review stage: work in progress and removed stage: work in progress stage: needs review The PR code is done & tested, needs review labels Jul 26, 2019
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Jul 26, 2019

The code for this is done in cypress-io/cypress#4831, but has yet to be released.
We'll update this issue and reference the changelog when it's released.

@indooorsman
Copy link
Author

@jennifer-shehane Great, thanks for your efficient actions 💯

@jennifer-shehane
Copy link
Member

Released in 3.4.1

@cypress-io cypress-io locked as resolved and limited conversation to collaborators Mar 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg/server This is due to an issue in the packages/server directory topic: framebusting type: bug
Projects
None yet
2 participants