Skip to content

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Oct 2, 2019

User facing changelog

Experimental fixes

To access these fixes, enable experimental source rewriting with the config value experimentalSourceRewriting: true:

Additional details

  • Adds AST-based rewriting behind experimental flag experimentalSourceRewriting:
    image
  • Explore serving source maps so users can still debug their original source code: https://github.com/benjamn/recast#source-maps
    • may possibly conflict with Error Improvements #6724 go-to-line functionality error improvements only consume sourcemaps for specfiles, not AUT JS (yet) - so we can get away with simply inlining sourcemaps for now
    • sourcemaps will be served over HTTP, we don't want the overhead of generating them for every single JS loaded - unfortunately this requires some caching of JS
    • also intercept SourceMap and X-SourceMap headers
      • order of preference: inlined sourcemappingurl, sourcemap header, x-sourcemap header
  • Strip script integrity parameters - fixes Cypress cannot test sites that implement SRI #2393
    • SRI integrity attributes in <script type="text/javascript"> tags will be rewritten to cypress:stripped-integrity attributes - <script type="text/javascript" integrity="foo"> becomes <script type="text/javascript" cypress:stripped-integrity="foo">
  • Cache rewritten code by ETags and potentially with a hash so it only needs 1 rewrite per session
    • HTTP caching semantics are hard, and existing cache mechanisms will still work inside a single browser session, so this can be implemented later
  • Fix SIGSEGV when exiting with threads active: SIGABRT when calling process.exit with worker_threads active electron/electron#23366
    • fixed with an ugly hack on intercepting process.exit, 😢
  • Throw error in driver if AST rewriting fails, since this is a bug in Cypress.

Note: This PR is huge because it does not replace the regex-based rewriting (yet), so there are some duplicate code paths and tests.

Checklist before removing "experimental" status: #7297

How has the user experience changed?

More user sites will work OOTB with Cypress 🎉

PR Tasks

@cypress
Copy link

cypress bot commented Oct 2, 2019



Test summary

7402 0 102 0


Run details

Project cypress
Status Passed
Commit 5877e66
Started May 11, 2020 4:26 PM
Ended May 11, 2020 4:32 PM
Duration 06:05 💡
OS Linux Debian - 10.1
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@brian-mann
Copy link
Member

@flotwig can we also fix this issue with this PR? #5099

@brian-mann
Copy link
Member

@flotwig another important note - if we apply these rules to the entire proxy then we will be rewriting virtually all JS files - which will break <script> tags using the integrity attribute.

We'll need to strip those too. https://developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity

@brian-mann
Copy link
Member

@flotwig another note - we should capture when window.location setters are used to navigate the page in JS such as window.location = '...' and window.location.href = '...' - so we can notify Cypress and capture the stack trace as well.

@flotwig
Copy link
Contributor Author

flotwig commented Oct 18, 2019

@flotwig another important note - if we apply these rules to the entire proxy then we will be rewriting virtually all JS files - which will break <script> tags using the integrity attribute.

We'll need to strip those too. developer.mozilla.org/en-US/docs/Web/Security/Subresource_Integrity

This uses hyntax to do streaming parsing of HTML, so this should be easy to add. It will be the first transform that we do directly to the HTML.

@jennifer-shehane
Copy link
Member

Will this also address this issue? #3994

@flotwig
Copy link
Contributor Author

flotwig commented Jan 16, 2020

@jennifer-shehane it will enable us to build a workaround for that issue, I believe, by making it so we can rewrite the use of window.location.(href|replace)

@flotwig flotwig mentioned this pull request Jan 17, 2020
flotwig added a commit to cypress-io/cypress-documentation that referenced this pull request May 8, 2020
@flotwig flotwig marked this pull request as ready for review May 8, 2020 17:04
Copy link
Contributor

@kuceb kuceb left a comment

Choose a reason for hiding this comment

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

made suggested changes as commit

@drumbeg
Copy link

drumbeg commented May 21, 2020

Early testing looking very promising on #4576.

@jennifer-shehane
Copy link
Member

@drumbeg This fix is available starting in 4.6.0 as an experiment which you can access by setting this config option in your cypress.json or elsewhere:

{
	"experimentalSourceRewriting": true
}

The fix is experimental, so there may be some situations where the this is not fixed.

If you're still this issue while setting the experimentalSourceRewriting to true in 4.6.0 - open a new issue with a reproducible example + screenshots, etc - filling out our issue template.

@cypress-io cypress-io locked as resolved and limited conversation to collaborators May 21, 2020
@flotwig flotwig deleted the issue-5271-fix-window-references branch January 24, 2022 18:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

8 participants