From 58bd408907b66bb3abd8dca2b7171de7c9bf71dd 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] 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       | 66 ++++++++++++++++++--
 2 files changed, 64 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..0809ad74d 100644
--- a/spot-client/src/common/utils/random.js
+++ b/spot-client/src/common/utils/random.js
@@ -1,3 +1,50 @@
+class RandomPool {
+    SIZE = 256;
+
+    constructor() {
+        this._rArray = new Uint8Array(this.SIZE);
+        this._idx = this.SIZE; // Fill the pool on first use.
+    }
+
+    getValue() {
+        if (this._idx > this.SIZE - 1) {
+            // Fill the pool.
+            globalThis.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);
+
+    while (num--) {
+        // Make sure the random value is in our alphabet's range.
+        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 +52,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);
 }