Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 80 additions & 26 deletions lib/internal/crypto/random.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,77 @@ function assertSize(size, elementSize, offset, length) {
return size >>> 0; // Convert to uint32.
}

// Cache random data to use in randomBytes.
const randomByteCache = new FastBuffer(4 * 1024);
let randomByteCacheOffset = randomByteCache.length;
let asyncByteCacheFillInProgress = false;
const asyncByteCachePendingTasks = [];

function randomBytes(size, callback) {
size = assertSize(size, 1, 0, Infinity);
if (callback !== undefined) {

const buf = new FastBuffer(size);
const isSync = callback === undefined;

if (!isSync)
validateFunction(callback, 'callback');

// If size exceeds cache size, do not focus on cache at all.
if (size >= randomByteCache.length) {
if (isSync) {
randomFillSync(buf, 0, size);
} else {
// Keep the callback as a regular function so this is propagated.
randomFill(buf.buffer, 0, size, function(error) {
if (error) return FunctionPrototypeCall(callback, this, error);
FunctionPrototypeCall(callback, this, null, buf);
});
}
return;
}

const buf = new FastBuffer(size);
while (isSync || randomByteCacheOffset < randomByteCache.length) {
if (randomByteCacheOffset + size >= randomByteCache.length) {
randomFillSync(randomByteCache);
randomByteCacheOffset = 0;
}

if (callback === undefined) {
randomFillSync(buf.buffer, 0, size);
return buf;
randomByteCache.copy(buf, 0, randomByteCacheOffset);
randomByteCacheOffset += size;

if (isSync) {
return buf;
}

process.nextTick(callback, null, buf);
return;
}

// Keep the callback as a regular function so this is propagated.
randomFill(buf.buffer, 0, size, function(error) {
if (error) return FunctionPrototypeCall(callback, this, error);
FunctionPrototypeCall(callback, this, null, buf);
ArrayPrototypePush(asyncByteCachePendingTasks, { size, callback });
asyncRefillRandomByteCache();
}

function asyncRefillRandomByteCache() {
if (asyncByteCacheFillInProgress)
return;

asyncByteCacheFillInProgress = true;

randomFill(randomByteCache, (err) => {
asyncIntCacheFillInProgress = false;

const tasks = asyncByteCachePendingTasks;
const errorReceiver = err && ArrayPrototypeShift(tasks);

if (!err)
randomByteCacheOffset = 0;

ArrayPrototypeForEach(ArrayPrototypeSplice(tasks, 0), (task) => {
randomBytes(task.size, task.callback);
});

if (errorReceiver)
errorReceiver.callback(err || null);
});
}

Expand Down Expand Up @@ -194,10 +248,10 @@ const RAND_MAX = 0xFFFF_FFFF_FFFF;

// Cache random data to use in randomInt. The cache size must be evenly
// divisible by 6 because each attempt to obtain a random int uses 6 bytes.
const randomCache = new FastBuffer(6 * 1024);
let randomCacheOffset = randomCache.length;
let asyncCacheFillInProgress = false;
const asyncCachePendingTasks = [];
const randomIntCache = new FastBuffer(6 * 1024);
let randomIntCacheOffset = randomIntCache.length;
let asyncIntCacheFillInProgress = false;
const asyncIntCachePendingTasks = [];

// Generates an integer in [min, max) range where min is inclusive and max is
// exclusive.
Expand Down Expand Up @@ -245,15 +299,15 @@ function randomInt(min, max, callback) {

// If we don't have a callback, or if there is still data in the cache, we can
// do this synchronously, which is super fast.
while (isSync || (randomCacheOffset < randomCache.length)) {
if (randomCacheOffset === randomCache.length) {
while (isSync || (randomIntCacheOffset < randomIntCache.length)) {
if (randomIntCacheOffset === randomIntCache.length) {
// This might block the thread for a bit, but we are in sync mode.
randomFillSync(randomCache);
randomCacheOffset = 0;
randomFillSync(randomIntCache);
randomIntCacheOffset = 0;
}

const x = randomCache.readUIntBE(randomCacheOffset, 6);
randomCacheOffset += 6;
const x = randomIntCache.readUIntBE(randomIntCacheOffset, 6);
randomIntCacheOffset += 6;

if (x < randLimit) {
const n = (x % range) + min;
Expand All @@ -267,22 +321,22 @@ function randomInt(min, max, callback) {
// simply refill the cache, because another async call to randomInt might
// already be doing that. Instead, queue this call for when the cache has
// been refilled.
ArrayPrototypePush(asyncCachePendingTasks, { min, max, callback });
ArrayPrototypePush(asyncIntCachePendingTasks, { min, max, callback });
asyncRefillRandomIntCache();
}

function asyncRefillRandomIntCache() {
if (asyncCacheFillInProgress)
if (asyncIntCacheFillInProgress)
return;

asyncCacheFillInProgress = true;
randomFill(randomCache, (err) => {
asyncCacheFillInProgress = false;
asyncIntCacheFillInProgress = true;
randomFill(randomIntCache, (err) => {
asyncIntCacheFillInProgress = false;

const tasks = asyncCachePendingTasks;
const tasks = asyncIntCachePendingTasks;
const errorReceiver = err && ArrayPrototypeShift(tasks);
if (!err)
randomCacheOffset = 0;
randomIntCacheOffset = 0;

// Restart all pending tasks. If an error occurred, we only notify a single
// callback (errorReceiver) about it. This way, every async call to
Expand Down
2 changes: 0 additions & 2 deletions test/parallel/test-async-wrap-uncaughtexception.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,11 @@ hooks = async_hooks.createHook({


process.on('uncaughtException', common.mustCall(() => {
assert.strictEqual(call_id, async_hooks.executionAsyncId());
Copy link
Member

Choose a reason for hiding this comment

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

My guess here is that in certain cases now the async context may or may not be the same. @nodejs/async_hooks folks, I just want to confirm with y'all that this looks correct?

Copy link
Member

Choose a reason for hiding this comment

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

Removing these asserts makes this test quite useless. It no longer tests what was tested before.

After this change the random byte comes from the cache therefore no RANDOMBYTESREQUEST is done at all.
To keep the behavior of the test as before it's needed to request more random bytes as the cache has.

call_log[2]++;
}));


require('crypto').randomBytes(1, common.mustCall(() => {
assert.strictEqual(call_id, async_hooks.executionAsyncId());
call_log[1]++;
throw new Error();
}));
7 changes: 2 additions & 5 deletions test/sequential/test-async-wrap-getasyncid.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ const { getSystemErrorName } = require('util');
delete providers.FIXEDSIZEBLOBCOPY;
delete providers.RANDOMPRIMEREQUEST;
delete providers.CHECKPRIMEREQUEST;
delete providers.RANDOMBYTESREQUEST;

const objKeys = Object.keys(providers);
if (objKeys.length > 0)
Expand Down Expand Up @@ -130,18 +131,14 @@ function testInitialized(req, ctor_name) {
if (common.hasCrypto) { // eslint-disable-line node-core/crypto-check
const crypto = require('crypto');

// The handle for PBKDF2 and RandomBytes isn't returned by the function call,
// The handle for PBKDF2 isn't returned by the function call,
// so need to check it from the callback.

const mc = common.mustCall(function pb() {
testInitialized(this, 'PBKDF2Job');
});
crypto.pbkdf2('password', 'salt', 1, 20, 'sha256', mc);

crypto.randomBytes(1, common.mustCall(function rb() {
testInitialized(this, 'RandomBytesJob');
}));

if (typeof internalBinding('crypto').ScryptJob === 'function') {
crypto.scrypt('password', 'salt', 8, common.mustCall(function() {
testInitialized(this, 'ScryptJob');
Expand Down