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

crypto: add randomInt function #34600

Closed
wants to merge 11 commits into from
40 changes: 40 additions & 0 deletions doc/api/crypto.md
Original file line number Diff line number Diff line change
Expand Up @@ -2800,6 +2800,46 @@ threadpool request. To minimize threadpool task length variation, partition
large `randomFill` requests when doing so as part of fulfilling a client
request.

### `crypto.randomInt([min, ]max[, callback])`
<!-- YAML
added:
- v14.8.0
olalonde marked this conversation as resolved.
Show resolved Hide resolved
-->

* `min` {integer} Start of random range. **Default**: `0`.
* `max` {integer} End of random range (non-inclusive).
addaleax marked this conversation as resolved.
Show resolved Hide resolved
* `callback` {Function} `function(err, n) {}`.

Return a random integer `n` such that `min <= n < max`. This
implementation avoids [modulo
bias](https://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle#Modulo_bias).
olalonde marked this conversation as resolved.
Show resolved Hide resolved

The maximum supported range value (`max - min`) is `2^48 - 1`.

If the `callback` function is not provided, the random integer is generated
synchronously.

```js
// Asynchronous
crypto.randomInt(3, (err, n) => {
if (err) throw err;
console.log(`Random number chosen from (0, 1, 2): ${n}`);
});
```

```js
// Synchronous
const n = crypto.randomInt(3);
console.log(`Random number chosen from (0, 1, 2): ${n}`);
```

```js
crypto.randomInt(1, 7, (err, n) => {
jasnell marked this conversation as resolved.
Show resolved Hide resolved
if (err) throw err;
console.log(`The dice rolled: ${n}`);
});
```

### `crypto.scrypt(password, salt, keylen[, options], callback)`
<!-- YAML
added: v10.5.0
Expand Down
4 changes: 3 additions & 1 deletion lib/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,8 @@ const {
const {
randomBytes,
randomFill,
randomFillSync
randomFillSync,
randomInt
} = require('internal/crypto/random');
const {
pbkdf2,
Expand Down Expand Up @@ -184,6 +185,7 @@ module.exports = {
randomBytes,
randomFill,
randomFillSync,
randomInt,
scrypt,
scryptSync,
sign: signOneShot,
Expand Down
72 changes: 71 additions & 1 deletion lib/internal/crypto/random.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
const {
MathMin,
NumberIsNaN,
NumberIsInteger
} = primordials;

const { AsyncWrap, Providers } = internalBinding('async_wrap');
Expand Down Expand Up @@ -119,6 +120,74 @@ function randomFill(buf, offset, size, cb) {
_randomBytes(buf, offset, size, wrap);
}

// Largest integer we can read from a buffer.
// e.g.: Buffer.from("ff".repeat(6), "hex").readUIntBE(0, 6);
const RAND_MAX = 281474976710655;
tniessen marked this conversation as resolved.
Show resolved Hide resolved
olalonde marked this conversation as resolved.
Show resolved Hide resolved

function randomInt(min, max, cb) {
addaleax marked this conversation as resolved.
Show resolved Hide resolved
jasnell marked this conversation as resolved.
Show resolved Hide resolved
// randomInt(max, cb)
// randomInt(max)
if (typeof max === 'function' || typeof max === 'undefined') {
cb = max;
max = min;
min = 0;
}
const isSync = typeof cb === 'undefined';
if (!isSync && typeof cb !== 'function') {
throw new ERR_INVALID_CALLBACK(cb);
}
if (!NumberIsInteger(min)) {
throw new ERR_INVALID_ARG_TYPE('min', 'integer', min);
}
if (!NumberIsInteger(max)) {
throw new ERR_INVALID_ARG_TYPE('max', 'integer', max);
}
addaleax marked this conversation as resolved.
Show resolved Hide resolved
if (!(max > min)) {
throw new ERR_OUT_OF_RANGE('max', `> ${min}`, max);
olalonde marked this conversation as resolved.
Show resolved Hide resolved
}

// First we generate a random int between [0..range)
const range = max - min;

if (!(range <= RAND_MAX)) {
addaleax marked this conversation as resolved.
Show resolved Hide resolved
throw new ERR_OUT_OF_RANGE('max - min', `<= ${RAND_MAX}`, range);
olalonde marked this conversation as resolved.
Show resolved Hide resolved
}
jasnell marked this conversation as resolved.
Show resolved Hide resolved

const excess = RAND_MAX % range;
const randLimit = RAND_MAX - excess;

if (isSync) {
// Sync API
while (true) {
jasnell marked this conversation as resolved.
Show resolved Hide resolved
const x = randomBytes(6).readUIntBE(0, 6);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Performance suggestion: allocate a buffer once with new FastBuffer(6) and use randomFillSync() to fill it up, that avoids creating potentially many throwaway buffers.

(I recognize that on average we'll get lucky 50% or more on the first try so consider this a take-it-or-leave-it suggestion, not a must.)

// If x > (maxVal - (maxVal % range)), we will get "modulo bias"
if (x > randLimit) {
// Try again
continue;
}
tniessen marked this conversation as resolved.
Show resolved Hide resolved
const n = (x % range) + min;
return n;
}
} else {
// Async API
const pickAttempt = () => {
randomBytes(6, (err, bytes) => {
if (err) return cb(err);
const x = bytes.readUIntBE(0, 6);
// If x > (maxVal - (maxVal % range)), we will get "modulo bias"
if (x > randLimit) {
// Try again
return pickAttempt();
}
const n = (x % range) + min;
cb(null, n);
});
};

pickAttempt();
}
}

function handleError(ex, buf) {
if (ex) throw ex;
return buf;
Expand All @@ -127,5 +196,6 @@ function handleError(ex, buf) {
module.exports = {
randomBytes,
randomFill,
randomFillSync
randomFillSync,
randomInt
};
144 changes: 144 additions & 0 deletions test/parallel/test-crypto-random.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,3 +315,147 @@ assert.throws(
assert.strictEqual(desc.writable, true);
assert.strictEqual(desc.enumerable, false);
});


{
const randomInts = [];
for (let i = 0; i < 100; i++) {
crypto.randomInt(1, 4, common.mustCall((err, n) => {
assert.ifError(err);
assert.ok(n >= 1);
assert.ok(n < 4);
randomInts.push(n);
if (randomInts.length === 100) {
assert.ok(!randomInts.includes(0));
assert.ok(randomInts.includes(1));
assert.ok(randomInts.includes(2));
assert.ok(randomInts.includes(3));
olalonde marked this conversation as resolved.
Show resolved Hide resolved
assert.ok(!randomInts.includes(4));
}
}));
addaleax marked this conversation as resolved.
Show resolved Hide resolved
}
}
{
const randomInts = [];
for (let i = 0; i < 100; i++) {
crypto.randomInt(-10, -7, common.mustCall((err, n) => {
assert.ifError(err);
assert.ok(n >= -10);
assert.ok(n < -7);
randomInts.push(n);
if (randomInts.length === 100) {
assert.ok(!randomInts.includes(-11));
assert.ok(randomInts.includes(-10));
assert.ok(randomInts.includes(-9));
assert.ok(randomInts.includes(-8));
assert.ok(!randomInts.includes(-7));
}
}));
}
}
{
const randomInts = [];
for (let i = 0; i < 100; i++) {
crypto.randomInt(3, common.mustCall((err, n) => {
assert.ifError(err);
assert.ok(n >= 0);
assert.ok(n < 3);
randomInts.push(n);
if (randomInts.length === 100) {
assert.ok(randomInts.includes(0));
assert.ok(randomInts.includes(1));
assert.ok(randomInts.includes(2));
}
}));
}
}
{
// Synchronous API
const randomInts = [];
for (let i = 0; i < 100; i++) {
const n = crypto.randomInt(3);
assert.ok(n >= 0);
assert.ok(n < 3);
randomInts.push(n);
}

assert.ok(randomInts.includes(0));
assert.ok(randomInts.includes(1));
assert.ok(randomInts.includes(2));
}
{
// Synchronous API with min
const randomInts = [];
for (let i = 0; i < 100; i++) {
const n = crypto.randomInt(3, 6);
assert.ok(n >= 3);
assert.ok(n < 6);
randomInts.push(n);
}

assert.ok(randomInts.includes(3));
assert.ok(randomInts.includes(4));
assert.ok(randomInts.includes(5));
}
{

['10', true, NaN, null, {}, []].forEach((i) => {
assert.throws(() => crypto.randomInt(i, 100, common.mustNotCall()), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "min" argument must be integer.' +
`${common.invalidArgTypeHelper(i)}`,
});

assert.throws(() => crypto.randomInt(0, i, common.mustNotCall()), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "max" argument must be integer.' +
`${common.invalidArgTypeHelper(i)}`,
});

assert.throws(() => crypto.randomInt(i, common.mustNotCall()), {
code: 'ERR_INVALID_ARG_TYPE',
name: 'TypeError',
message: 'The "max" argument must be integer.' +
`${common.invalidArgTypeHelper(i)}`,
});
});

assert.throws(() => crypto.randomInt(0, 0, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max" is out of range. It must be > 0. Received 0'
});

assert.throws(() => crypto.randomInt(0, -1, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max" is out of range. It must be > 0. Received -1'
});

assert.throws(() => crypto.randomInt(0, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max" is out of range. It must be > 0. Received 0'
});

assert.throws(() => crypto.randomInt(2 ** 48, common.mustNotCall()), {
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError',
message: 'The value of "max - min" is out of range. ' +
`It must be <= ${(2 ** 48) - 1}. ` +
'Received 281_474_976_710_656'
});

[1, true, NaN, null, {}, []].forEach((i) => {
assert.throws(
() => crypto.randomInt(1, 2, i),
{
code: 'ERR_INVALID_CALLBACK',
name: 'TypeError',
message: `Callback must be a function. Received ${inspect(i)}`
}
);
});
}