-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[wasm][tests] Use a polyfill library to provide Web Crypto API #42731
Conversation
Tagging subscribers to this area: @dotnet/ncl |
Ah, cool. We'll have something more legit when running against v8. |
Why do we need this? This looks like the scenario we don't need to support and the one we need to support cannot be tested this way |
It provides the same interface as the browser web cryptography api that can be run in the CI test harness without the need for browser frontend to provide them. Edit: Also to follow up on the back of Steve's comment it does provide a pretty much real-ish implementation of the getRandomValues instead of workaround.
|
@marek-safar While not absolutely necessary, I think it's more 'complete' to have the polyfill. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@steveisok the idea was to cut any custom implementation for crypto by calling into browser's APIs. Replacing one external implementation with another proxy implementation does not help us as we'll responsible for all the deployment and security problems.
This will not be deployed with our implementations. It is only to facilitate the testing of the API via the CI. The test code will be written to the browser spec and the polyfill will only facilitate the test api without the need for browser test harness at all. Actually what the polyfill implementation that is linked was written for. |
This adds ~400KB to the source repos. Since we'll probably need to keep these new dependencies up-to-date, I imagine this cost will grow over time. Is there any other way to pull this in? For example, via a runtime-assets package? |
Also, if we're going to take a dependency on the MSR polyfill, what commit hash are we basing these files from? I can send an update for Component Governance once you're finished. |
I don’t think it’s worth taking this dependency. I would recommend we close the PR. |
Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @jeffhandley |
Closing the PR as it brings in an unwanted dependency as per the comments above. Thanks everyone for providing feedback. |
Use Microsoft's provided Web Cryptography polyfill API in the test CI.
runtime.js
to load extra scripts such as this one:--extra-scripts=
--extra-scripts=
parameter takes comma separated list of scripts to load.getRandomValues
can be found in the PR [wasm][crypto] RandomNumberGenerator mapped to Web Crypto getRandomValues #42728close #42730
part of #40074