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 EvalError caused by regenerator-runtime (#4535) #4577

Merged
merged 1 commit into from Feb 25, 2021
Merged

Fix EvalError caused by regenerator-runtime (#4535) #4577

merged 1 commit into from Feb 25, 2021

Conversation

snoack
Copy link
Contributor

@snoack snoack commented Feb 18, 2021

Description of the Change

Mocha provides a bundle that can be used directly in the browser. For compatibility with Internet Explorer 11, the bundled code is transpiled using Babel. When using async functions (or generator functions) as Mocha started in 12db9db (8.2.0), Babel adds the regenerator-runtime module to the bundle, which relies on unsafe eval to access the global scope in strict mode, which in turn makes Mocha incompatible with environments that don't allow unsafe eval.

In this PR, I implement a workaround that prevents regenerator-runtime from using unsafe eval. First I had to update regenerator-runtime since the currently used version relies on this in an unbound function referring to the global object which we don't have any control over, while the new version instead exploits implicit global assignments:

try {
  regeneratorRuntime = runtime;
} catch (accidentalStrictMode) {
  // This module should not be running in strict mode, so the above
  // assignment should always work unless something is misconfigured. Just
  // in case runtime.js accidentally runs in strict mode, we can escape
  // strict mode using a global Function call. This could conceivably fail
  // if a Content Security Policy forbids using Function, but in that case
  // the proper solution is to fix the accidental strict mode problem. If
  // you've misconfigured your bundler to force strict mode and applied a
  // CSP to forbid Function, and you're not willing to fix either of those
  // problems, please detail your unique predicament in a GitHub issue.
  Function("r", "regeneratorRuntime = r")(runtime);
}

While this isn't working either in strict mode without unsafe eval, it allows us to work around the issue by declaring the regeneratorRuntime variable before this code is reached (done here via Rollup's output.intro setting). That way regeneratorRuntime = runtime is no longer an implicit definition of a global variable (which fails in strict mode), but it becomes a simple reassignment of an existing variable, and so the code in the catch block (which requires unsafe eval) is never reached.

Alternate Designs

  1. If it wouldn't be for Internet Explorer 11, we wouldn't need to transpile the code in the first place, and this issue wouldn't exist. There is also the option to tell Babel to just not use transform-regenerator in which case the transpiled code would at least remain compatible with legacy browsers that support generator functions but might lack other modern ES features.
  2. Refactoring the code to not use async/await.
  3. Providing two bundles, one with transpiled code that works in IE11 (but will require unsafe eval), and one that isn't transpiled.

Alternative 1 and 2 were ruled out by @juergba in the discussion in #4535. Alternative 3 seems somewhat more complicated than the workaround I ended up implementing here, and it would require users to choose the correct bundle for their environment.

Why should this be in core?

Because it fixes a regression introduced in Mocha 3.2.0.

Benefits

Mocha can be used once again in environments where unsafe eval is not allowed. This includes browser extensions, as well as websites with restrictive Content-Security-Policy.

@juergba
Copy link
Member

juergba commented Feb 19, 2021

@snoack thanks for this PR!

The browser tests never work on PR's from forked repos. This is a problem of secrets (saucelabs login info) not being shared with forked repos. I'm going to push your branch to Mocha's repo directly.

Edit: browser tests are passing now.

@juergba juergba added area: browser browser-specific type: bug a defect, confirmed by a maintainer dependencies Pull requests that update a dependency file labels Feb 19, 2021
package.json Outdated Show resolved Hide resolved
@juergba
Copy link
Member

juergba commented Feb 19, 2021

@Munter I need your review for this PR, please.

@juergba
Copy link
Member

juergba commented Feb 19, 2021

@snoack have you tested this new bundle in your environment?

@juergba juergba requested a review from a team February 19, 2021 08:32
@snoack
Copy link
Contributor Author

snoack commented Feb 19, 2021

Yes, I did. Works.

@juergba juergba linked an issue Feb 19, 2021 that may be closed by this pull request
@juergba juergba added the semver-patch implementation requires increase of "patch" version number; "bug fixes" label Feb 19, 2021
@juergba juergba added this to the next milestone Feb 19, 2021
Copy link
Member

@juergba juergba left a comment

Choose a reason for hiding this comment

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

@snoack thank you!
Please be patient for some days, I will wait for more reviews.

@juergba
Copy link
Member

juergba commented Feb 19, 2021

Babel seems to struggle a bit with publishing, see Babel v7.12.18.
v7.12.18 is not available yet on npm.

@juergba
Copy link
Member

juergba commented Feb 24, 2021

@snoack I'm going to merge this branch tomorrow.

Babel is publishing new releases every few hours, some kind of weird. I don't think we need to upgrade to the newest one, what do you think?

@snoack
Copy link
Contributor Author

snoack commented Feb 24, 2021

Sounds good.

This was referenced Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: browser browser-specific dependencies Pull requests that update a dependency file semver-patch implementation requires increase of "patch" version number; "bug fixes" type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

content_security_policy - EvalError: Refused to evaluate a string
2 participants