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.

Sign up for GitHub

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

Jump to bottom

getRandomValues() and BigInt #255

Closed
annevk opened this issue May 3, 2021 · 4 comments · Fixed by #266
Closed

getRandomValues() and BigInt #255

annevk opened this issue May 3, 2021 · 4 comments · Fixed by #266
Labels
proposal

Comments

@annevk
Copy link
Member

annevk commented May 3, 2021

As per web-platform-tests/wpt#27920 (comment) this should be clarified whether BigInt64Array and BigUint64Array are to be supported. It should probably also not speak of an "integer type" and instead replace that with the text between parenthesis.

If they are not supported, please turn this into a feature request as it would make sense for them to work here.

cc @bathos
@twiss
Copy link
Member

twiss commented May 3, 2021

They aren't supported anywhere I tested - so I'll make the change you suggested. I believe, as the linked comment does, that the intent was to exclude Float32Array and Float64Array (the issue with those is that the caller might expect some uniform distribution that you don't get if you interpret random bytes as floats). I don't see any issues with BigInt64Array and BigUint64Array, so I agree it would make sense to add support for those, if browsers are interested in implementing this.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 2, 2021
@TimothyGu @chromium-wpt-export-bot
Support BigInt/Uint64Array in crypto.getRandomValues
c8c3f25 
The check for "integer arrays" is to prevent people from using
getRandomValues with floating-point arrays, which doesn't work as people
might expect. However, the check was not updated when Chrome gained
support for BigInt64Array and BigUint64Array, which do work as expected.
This allows code fragments such as

    const b = new BigInt64Array(10);
    crypto.getRandomValues(b);

See w3c/webcrypto#255 for the spec issue.

Bug: 1225765
Change-Id: I338ecc5594492e6f329580f4e8f04d367f866361
Merged
@TimothyGu
Copy link
Member

Filed issues:

Tests are in web-platform-tests/wpt#29565. I'm working on implementing this change in Chrome right now, and I will get around to implementing it in Firefox soon. I think this would likely satisfy the implementor interest requirement.

@twiss twiss mentioned this issue Jul 2, 2021
Merged
@twiss
Copy link
Member

twiss commented Jul 2, 2021

@TimothyGu Thanks! I've opened #266 to fix this issue, let me know if you have any comments there as well.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 5, 2021
@TimothyGu @chromium-wpt-export-bot
Support BigInt/Uint64Array in crypto.getRandomValues
4c88fc7 
The check for "integer arrays" is to prevent people from using
getRandomValues with floating-point arrays, which doesn't work as people
might expect. However, the check was not updated when Chrome gained
support for BigInt64Array and BigUint64Array, which do work as expected.
This allows code fragments such as

    const b = new BigInt64Array(10);
    crypto.getRandomValues(b);

See w3c/webcrypto#255 for the spec issue.

Bug: 1225765
Change-Id: I338ecc5594492e6f329580f4e8f04d367f866361
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2021
@TimothyGu @chromium-wpt-export-bot
Support BigInt/Uint64Array in crypto.getRandomValues
6820081 
The check for "integer arrays" is to prevent people from using
getRandomValues with floating-point arrays, which doesn't work as people
might expect. However, the check was not updated when Chrome gained
support for BigInt64Array and BigUint64Array, which do work as expected.
This allows code fragments such as

    const b = new BigInt64Array(10);
    crypto.getRandomValues(b);

See w3c/webcrypto#255 for the spec issue.

Bug: 1225765
Change-Id: I338ecc5594492e6f329580f4e8f04d367f866361
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3002049
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Timothy Gu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#898924}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 6, 2021
@TimothyGu @chromium-wpt-export-bot
Support BigInt/Uint64Array in crypto.getRandomValues
b342b70 
The check for "integer arrays" is to prevent people from using
getRandomValues with floating-point arrays, which doesn't work as people
might expect. However, the check was not updated when Chrome gained
support for BigInt64Array and BigUint64Array, which do work as expected.
This allows code fragments such as

    const b = new BigInt64Array(10);
    crypto.getRandomValues(b);

See w3c/webcrypto#255 for the spec issue.

Bug: 1225765
Change-Id: I338ecc5594492e6f329580f4e8f04d367f866361
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3002049
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Timothy Gu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#898924}
pull bot pushed a commit to Yannic/chromium that referenced this issue Jul 7, 2021
@TimothyGu
Support BigInt/Uint64Array in crypto.getRandomValues
88825b3 
The check for "integer arrays" is to prevent people from using
getRandomValues with floating-point arrays, which doesn't work as people
might expect. However, the check was not updated when Chrome gained
support for BigInt64Array and BigUint64Array, which do work as expected.
This allows code fragments such as

    const b = new BigInt64Array(10);
    crypto.getRandomValues(b);

See w3c/webcrypto#255 for the spec issue.

Bug: 1225765
Change-Id: I338ecc5594492e6f329580f4e8f04d367f866361
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3002049
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Timothy Gu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#898924}
This was referenced Jul 19, 2021
Closed
Closed
@lucacasonato
Copy link
Contributor

Tracking issue for Deno: denoland/deno#11445
Tracking issue for Node: nodejs/node#39442

@targos targos mentioned this issue Jul 19, 2021
Closed
targos added a commit to targos/node that referenced this issue Jul 19, 2021
@targos
crypto: support Big(U)Int64Array in getRandomValues
a3eba4a 
Refs: w3c/webcrypto#255
Fixes: nodejs#39442
targos added a commit to targos/node that referenced this issue Jul 19, 2021
@targos
crypto: support Big(U)Int64Array in getRandomValues
c8ec35d 
Refs: w3c/webcrypto#255
Fixes: nodejs#39442
nodejs-github-bot pushed a commit to nodejs/node that referenced this issue Jul 23, 2021
@targos @nodejs-github-bot
crypto: support Big(U)Int64Array in getRandomValues
56a7e0a 
Refs: w3c/webcrypto#255
Fixes: #39442

PR-URL: #39443
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
targos added a commit to nodejs/node that referenced this issue Jul 25, 2021
@targos
crypto: support Big(U)Int64Array in getRandomValues
4ae5289 
Refs: w3c/webcrypto#255
Fixes: #39442

PR-URL: #39443
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
richardlau pushed a commit to nodejs/node that referenced this issue Jul 29, 2021
@targos @richardlau
crypto: support Big(U)Int64Array in getRandomValues
66c1bcc 
Refs: w3c/webcrypto#255
Fixes: #39442

PR-URL: #39443
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
BethGriggs pushed a commit to nodejs/node that referenced this issue Jul 29, 2021
@targos @BethGriggs
crypto: support Big(U)Int64Array in getRandomValues
d16d36f 
Refs: w3c/webcrypto#255
Fixes: #39442

PR-URL: #39443
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
@TimothyGu @copybara-github
Support BigInt/Uint64Array in crypto.getRandomValues
b7caea7 
The check for "integer arrays" is to prevent people from using
getRandomValues with floating-point arrays, which doesn't work as people
might expect. However, the check was not updated when Chrome gained
support for BigInt64Array and BigUint64Array, which do work as expected.
This allows code fragments such as

    const b = new BigInt64Array(10);
    crypto.getRandomValues(b);

See w3c/webcrypto#255 for the spec issue.

Bug: 1225765
Change-Id: I338ecc5594492e6f329580f4e8f04d367f866361
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3002049
Reviewed-by: Mike West <[email protected]>
Commit-Queue: Timothy Gu <[email protected]>
Cr-Commit-Position: refs/heads/master@{#898924}
NOKEYCHECK=True
GitOrigin-RevId: 88825b38ea9859ca06f31aa60bc8e8ea6563f57d
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
proposal
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow passing a BigInt64Array/BigUint64Array to getRandomValues w3c/webcrypto
4 participants
@twiss @TimothyGu @annevk @lucacasonato