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.

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

Use fast, documented generator instead of slow, undocumented generator #6

Closed
wants to merge 4 commits into from

Conversation

tniessen
Copy link
Contributor

@tniessen tniessen commented Sep 1, 2018

crypto.pseudoRandomBytes is deprecated on nodejs:master, see nodejs/node@221df22. This replaces the call to the deprecated API with crypto.randomBytes which is functionally equivalent.

In recent versions of Node.js, crypto.randomBytes usually produces pseudo-random bytes with no delay. Only if the CSPRNG provided by OpenSSL is not properly seeded and the system is low on entropy a noticeable delay of a few milliseconds can occur, which is basically never.

I changed this PR to produce non-cryptographically strong pseudorandom numbers after talking to @iarna about it. This is about two to three times faster and has a similar entropy.

crypto.pseudoRandomBytes is deprecated on nodejs:master. This replaces
the call to the deprecated API with crypto.randomBytes which is
functionally equivalent.
@Trott
Copy link
Contributor

Trott commented Dec 29, 2018

This will fix an issue that results in a broken test in npm, at least when run via make test-npm in Node.js core. It would be great if this could land and a release could go out, that would be great.

I updated .travis.yml at #7 so that tests will pass on Travis.

@Trott
Copy link
Contributor

Trott commented Dec 30, 2018

This will fix an issue that results in a broken test in npm, at least when run via make test-npm in Node.js core.

Sheepish update: The warning was only causing problems because I run with --pending-deprecation enabled by default. Sorry about that!

Still, it would be great to move off the deprecated alias and use the supported name for the function. This way, if Node.js ever does runtime-deprecate the alias, npm users won't start seeing deprecation warnings all of a sudden...

@tniessen tniessen changed the title Use non-deprecated random API Use fast, documented and non-cryptographically strong PRNG instead of slow, undocumented generator Dec 30, 2018
@tniessen tniessen changed the title Use fast, documented and non-cryptographically strong PRNG instead of slow, undocumented generator Use fast, documented generator instead of slow, undocumented generator Feb 21, 2019
@zkat
Copy link
Contributor

zkat commented Jun 1, 2019

merged

@zkat zkat closed this Jun 1, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants