From 15ce2fd0841f46141cb2a3c7bf333e3ab064c416 Mon Sep 17 00:00:00 2001
From: Dmitry Chestnykh <dmitry@codingrobots.com>
Date: Fri, 12 Feb 2016 17:21:14 +0100
Subject: [PATCH] Fix modulo bias

Add tests for unique and unbiased strings.

Closes #15
---
 lib/randomstring.js | 33 +++++++++++++++++++++------------
 test/index.js       | 31 +++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/lib/randomstring.js b/lib/randomstring.js
index 8c7b129..7459ab9 100644
--- a/lib/randomstring.js
+++ b/lib/randomstring.js
@@ -3,6 +3,16 @@
 var crypto  = require('crypto');
 var Charset = require('./charset.js');
 
+function safeRandomBytes(length) {
+  while (true) {
+    try {
+      return crypto.randomBytes(length);
+    } catch(e) {
+      continue;
+    }
+  }
+}
+
 exports.generate = function(options) {
   
   var charset = new Charset();
@@ -40,19 +50,18 @@ exports.generate = function(options) {
   }
   
   // Generate the string
-  while (string.length < length) {
-    var bf;
-    try {
-      bf = crypto.randomBytes(length);
-    }
-    catch (e) {
-      continue;
-    }
-    for (var i = 0; i < bf.length; i++) {
-      var index = bf.readUInt8(i) % charset.chars.length;
-      string += charset.chars.charAt(index);
+  var charsLen = charset.chars.length;
+  var maxByte = 256 - (256 % charsLen);
+  while (length > 0) {
+    var buf = safeRandomBytes(Math.ceil(length * 256 / maxByte));
+    for (var i = 0; i < buf.length && length > 0; i++) {
+      var randomByte = buf.readUInt8(i);
+      if (randomByte < maxByte) {
+        string += charset.chars.charAt(randomByte % charsLen);
+        length--;
+      }
     }
   }
 
   return string;
-}
+};
diff --git a/test/index.js b/test/index.js
index 5e8c91d..3239ab4 100644
--- a/test/index.js
+++ b/test/index.js
@@ -66,4 +66,35 @@ describe("randomstring.generate(options)", function() {
     assert.equal(search, -1);
   });
 
+  it("returns unique strings", function() {
+    var results = {};
+    for (var i = 0; i < 1000; i++) {
+      var s = random();
+      assert.notEqual(results[s], true);
+      results[s] = true;
+    }
+    return true;
+  });
+
+  it("returns unbiased strings", function() {
+    var charset = 'abcdefghijklmnopqrstuvwxyz';
+    var slen = 100000;
+    var s = random({ charset: charset, length: slen });
+    var counts = {};
+    for (var i = 0; i < s.length; i++) {
+      var c = s.charAt(i);
+      if (typeof counts[c] === "undefined") {
+        counts[c] = 0;
+      } else {
+        counts[c]++;
+      }
+    }
+    var avg = slen / charset.length;
+    Object.keys(counts).sort().forEach(function(k) {
+      var diff = counts[k] / avg;
+      assert(diff > 0.95 && diff < 1.05,
+             "bias on `" + k + "': expected average is " + avg + ", got " + counts[k]);
+    });
+  });
+
 });