crypto: fix WebCryptoAPI WebIDL harness #45590
Conversation
panva
added
crypto
|
Review requested:
|
nodejs-github-bot
added
needs-ci
|
@jasnell @addaleax @tniessen asking for your review wrt. the I've had a look at existing tests for KeyObject that deal with transferring the objects and made sure to add CryptoKey instances to those tests, they still pass, but I really don't pretend to fully understand the consequences of this change. |
|
@aduh95 asking for your review wrt. the global getter. It must not be callable unless this is globalThis (or undefined which is a special case for Window-like (globalThis) getters). |
github-actions
bot
removed
the
request-ci
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
cc @jasnell because of the |
| @@ -634,7 +637,7 @@ function isKeyObject(obj) { | |||
| // here similar to other things like URL. A chromium provided CryptoKey | |||
| // will not be recognized as a Node.js CryptoKey, and vice versa. It | |||
| // would be fantastic if we could find a way of making those interop. | |||
| class CryptoKey extends JSTransferable { | |||
There was a problem hiding this comment.
This is a breaking change. While WebCrypto doesn't define CryptoKey as transferable, we've implemented it this way from the beginning. Especially since we've already marked Web Crypto as stable we should preserve this.
There was a problem hiding this comment.
Would you take a closer look please? The change does not seem to have impacted CryptoKey's transferability. I added tests for it too.
|
I'll slice this up into smaller bits, #45659 messed up the idlharness tests even more anyway. |
Individual commits tell the journey. This fixes the remaining WebCryptoAPI idlharness WPTs. Only landing where #42083 is.