Skip to content

Commit 9da8f60

Browse files
authored
Fix pool pollution, infinite loop (#508)
When nanoid is called with a fractional value, there were a number of undesirable effects: - in browser and non-secure, the code infinite loops on `while (size--)` - in node, the value of poolOffset becomes fractional, causing calls to nanoid to return zeroes until the pool is next filled: when `i` is initialized to `poolOffset`, `pool[i] & 63` -> `undefined & 63` -> `0` - if the first call in node is a fractional argument, the initial buffer allocation fails with an error I chose `|0` to cast to a signed integer primarily because that has a slightly better outcome in the third case above: if the first call is negative (e.g. `nanoid(-1)`) then Node will throw an error for an invalid Buffer size, rather than attempting to allocate a buffer of size `2**32-1`. It's also more compact than `>>>0`, which would be necessary to cast to an unsigned integer. I don't _think_ there is a use case for generating ids longer than `2**31-1` :)
1 parent 009b5f4 commit 9da8f60

File tree

4 files changed

+15
-8
lines changed

4 files changed

+15
-8
lines changed

index.browser.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -47,11 +47,11 @@ export let customRandom = (alphabet, defaultSize, getRandom) => {
4747
}
4848

4949
export let customAlphabet = (alphabet, size = 21) =>
50-
customRandom(alphabet, size, random)
50+
customRandom(alphabet, size | 0, random)
5151

5252
export let nanoid = (size = 21) => {
5353
let id = ''
54-
let bytes = crypto.getRandomValues(new Uint8Array(size))
54+
let bytes = crypto.getRandomValues(new Uint8Array((size |= 0)))
5555
while (size--) {
5656
// Using the bitwise AND operator to "cap" the value of
5757
// the random byte from 255 to 63, in that way we can make sure

index.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ function fillPool(bytes) {
2525
}
2626

2727
export function random(bytes) {
28-
// `-=` convert `bytes` to number to prevent `valueOf` abusing
29-
fillPool((bytes -= 0))
28+
// `|=` convert `bytes` to number to prevent `valueOf` abusing and pool pollution
29+
fillPool((bytes |= 0))
3030
return pool.subarray(poolOffset - bytes, poolOffset)
3131
}
3232

@@ -70,8 +70,8 @@ export function customAlphabet(alphabet, size = 21) {
7070
}
7171

7272
export function nanoid(size = 21) {
73-
// `-=` convert `size` to number to prevent `valueOf` abusing
74-
fillPool((size -= 0))
73+
// `|=` convert `size` to number to prevent `valueOf` abusing and pool pollution
74+
fillPool((size |= 0))
7575
let id = ''
7676
// We are reading directly from the random pool to avoid creating new array
7777
for (let i = poolOffset - size; i < poolOffset; i++) {

non-secure/index.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export let customAlphabet = (alphabet, defaultSize = 21) => {
1111
return (size = defaultSize) => {
1212
let id = ''
1313
// A compact alternative for `for (var i = 0; i < step; i++)`.
14-
let i = size
14+
let i = size | 0
1515
while (i--) {
1616
// `| 0` is more compact and faster than `Math.floor()`.
1717
id += alphabet[(Math.random() * alphabet.length) | 0]
@@ -23,7 +23,7 @@ export let customAlphabet = (alphabet, defaultSize = 21) => {
2323
export let nanoid = (size = 21) => {
2424
let id = ''
2525
// A compact alternative for `for (var i = 0; i < step; i++)`.
26-
let i = size
26+
let i = size | 0
2727
while (i--) {
2828
// `| 0` is more compact and faster than `Math.floor()`.
2929
id += urlAlphabet[(Math.random() * 64) | 0]

test/index.test.js

+7
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,13 @@ for (let type of ['node', 'browser']) {
5757
}
5858
})
5959

60+
test(`avoids pool pollution, infinite loop`, () => {
61+
nanoid(2.1)
62+
const second = nanoid()
63+
const third = nanoid()
64+
notEqual(second, third)
65+
})
66+
6067
test(`has flat distribution`, () => {
6168
let COUNT = 100 * 1000
6269
let LENGTH = nanoid().length

0 commit comments

Comments
 (0)