Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix invalid invocation on getRandomValues() #2730

Merged
merged 1 commit into from
Sep 18, 2024
Merged

Conversation

anonrig
Copy link
Member

@anonrig anonrig commented Sep 17, 2024

Fixes #2716

@anonrig anonrig requested a review from jasnell September 17, 2024 20:12
@anonrig anonrig requested review from a team as code owners September 17, 2024 20:12
@anonrig anonrig requested a review from garrettgu10 September 17, 2024 20:12
@anonrig anonrig force-pushed the yagiz/fix-getrandombytes branch 2 times, most recently from 3eda8c8 to 1af6beb Compare September 17, 2024 20:14
Copy link
Contributor

@petebacondarwin petebacondarwin 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 fixing this. Can you check that other scenario too?

src/workerd/api/node/tests/crypto_random-test.js Outdated Show resolved Hide resolved
@anonrig anonrig force-pushed the yagiz/fix-getrandombytes branch from 1af6beb to e54450b Compare September 17, 2024 20:19
@anonrig anonrig force-pushed the yagiz/fix-getrandombytes branch from e54450b to 8b358ec Compare September 17, 2024 20:25
async test() {
const crypto = await import('node:crypto');
{
// The following assertion doesn't fail as of Node.js v20.17.0
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't fail for me in node 18 either...

@anonrig anonrig requested review from npaun and mikea September 17, 2024 20:28
@IgorMinar
Copy link
Collaborator

thank you @anonrig!!

@anonrig
Copy link
Member Author

anonrig commented Sep 17, 2024

@IgorMinar 🫡

@anonrig anonrig merged commit 20f3a42 into main Sep 18, 2024
12 of 13 checks passed
@anonrig anonrig deleted the yagiz/fix-getrandombytes branch September 18, 2024 10:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Bug Report — Runtime APIs - nodejs compat - crypto - getRandomBytes()
4 participants