Skip to content

crypto: forbid passing Float16Array to getRandomValues()#57880

Merged
nodejs-github-bot merged 2 commits intonodejs:mainfrom
LiviaMedeiros:crypto-getrandomvalues-no-float16array
Apr 17, 2025
Merged

crypto: forbid passing Float16Array to getRandomValues()#57880
nodejs-github-bot merged 2 commits intonodejs:mainfrom
LiviaMedeiros:crypto-getrandomvalues-no-float16array

Conversation

@LiviaMedeiros
Copy link
Member

@LiviaMedeiros LiviaMedeiros commented Apr 14, 2025

Depends on: #57879

@LiviaMedeiros LiviaMedeiros added the blocked PRs that are blocked by other issues or PRs. label Apr 14, 2025
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 14, 2025

Review requested:

  • @nodejs/crypto

@nodejs-github-bot nodejs-github-bot added crypto Issues and PRs related to the crypto subsystem. needs-ci PRs that need a full CI run. labels Apr 14, 2025
@anonrig
Copy link
Member

anonrig commented Apr 15, 2025

Can you add a test?

@LiviaMedeiros
Copy link
Member Author

LiviaMedeiros commented Apr 15, 2025

I believe this is already covered by wpt (although the relevant test is temporarily disabled exactly because of no Float16Array support), but if you think it needs to be tested here as well, I can add it.

@LiviaMedeiros LiviaMedeiros mentioned this pull request Apr 15, 2025
Merged
@aduh95 aduh95 added dont-land-on-v18.x dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. and removed dont-land-on-v18.x dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. dont-land-on-v22.x PRs that should not land on the v22.x-staging branch and should not be released in v22.x. labels Apr 15, 2025
@aduh95
Copy link
Contributor

aduh95 commented Apr 15, 2025

Adding the dont-land label as I think there's no point to have this check on release lines that do not have Float16Array support

@aduh95
Copy link
Contributor

aduh95 commented Apr 15, 2025

I believe this is already covered by wpt (although the relevant test is temporarily disabled exactly because of no Float16Array support), but if you think it needs to be tested here as well, I can add it.

It would probably make sense to enable the test in this PR, passing the V8 flag to expose Float16Array to WPT tests

@aduh95 aduh95 marked this pull request as ready for review April 16, 2025 22:48
@aduh95 aduh95 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. and removed blocked PRs that are blocked by other issues or PRs. labels Apr 16, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 16, 2025
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 16, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/66324/

@codecov
Copy link

codecov bot commented Apr 16, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.23%. Comparing base (b2405e9) to head (d5fe39e).
Report is 15 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #57880      +/-   ##
==========================================
- Coverage   90.23%   90.23%   -0.01%     
==========================================
  Files         630      630              
  Lines      185688   185788     +100     
  Branches    36405    36416      +11     
==========================================
+ Hits       167559   167648      +89     
- Misses      11000    11007       +7     
- Partials     7129     7133       +4
Files with missing lines Coverage Δ
lib/internal/crypto/random.js 96.09% <100.00%> (+0.01%) ⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 17, 2025

CI: https://ci.nodejs.org/job/node-test-pull-request/66325/

@aduh95 aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Apr 17, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 17, 2025
@nodejs-github-bot nodejs-github-bot merged commit c11c7be into nodejs:main Apr 17, 2025
85 checks passed
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Apr 17, 2025

Landed in c11c7be

RafaelGSS pushed a commit that referenced this pull request May 1, 2025
@LiviaMedeiros @RafaelGSS
crypto: forbid passing Float16Array to getRandomValues()
a1ad082 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@RafaelGSS RafaelGSS mentioned this pull request May 1, 2025
Merged
RafaelGSS pushed a commit that referenced this pull request May 2, 2025
@LiviaMedeiros @RafaelGSS
crypto: forbid passing Float16Array to getRandomValues()
0f55a96 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
62cd7c5 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 6, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
2ac49e4 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
RafaelGSS pushed a commit that referenced this pull request May 14, 2025
@LiviaMedeiros @RafaelGSS
crypto: forbid passing Float16Array to getRandomValues()
426298e 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 16, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
46ac523 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
f65cd7c 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
63f0197 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 17, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
05f8709 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
aduh95 pushed a commit that referenced this pull request May 18, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
be40386 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@github-actions github-actions bot mentioned this pull request May 19, 2025
Merged
aduh95 pushed a commit that referenced this pull request May 19, 2025
@LiviaMedeiros @aduh95
crypto: forbid passing Float16Array to getRandomValues()
f606352 
PR-URL: #57880
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
@ghost ghost mentioned this pull request Jun 8, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@panva panva panva approved these changes

@aduh95 aduh95 aduh95 approved these changes

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. crypto Issues and PRs related to the crypto subsystem. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. needs-ci PRs that need a full CI run.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@LiviaMedeiros @nodejs-github-bot @anonrig @aduh95 @panva