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 CSP error for wasm bindgen #92

Merged
merged 1 commit into from
Aug 17, 2019

Conversation

evq
Copy link
Contributor

@evq evq commented Aug 17, 2019

Fixes #91

Access the global object via self directly instead of the current return this function created from a string. Creating a function from a string results in a CSP error as it requires unsafe-eval.

See tweetnacl which uses this strategy (although it also checks for msCrypto for IE11 compatibility - here we don't because IE11 does not have wasm support).

Tested on node, firefox and brave (chromium) using the wasm-pack test.

Copy link
Member

@newpavlov newpavlov left a comment

Choose a reason for hiding this comment

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

Thank you! LGTM.

@josephlr
If you don't have objections I will merge it.

@josephlr
Copy link
Member

I think we would be better off using js_sys to get the global object, as that seems to be the more supported way of doing things

@evq
Copy link
Contributor Author

evq commented Aug 17, 2019

The js_sys strategy is a bit overkill here since we only need a way to fetch the global object on browsers, for which self should always work. They have to go through a lot of extra hoops because they're trying to make it possible to get the global object even on nodejs and other non-browser platforms.

@newpavlov newpavlov mentioned this pull request Aug 17, 2019
Copy link
Member

@josephlr josephlr left a comment

Choose a reason for hiding this comment

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

@evq that makes sense. @newpavlov we should think about using js_sys/web_sys in a different PR, but I think merging this and fixing #91 before releasing 0.1.10 is a good idea.

@newpavlov newpavlov merged commit f073a95 into rust-random:master Aug 17, 2019
@newpavlov
Copy link
Member

I think we can discuss it in #63, but I still don't see a reason why we would want to use crates built on top of wasm-bindgen instead of using it directly.

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.

use with wasm-bindgen triggers CSP error
3 participants