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

patch: Add dynamic execution flag to emcc build to allow for wasm-unsafe-eval in csp #263

Merged
merged 1 commit into from
Sep 19, 2022

Conversation

zplata
Copy link
Contributor

@zplata zplata commented Sep 16, 2022

Should help address #131

If folks set CSP policies that block unsafe-eval scripts (i.e use of new Function() or eval()), they may have issues rendering Rives because our WASM (built using Emscripten) that has binding code to JS includes some new Function() code as part of Emscripten's inner-workings around binding. There's some effort on Emscripten's side to remove some of these pieces, but the guidance for consumers as seen in this issue is to allow wasm-unsafe-eval in the CSP. This alone however still doesn't solve everything. We need to set this DYNAMIC_EXECUTION=0 flag to prevent the use of new Function() or eval() in Emscripten's native binding code during build. The pairing of this fix in our WASM build setup, and the consumer setting wasm-unsafe-eval should get Rives running in web apps if it were blocked before. While not perfect, it's better than setting unsafe-eval for sure as a content policy.

If this takes, we'll document this in our JS runtimes gitbook section

@csmartdalton
Copy link
Contributor

Could you add a note about what is causing this error in Rive, and what it means for our code to enable wasm-unsafe-eval? I read that and it sounds scary to enable 😱

@zplata
Copy link
Contributor Author

zplata commented Sep 16, 2022

@csmartdalton yeah good point! Sorry, didn't really document the issue clearly. I updated the description.

We're not enabling any wasm-unsafe-eval on our side; this is a CSP policy attribute set from the user side of things on their web apps if they decide to set content security policies so that their apps aren't blocking our WASM from running

Copy link
Contributor

@csmartdalton csmartdalton left a comment

Choose a reason for hiding this comment

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

Thanks for clarifying! I understand what's going on now

@zplata zplata merged commit 273e541 into master Sep 19, 2022
@zplata zplata deleted the zp/fix-wasm-setup branch September 19, 2022 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants