From ecde2494eff98b32958cfb22e4bd3802f4daa9e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= <s@saghul.net> Date: Wed, 14 Aug 2024 10:27:55 +0200 Subject: [PATCH 1/2] fix(client) use a UUIDv4 as the request ID No need for a home-grown 8 16 char random string. --- spot-client/src/common/remote-control/P2PSignalingClient.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/spot-client/src/common/remote-control/P2PSignalingClient.js b/spot-client/src/common/remote-control/P2PSignalingClient.js index 66b4a0a9e..99b6d8f13 100644 --- a/spot-client/src/common/remote-control/P2PSignalingClient.js +++ b/spot-client/src/common/remote-control/P2PSignalingClient.js @@ -1,7 +1,5 @@ import { logger } from 'common/logger'; -import { generate8Characters } from '../utils'; - import P2PSignalingBase from './P2PSignalingBase'; @@ -134,7 +132,7 @@ export default class P2PSignalingClient extends P2PSignalingBase { throw new Error('No PeerConnection'); } - const requestId = `${generate8Characters()}-${generate8Characters}`; + const requestId = window.crypto.randomUUID(); // FIXME is there any better way to do this? // The 'reject' and 'resolve' methods are set when the promise is created. From 1a33bb7941a89a0d362b238ae69300c7cff92fbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= <s@saghul.net> Date: Wed, 14 Aug 2024 10:29:24 +0200 Subject: [PATCH 2/2] feat(client,utils) refactor random char generation - Generate actually unique GUIDs - Use a better random character generator - Use a better alphabet for join codes which eliminates ambiguity --- spot-client/src/common/utils/cryptography.js | 40 +--------- spot-client/src/common/utils/random.js | 83 +++++++++++++++++++- 2 files changed, 81 insertions(+), 42 deletions(-) diff --git a/spot-client/src/common/utils/cryptography.js b/spot-client/src/common/utils/cryptography.js index 546284826..ffea03440 100644 --- a/spot-client/src/common/utils/cryptography.js +++ b/spot-client/src/common/utils/cryptography.js @@ -1,45 +1,9 @@ /** - * Generates a random 8 character long string. - * - * @returns {string} - */ -export function generate8Characters() { - const buf = new Uint16Array(2); - - window.crypto.getRandomValues(buf); - - return `${s4(buf[0])}${s4(buf[1])}`; -} - -/** - * Generate a likely-to-be-unique guid. + * Generate a GUID. * * @private * @returns {string} The generated string. */ export function generateGuid() { - const buf = new Uint16Array(8); - - window.crypto.getRandomValues(buf); - - return `${s4(buf[0])}${s4(buf[1])}-${s4(buf[2])}-${s4(buf[3])}-${ - s4(buf[4])}-${s4(buf[5])}${s4(buf[6])}${s4(buf[7])}`; -} - -/** - * Converts the passed in number to a string and ensure it is at least 4 - * characters in length, prepending 0's as needed. - * - * @param {number} num - The number to pad and convert to a string. - * @private - * @returns {string} - The number converted to a string. - */ -function s4(num) { - let ret = num.toString(16); - - while (ret.length < 4) { - ret = `0${ret}`; - } - - return ret; + return window.crypto.randomUUID(); } diff --git a/spot-client/src/common/utils/random.js b/spot-client/src/common/utils/random.js index acf66490e..9818d486f 100644 --- a/spot-client/src/common/utils/random.js +++ b/spot-client/src/common/utils/random.js @@ -1,3 +1,67 @@ +/** + * Helper class to generate random values one by one, using an auxiliary + * pool to avoid calling getRandomValues every time. + */ +class RandomPool { + /** + * Default size for the pool. + */ + SIZE = 256; + + /** + * Build a new pool. + */ + constructor() { + this._rArray = new Uint8Array(this.SIZE); + this._idx = this.SIZE; // Fill the pool on first use. + } + + /** + * Gets a random number in the Uint8 range. + * + * @returns {number} + */ + getValue() { + if (this._idx > this.SIZE - 1) { + // Fill the pool. + window.crypto.getRandomValues(this._rArray); + this._idx = 0; + } + + return this._rArray[this._idx++]; + } +} + +const pool = new RandomPool(); + +/** + * Choose `num` elements from `seq`, randomly. + * + * @param {string} seq - Sequence of characters, the alphabet. + * @param {number} num - The amount of characters from the alphabet we want. + * @returns {string} + */ +function randomChoice(seq, num) { + const x = seq.length - 1; + const r = new Array(num); + let len = num; + + while (len--) { + // Make sure the random value is in our alphabet's range. + // eslint-disable-next-line no-bitwise + const idx = pool.getValue() & x; + + r.push(seq[idx]); + } + + return r.join(''); +} + +/** + * This alphabet removes all potential ambiguous symbols, so it's well suited for a code. + */ +const BASE32 = 'ABCDEFGHIJKLMNOPQRSTUVWXYZ234567'; + /** * A method to generate a random string, intended to be used to create a random join code. * @@ -5,8 +69,19 @@ * @returns {string} */ export function generateRandomString(length) { - // XXX the method may not always give desired length above 9 - return Math.random() - .toString(36) - .slice(2, 2 + length); + return randomChoice(BASE32, length); +} + +/** + * This alphabet is similar to BASE32 above, but includes lowercase characters too. + */ +const BASE58 = '123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz'; + +/** + * Generates a random 8 character long string. + * + * @returns {string} + */ +export function generate8Characters() { + return randomChoice(BASE58, 8); }