Use rejection sampling to avoid bias in generateRandomString()#78
Conversation
🦋 Changeset detectedLatest commit: fdb93fc The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Hmm, thanks for taking a pass at it, but I was sort of thinking we should just add two characters to the alphabet instead. If we add |
|
Even easier! I am perfectly happy to do that instead, but it seemed that you might have had a few reservations about taking that approach so I figured this would be a fun little exercise to try. If not that simplifies the implementation a good bit 🙂 |
|
Done; I made sure |
commit: |
|
@threepointone I think we should merge this but I've lost track of the tooling setup so not sure if there's something we should add, like a changeset or whatever, before merging. Can you take it? |
|
And, thanks @rc4! |
|
on it |
threepointone
force-pushed
the
fix-42-generateRandomString-bias
branch
from
8966873 to
fdb93fc
Compare
This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @cloudflare/workers-oauth-provider@0.0.11 ### Patch Changes - [#78](#78) [`32560d1`](32560d1) Thanks [@rc4](https://github.com/rc4)! - Use rejection sampling to avoid bias in `generateRandomString()` Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
released in 0.0.11, thank you! |
3 participants
Fix #42 by rejecting outputs >=
characters.length * 4(248).We initially generate 10% more random bytes than requested. If we expect to reject 3.1% of them; only one call to
crypto.getRandomValues()should be required in >99% of cases.If not, we get fresh values and go again until we've satisfied the requested length.
Let me know if there are any questions/concerns/anything I need to do on my end - I dug through the docs and it seemed pretty straightforward to contribute. 😄