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: ensure Cloudflare webcrypto.getRandomBytes is bound to the correct this object #309

Merged
merged 1 commit into from
Sep 16, 2024

Conversation

petebacondarwin
Copy link
Contributor

@petebacondarwin petebacondarwin commented Sep 16, 2024

It appears that this is the only function in the webcrypto exports list that
needs to be bound to its original object.

Note that it must be bound to the webcrypto object even though workerd also
exports it on the crypto namespace directly.

@petebacondarwin petebacondarwin requested a review from a team as a code owner September 16, 2024 07:15
@petebacondarwin petebacondarwin marked this pull request as draft September 16, 2024 07:36
…ct this object

It appears that this is the only function in the webcrypto exports list that
needs to be bound to its original object.

Note that it must be bound to the `webcrypto` object even though workerd also
exports it on the `crypto` namespace directly.
Comment on lines +109 to +111
export const getRandomValues = workerdCrypto.getRandomValues.bind(
workerdCrypto.webcrypto,
);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the correct fix.
We need to ensure that every export of this function is correctly bound.
Not just the one from webcrypto.

@petebacondarwin petebacondarwin marked this pull request as ready for review September 16, 2024 08:01
@petebacondarwin petebacondarwin changed the title fix: ensure Cloudflare webcrypto bindings are bound to the correct this object fix: ensure Cloudflare webcrypto.getRandomBytes is bound to the correct this object Sep 16, 2024
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