brendanashworth / generate-password

NodeJS library for generating cryptographically-secure passwords.

MIT License

354 stars 67 forks source link

Changed randomNumber to be no longer biased #26

Closed Starkteetje closed 5 years ago

Starkteetje commented 6 years ago

Before in randomNumber return crypto.randomBytes(1)[0] % max; was used to generate the random number. Using modulo at the end does lead to a bias in the probability distribution of the generated characters if the character count does not divide 256. E.g. if using randomNumber(255) the number 0 is twice as likely as all other numbers and thus the character 'a' will appear much more frequently.

The fix is to discard the random value if it is above the highest mulitple of the character count that is not greater than 256.

brendanashworth commented 5 years ago

@Starkteetje thank you, you're right about the bias, this needs to be fixed. I have a small question, but besides that I really appreciate the work.

codecov-io commented 5 years ago

Codecov Report

Merging #26 into master will not change coverage. The diff coverage is 100%.

@@          Coverage Diff          @@
##           master    #26   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          48     51    +3     
=====================================
+ Hits           48     51    +3

Impacted Files	Coverage Δ
src/generate.js	`100% <100%> (ø)`	:arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 4d40444...89377d6. Read the comment docs.

brendanashworth commented 5 years ago

Thanks @Starkteetje, I'll be publishing a release with this soon!