downgrade regenerator-runtime due to unsafe eval usages #1062

oshi97 · 2020-09-03T15:19:12Z

This commit downgrades regenerator-runtime from
^0.13.3 (currently 0.13.7) to being pinned to exactly
0.13.1. This is because there is a change in 0.13.2 which
adds back in a Function() constructor to define
regenerator runtime globally.
See #369 https://github.com/facebook/regenerator/commits/master/packages/regenerator-runtime

T=https://yextops.zendesk.com/agent/tickets/347915
TEST=manual

tested with a local apache httpd server on a searchbar only page
added below line to .htaccess, which sets the CSP to
the same as Syncreon

also test csp with a <meta> tag

tested both answers.js, answers.min.js, answers-modern.js, and answers-modern.min.js

double checked that our code still runs correctly in ie11, for both a searchbar only
page with no unsafe-eval csp, and a regular page with facets and results
that allows unsafe eval

oshi97 · 2020-09-03T16:16:14Z

ci isn't working for me for some reason (couldn't figure out why so filed a support ticket), manually ran npm run build and npm test
edit: now working no idea what was happening

By default, rollup bundles js in strict mode. When regenerator-runtime is included in use-strict mode, they use a Function() call (which is like a slightly safer eval()) to initialize things. Specifically ```js Function("r", "regeneratorRuntime = r")(runtime); ``` Removing use-strict mode lets regenerator runtime init without any unsafe evals. Because strict mode javascript is a subset of regular javascript, this should not break any code. AFAIK there is no way to require a script tag to be in 'use strict' mode. T=https://yextops.zendesk.com/agent/tickets/347915 TEST=manual tested with a local apache httpd server added below line to .htaccess, which sets the CSP to the same as Syncreon also test csp with a `<meta>` tag tested both answers.js and answers.min.js

tmeyer2115 · 2020-09-08T22:13:47Z

conf/gulp-tasks/library.gulpfile.js

