Merge branch 'pr/18'

* pr/18:
  Fix modulo bias

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;
-}
+};

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]);
+    });
+  });
+
 });