Skip to content

Commit

Permalink
Fix pool pollution, infinite loop
Browse files Browse the repository at this point in the history
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` :)
  • Loading branch information
myndzi committed Nov 25, 2024
1 parent 009b5f4 commit 6d6767d
Show file tree
Hide file tree
Showing 4 changed files with 15 additions and 8 deletions.
4 changes: 2 additions & 2 deletions index.browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ export let customRandom = (alphabet, defaultSize, getRandom) => {
}

export let customAlphabet = (alphabet, size = 21) =>
customRandom(alphabet, size, random)
customRandom(alphabet, size | 0, random)

export let nanoid = (size = 21) => {
let id = ''
let bytes = crypto.getRandomValues(new Uint8Array(size))
let bytes = crypto.getRandomValues(new Uint8Array((size |= 0)))
while (size--) {
// Using the bitwise AND operator to "cap" the value of
// the random byte from 255 to 63, in that way we can make sure
Expand Down
8 changes: 4 additions & 4 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ function fillPool(bytes) {
}

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

Expand Down Expand Up @@ -70,8 +70,8 @@ export function customAlphabet(alphabet, size = 21) {
}

export function nanoid(size = 21) {
// `-=` convert `size` to number to prevent `valueOf` abusing
fillPool((size -= 0))
// `|=` convert `size` to number to prevent `valueOf` abusing and pool pollution
fillPool((size |= 0))
let id = ''
// We are reading directly from the random pool to avoid creating new array
for (let i = poolOffset - size; i < poolOffset; i++) {
Expand Down
4 changes: 2 additions & 2 deletions non-secure/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export let customAlphabet = (alphabet, defaultSize = 21) => {
return (size = defaultSize) => {
let id = ''
// A compact alternative for `for (var i = 0; i < step; i++)`.
let i = size
let i = size | 0
while (i--) {
// `| 0` is more compact and faster than `Math.floor()`.
id += alphabet[(Math.random() * alphabet.length) | 0]
Expand All @@ -23,7 +23,7 @@ export let customAlphabet = (alphabet, defaultSize = 21) => {
export let nanoid = (size = 21) => {
let id = ''
// A compact alternative for `for (var i = 0; i < step; i++)`.
let i = size
let i = size | 0
while (i--) {
// `| 0` is more compact and faster than `Math.floor()`.
id += urlAlphabet[(Math.random() * 64) | 0]
Expand Down
7 changes: 7 additions & 0 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,13 @@ for (let type of ['node', 'browser']) {
}
})

test(`avoids pool pollution, infinite loop`, () => {
nanoid(2.1)
const second = nanoid()
const third = nanoid()
notEqual(second, third)
})

test(`has flat distribution`, () => {
let COUNT = 100 * 1000
let LENGTH = nanoid().length
Expand Down

0 comments on commit 6d6767d

Please sign in to comment.