@@ -68,7 +68,8 @@ function legacyBundleIIFE () {
  return legacyBundle({
    format: 'iife',
    name: NAMESPACE,
-    sourcemap: true
+    sourcemap: true,


Do we need to fix this for the modern bundle as well?

nope! just checked, it's not an issue for the modern bundle. I believe it's because the modern bundle doesn't have the regenerator-runtime polyfill (or any of the polyfill-prefix stuff)?

updated the test desc

tmeyer2115

I'm a bit wary of just disabling strict mode. How much research have we done into it? I took a quick look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode. It sounds like strict mode has some benefits:

Can produce more optimized JS.
Can help produce more secure JS.

Are we sure we want to get rid of this? Is it standard for libraries such as ours to use strict mode? Others have encountered this same CSP issue: mozilla/pdf.js#11036. One recommendation was to downgrade regenerator-runtime (apparently an older version(s) did not have this problem with strict mode). Is that a viable option here?

tmeyer2115 · 2020-09-09T14:00:23Z

I'm a bit wary of just disabling strict mode. How much research have we done into it? I took a quick look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode. It sounds like strict mode has some benefits:

Can produce more optimized JS.

Can help produce more secure JS.

Are we sure we want to get rid of this? Is it standard for libraries such as ours to use strict mode? Others have encountered this same CSP issue: mozilla/pdf.js#11036. One recommendation was to downgrade regenerator-runtime (apparently an older version(s) did not have this problem with strict mode). Is that a viable option here?

I'm not sure that we're getting much performance benefit from strict mode. It sounds like strict mode supports better compiler optimization. Uglify is a compiler of sorts. Is the bundle size different at all between strict and sloppy?

oshi97 · 2020-09-09T14:27:55Z

I'm a bit wary of just disabling strict mode. How much research have we done into it? I took a quick look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Strict_mode. It sounds like strict mode has some benefits:

Can produce more optimized JS.

Can help produce more secure JS.

Are we sure we want to get rid of this? Is it standard for libraries such as ours to use strict mode? Others have encountered this same CSP issue: mozilla/pdf.js#11036. One recommendation was to downgrade regenerator-runtime (apparently an older version(s) did not have this problem with strict mode). Is that a viable option here?

I'm not sure that we're getting much performance benefit from strict mode. It sounds like strict mode supports better compiler optimization. Uglify is a compiler of sorts. Is the bundle size different at all between strict and sloppy?

The bundle size is identical. Since the SDK doesn't have regenerator runtime in develop anymore, we don't need this change in future versions, at least right now. I do want to add an acceptance test for no unsafe-eval csp on a searchbar only page, though (created slap-619 last week for this)

here are the two builds I looked at
https://app.circleci.com/pipelines/github/yext/answers/4300/workflows/09e8f06a-39bb-4732-a6a8-4cc591c0d94b/jobs/11833
https://app.circleci.com/pipelines/github/yext/answers/4299/workflows/56f52e95-829c-4a49-8a2d-b348bdfbf418/jobs/11832

This commit downgrades regenerator-runtime from ^0.13.3 (currently 0.13.7) to being pinned to exactly 0.13.1. This is because there is a change in 0.13.2 which adds back in a Function() constructor to define regenerator runtime globally. See #369 https://github.com/facebook/regenerator/commits/master/packages/regenerator-runtime T=https://yextops.zendesk.com/agent/tickets/347915 TEST=manual tested with a local apache httpd server on a searchbar only page added below line to .htaccess, which sets the CSP to the same as Syncreon also test csp with a `<meta>` tag tested both answers.js, answers.min.js, answers-modern.js, and answers-modern.min.js double checked that our code still runs correctly in ie11, for both a searchbar only page with no unsafe-eval csp, and a regular page with facets and results that allows unsafe eval

## Fixes: * Remove unsafe eval usages in a search bar only page (#1062) * Pagination: Fix disappearing pagination on back nav and refresh (#1060) * FilterOptions: fix ie11 issue with clearing searchable facets (#1058)

oshi97 requested review from alexisgrow, tmeyer2115, cea2aj and creotutar September 3, 2020 16:17

oshi97 force-pushed the dev/remove-use-strict branch from 25cf110 to a291373 Compare September 3, 2020 17:00

oshi97 changed the title ~~Remove use-strict from js non modern js bundle~~ disable use-strict in legacy js bundles to remove unsafe eval usages Sep 3, 2020

oshi97 force-pushed the support/v1.4 branch from 24c08e4 to 3037f88 Compare September 4, 2020 16:58

oshi97 force-pushed the dev/remove-use-strict branch from a291373 to 1e3df7c Compare September 4, 2020 16:59

tmeyer2115 reviewed Sep 8, 2020

View reviewed changes

tmeyer2115 self-requested a review September 9, 2020 13:35

tmeyer2115 reviewed Sep 9, 2020

View reviewed changes

downgrade regenerator-runtime to 0.13.1

1e2723b

oshi97 changed the title ~~disable use-strict in legacy js bundles to remove unsafe eval usages~~ downgrade regenerator-runtime due to unsafe eval usages Sep 9, 2020

tmeyer2115 self-requested a review September 9, 2020 17:40

tmeyer2115 approved these changes Sep 9, 2020

View reviewed changes

oshi97 merged commit 1efcda1 into support/v1.4 Sep 9, 2020

oshi97 deleted the dev/remove-use-strict branch September 18, 2020 18:51

oshi97 mentioned this pull request Oct 1, 2020

Version 1.5.5 #1117

Merged

agilgur5 mentioned this pull request Jan 13, 2021

CSP that blocks unsafe eval errors on regenerator-runtime jaredpalmer/tsdx#954

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

downgrade regenerator-runtime due to unsafe eval usages #1062

downgrade regenerator-runtime due to unsafe eval usages #1062

oshi97 commented Sep 3, 2020 •

edited

Loading

oshi97 commented Sep 3, 2020 •

edited

Loading

tmeyer2115 Sep 8, 2020

oshi97 Sep 8, 2020

oshi97 Sep 8, 2020 •

edited

Loading

tmeyer2115 left a comment

tmeyer2115 commented Sep 9, 2020

oshi97 commented Sep 9, 2020 •

edited

Loading

downgrade regenerator-runtime due to unsafe eval usages #1062

downgrade regenerator-runtime due to unsafe eval usages #1062

Conversation

oshi97 commented Sep 3, 2020 • edited Loading

oshi97 commented Sep 3, 2020 • edited Loading

tmeyer2115 Sep 8, 2020

Choose a reason for hiding this comment

oshi97 Sep 8, 2020

Choose a reason for hiding this comment

oshi97 Sep 8, 2020 • edited Loading

Choose a reason for hiding this comment

tmeyer2115 left a comment

Choose a reason for hiding this comment

tmeyer2115 commented Sep 9, 2020

oshi97 commented Sep 9, 2020 • edited Loading

oshi97 commented Sep 3, 2020 •

edited

Loading

oshi97 commented Sep 3, 2020 •

edited

Loading

oshi97 Sep 8, 2020 •

edited

Loading

oshi97 commented Sep 9, 2020 •

edited

Loading