Skip to content

Conversation

@kjpou1
Copy link
Contributor

@kjpou1 kjpou1 commented Feb 24, 2021

Add batched implementation to fix error with quota exceeded limit.

Add and/or update tests for exceeding the quota limit of browser set at 65536.

The tests only went up to 65536 so this error was never caught.

A small repo from issue:

Span<byte> data = new byte[ 1024 * 1024 ];
RandomNumberGenerator.Fill( data );

1024 * 1024 = 1048576

Fix #48584

@ghost ghost added the area-System.Security label Feb 24, 2021
@kjpou1 kjpou1 requested a review from vcsjones February 24, 2021 08:38
@ghost
Copy link

ghost commented Feb 24, 2021

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

Add batched implementation to fix error with quota exceeded limit.

Add and/or update tests for exceeding the quota limit of browser set at 65536.

The tests only went up to 65536 so this error was never caught.

Fix #48584

Author: kjpou1
Assignees: -
Labels:

area-System.Security

Milestone: -

@kjpou1 kjpou1 self-assigned this Feb 24, 2021
@kjpou1 kjpou1 added the arch-wasm WebAssembly architecture label Feb 24, 2021
@ghost
Copy link

ghost commented Feb 24, 2021

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

Add batched implementation to fix error with quota exceeded limit.

Add and/or update tests for exceeding the quota limit of browser set at 65536.

The tests only went up to 65536 so this error was never caught.

Fix #48584

Author: kjpou1
Assignees: kjpou1
Labels:

arch-wasm, area-System.Security

Milestone: -

@kjpou1 kjpou1 added this to the Future milestone Feb 24, 2021
@marek-safar marek-safar requested a review from kg February 24, 2021 08:59
@kg
Copy link
Member

kg commented Feb 24, 2021

Is there a specific reason why we aren't just writing directly into the wasm heap using views, instead of using these scratch work arrays?

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 24, 2021

Do you mean this one: var wrkArray = new Uint8Array(Module.HEAPU8.buffer, buffer, bufferLength);

That does write into wasm heap.

@kg
Copy link
Member

kg commented Feb 24, 2021

Do you mean this one: var wrkArray = new Uint8Array(Module.HEAPU8.buffer, buffer, bufferLength);

That does write into wasm heap.

I don't think you need wrkArray, you should be able to create small views into the wasm heap and then use crypto.getRandomValues to write directly into the view

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 24, 2021

ok

@kjpou1
Copy link
Contributor Author

kjpou1 commented Feb 24, 2021

updated to use suggested code from katelyn

@kjpou1 kjpou1 merged commit 6b7263d into dotnet:master Feb 24, 2021
@GrabYourPitchforks
Copy link
Member

Doesn't line 13 have the potential to integer overflow?

@kg
Copy link
Member

kg commented Feb 24, 2021

Doesn't line 13 have the potential to integer overflow?

It's not an integer, so no. JS can represent integral numbers up to 53 bits in size

@kjpou1 kjpou1 deleted the wasm-quotaexceeded-getrandomvalues branch February 25, 2021 05:32
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-System.Security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RandomNumberGenerator.Fill( Span<byte> data ) fails in browser when data length is > 65536.

6 participants