Avoid use of Buffer as it does not exist in the Web natively (#4569)

* Avoid use of Buffer as it does not exist in the Web natively Signed-off-by: Michael Telatynski <[email protected]> * Fix tests Signed-off-by: Michael Telatynski <[email protected]> * Use TC39 Uint8Array methods when available Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Iterate Signed-off-by: Michael Telatynski <[email protected]> * Replace usage of Buffer in encodeRecoveryKey Signed-off-by: Michael Telatynski <[email protected]> --------- Signed-off-by: Michael Telatynski <[email protected]>
matrix-org · Dec 4, 2024 · 17df9f8 · 17df9f8
1 parent 1cad6f4
commit 17df9f8
Show file tree

Hide file tree

Showing 15 changed files with 112 additions and 113 deletions.
diff --git a/spec/integ/crypto/verification.spec.ts b/spec/integ/crypto/verification.spec.ts
@@ -473,21 +473,23 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
             expect(request.phase).toEqual(VerificationPhase.Ready);
 
             // we should now have QR data we can display
-            const qrCodeBuffer = (await request.generateQRCode())!;
-            expect(qrCodeBuffer).toBeTruthy();
+            const rawQrCodeBuffer = (await request.generateQRCode())!;
+            expect(rawQrCodeBuffer).toBeTruthy();
+            const qrCodeBuffer = new Uint8Array(rawQrCodeBuffer);
 
+            const textDecoder = new TextDecoder();
             // https://spec.matrix.org/v1.7/client-server-api/#qr-code-format
-            expect(qrCodeBuffer.subarray(0, 6).toString("latin1")).toEqual("MATRIX");
-            expect(qrCodeBuffer.readUint8(6)).toEqual(0x02); // version
-            expect(qrCodeBuffer.readUint8(7)).toEqual(0x02); // mode
-            const txnIdLen = qrCodeBuffer.readUint16BE(8);
-            expect(qrCodeBuffer.subarray(10, 10 + txnIdLen).toString("utf-8")).toEqual(transactionId);
+            expect(textDecoder.decode(qrCodeBuffer.slice(0, 6))).toEqual("MATRIX");
+            expect(qrCodeBuffer[6]).toEqual(0x02); // version
+            expect(qrCodeBuffer[7]).toEqual(0x02); // mode
+            const txnIdLen = (qrCodeBuffer[8] << 8) + qrCodeBuffer[9];
+            expect(textDecoder.decode(qrCodeBuffer.slice(10, 10 + txnIdLen))).toEqual(transactionId);
             // Alice's device's public key comes next, but we have nothing to do with it here.
-            // const aliceDevicePubKey = qrCodeBuffer.subarray(10 + txnIdLen, 32 + 10 + txnIdLen);
-            expect(qrCodeBuffer.subarray(42 + txnIdLen, 32 + 42 + txnIdLen)).toEqual(
-                Buffer.from(MASTER_CROSS_SIGNING_PUBLIC_KEY_BASE64, "base64"),
+            // const aliceDevicePubKey = qrCodeBuffer.slice(10 + txnIdLen, 32 + 10 + txnIdLen);
+            expect(encodeUnpaddedBase64(qrCodeBuffer.slice(42 + txnIdLen, 32 + 42 + txnIdLen))).toEqual(
+                MASTER_CROSS_SIGNING_PUBLIC_KEY_BASE64,
             );
-            const sharedSecret = qrCodeBuffer.subarray(74 + txnIdLen);
+            const sharedSecret = qrCodeBuffer.slice(74 + txnIdLen);
 
             // we should still be "Ready" and have no verifier
             expect(request.phase).toEqual(VerificationPhase.Ready);
@@ -805,7 +807,7 @@ describe.each(Object.entries(CRYPTO_BACKENDS))("verification (%s)", (backend: st
             // we should now have QR data we can display
             const qrCodeBuffer = (await request.generateQRCode())!;
             expect(qrCodeBuffer).toBeTruthy();
-            const sharedSecret = qrCodeBuffer.subarray(74 + transactionId.length);
+            const sharedSecret = qrCodeBuffer.slice(74 + transactionId.length);
 
             // the dummy device "scans" the displayed QR code and acknowledges it with a "m.key.verification.start"
             returnToDeviceMessageFromSync(buildReciprocateStartMessage(transactionId, sharedSecret));
@@ -1627,7 +1629,7 @@ function buildReadyMessage(
 }
 
 /** build an m.key.verification.start to-device message suitable for the m.reciprocate.v1 flow, originating from the dummy device */
-function buildReciprocateStartMessage(transactionId: string, sharedSecret: Uint8Array) {
+function buildReciprocateStartMessage(transactionId: string, sharedSecret: ArrayBuffer) {
     return {
         type: "m.key.verification.start",
         content: {
@@ -1723,7 +1725,7 @@ function buildQRCode(
     key2Base64: string,
     sharedSecret: string,
     mode = 0x02,
-): Uint8Array {
+): Uint8ClampedArray {
     // https://spec.matrix.org/v1.7/client-server-api/#qr-code-format
 
     const qrCodeBuffer = Buffer.alloc(150); // oversize
@@ -1739,5 +1741,5 @@ function buildQRCode(
     idx += qrCodeBuffer.write(sharedSecret, idx);
 
     // truncate to the right length
-    return qrCodeBuffer.subarray(0, idx);
+    return new Uint8ClampedArray(qrCodeBuffer.subarray(0, idx));
 }
diff --git a/spec/unit/base64.spec.ts b/spec/unit/base64.spec.ts
@@ -15,34 +15,10 @@ limitations under the License.
 */
 
 import { TextEncoder, TextDecoder } from "util";
-import NodeBuffer from "node:buffer";
 
 import { decodeBase64, encodeBase64, encodeUnpaddedBase64, encodeUnpaddedBase64Url } from "../../src/base64";
 
-describe.each(["browser", "node"])("Base64 encoding (%s)", (env) => {
-    let origBuffer = Buffer;
-
-    beforeAll(() => {
-        if (env === "browser") {
-            origBuffer = Buffer;
-            // @ts-ignore
-            // eslint-disable-next-line no-global-assign
-            Buffer = undefined;
-
-            globalThis.atob = NodeBuffer.atob;
-            globalThis.btoa = NodeBuffer.btoa;
-        }
-    });
-
-    afterAll(() => {
-        // eslint-disable-next-line no-global-assign
-        Buffer = origBuffer;
-        // @ts-ignore
-        globalThis.atob = undefined;
-        // @ts-ignore
-        globalThis.btoa = undefined;
-    });
-
+describe("Base64 encoding", () => {
     it("Should decode properly encoded data", () => {
         const decoded = new TextDecoder().decode(decodeBase64("ZW5jb2RpbmcgaGVsbG8gd29ybGQ="));
 

diff --git a/spec/unit/matrixrtc/MatrixRTCSession.spec.ts b/spec/unit/matrixrtc/MatrixRTCSession.spec.ts
@@ -38,6 +38,8 @@ const membershipTemplate: CallMembershipData = {
 
 const mockFocus = { type: "mock" };
 
+const textEncoder = new TextEncoder();
+
 describe("MatrixRTCSession", () => {
     let client: MatrixClient;
     let sess: MatrixRTCSession | undefined;
@@ -1345,7 +1347,7 @@ describe("MatrixRTCSession", () => {
                 sess!.reemitEncryptionKeys();
                 expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1);
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("this is the key", "utf-8"),
+                    textEncoder.encode("this is the key"),
                     0,
                     "@bob:example.org:bobsphone",
                 );
@@ -1377,7 +1379,7 @@ describe("MatrixRTCSession", () => {
                 sess!.reemitEncryptionKeys();
                 expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1);
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("this is the key", "utf-8"),
+                    textEncoder.encode("this is the key"),
                     4,
                     "@bob:example.org:bobsphone",
                 );
@@ -1409,7 +1411,7 @@ describe("MatrixRTCSession", () => {
                 sess!.reemitEncryptionKeys();
                 expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1);
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("this is the key", "utf-8"),
+                    textEncoder.encode("this is the key"),
                     0,
                     "@bob:example.org:bobsphone",
                 );
@@ -1436,12 +1438,12 @@ describe("MatrixRTCSession", () => {
                 sess!.reemitEncryptionKeys();
                 expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(2);
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("this is the key", "utf-8"),
+                    textEncoder.encode("this is the key"),
                     0,
                     "@bob:example.org:bobsphone",
                 );
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("this is the key", "utf-8"),
+                    textEncoder.encode("this is the key"),
                     4,
                     "@bob:example.org:bobsphone",
                 );
@@ -1489,7 +1491,7 @@ describe("MatrixRTCSession", () => {
                 sess!.reemitEncryptionKeys();
                 expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1);
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("newer key", "utf-8"),
+                    textEncoder.encode("newer key"),
                     0,
                     "@bob:example.org:bobsphone",
                 );
@@ -1537,7 +1539,7 @@ describe("MatrixRTCSession", () => {
                 sess!.reemitEncryptionKeys();
                 expect(encryptionKeyChangedListener).toHaveBeenCalledTimes(1);
                 expect(encryptionKeyChangedListener).toHaveBeenCalledWith(
-                    Buffer.from("second key", "utf-8"),
+                    textEncoder.encode("second key"),
                     0,
                     "@bob:example.org:bobsphone",
                 );

diff --git a/src/@types/global.d.ts b/src/@types/global.d.ts
@@ -67,4 +67,24 @@ declare global {
         // on webkit: we should check if we still need to do this
         webkitGetUserMedia?: DummyInterfaceWeShouldntBeUsingThis;
     }
+
+    export interface Uint8ArrayToBase64Options {
+        alphabet?: "base64" | "base64url";
+        omitPadding?: boolean;
+    }
+
+    interface Uint8Array {
+        // https://tc39.es/proposal-arraybuffer-base64/spec/#sec-uint8array.prototype.tobase64
+        toBase64?(options?: Uint8ArrayToBase64Options): string;
+    }
+
+    export interface Uint8ArrayFromBase64Options {
+        alphabet?: "base64"; // Our fallback code only handles base64.
+        lastChunkHandling?: "loose"; // Our fallback code doesn't support other handling at this time.
+    }
+
+    interface Uint8ArrayConstructor {
+        // https://tc39.es/proposal-arraybuffer-base64/spec/#sec-uint8array.frombase64
+        fromBase64?(base64: string, options?: Uint8ArrayFromBase64Options): Uint8Array;
+    }
 }
diff --git a/src/base64.ts b/src/base64.ts
@@ -18,48 +18,61 @@ limitations under the License.
  * Base64 encoding and decoding utilities
  */
 
+function toBase64(uint8Array: Uint8Array, options: Uint8ArrayToBase64Options): string {
+    if (typeof uint8Array.toBase64 === "function") {
+        // Currently this is only supported in Firefox,
+        // but we match the options in the hope in the future we can rely on it for all environments.
+        // https://tc39.es/proposal-arraybuffer-base64/spec/#sec-uint8array.prototype.tobase64
+        return uint8Array.toBase64(options);
+    }
+
+    let base64 = btoa(uint8Array.reduce((acc, current) => acc + String.fromCharCode(current), ""));
+    if (options.omitPadding) {
+        base64 = base64.replace(/={1,2}$/, "");
+    }
+    if (options.alphabet === "base64url") {
+        base64 = base64.replace(/\+/g, "-").replace(/\//g, "_");
+    }
+
+    return base64;
+}
+
 /**
  * Encode a typed array of uint8 as base64.
  * @param uint8Array - The data to encode.
  * @returns The base64.
  */
-export function encodeBase64(uint8Array: ArrayBuffer | Uint8Array): string {
-    // A brief note on the state of base64 encoding in Javascript.
-    // As of 2023, there is still no common native impl between both browsers and
-    // node. Older Webpack provides an impl for Buffer and there is a polyfill class
-    // for it. There are also plenty of pure js impls, eg. base64-js which has 2336
-    // dependents at current count. Using this would probably be fine although it's
-    // a little under-docced and run by an individual. The node impl works fine,
-    // the browser impl works but predates Uint8Array and so only uses strings.
-    // Right now, switching between native (or polyfilled) impls like this feels
-    // like the least bad option, but... *shrugs*.
-    if (typeof Buffer === "function") {
-        return Buffer.from(uint8Array).toString("base64");
-    } else if (typeof btoa === "function" && uint8Array instanceof Uint8Array) {
-        // ArrayBuffer is a node concept so the param should always be a Uint8Array on
-        // the browser. We need to check because ArrayBuffers don't have reduce.
-        return btoa(uint8Array.reduce((acc, current) => acc + String.fromCharCode(current), ""));
-    } else {
-        throw new Error("No base64 impl found!");
-    }
+export function encodeBase64(uint8Array: Uint8Array): string {
+    return toBase64(uint8Array, { alphabet: "base64", omitPadding: false });
 }
 
 /**
  * Encode a typed array of uint8 as unpadded base64.
  * @param uint8Array - The data to encode.
  * @returns The unpadded base64.
  */
-export function encodeUnpaddedBase64(uint8Array: ArrayBuffer | Uint8Array): string {
-    return encodeBase64(uint8Array).replace(/={1,2}$/, "");
+export function encodeUnpaddedBase64(uint8Array: Uint8Array): string {
+    return toBase64(uint8Array, { alphabet: "base64", omitPadding: true });
 }
 
 /**
  * Encode a typed array of uint8 as unpadded base64 using the URL-safe encoding.
  * @param uint8Array - The data to encode.
  * @returns The unpadded base64.
  */
-export function encodeUnpaddedBase64Url(uint8Array: ArrayBuffer | Uint8Array): string {
-    return encodeUnpaddedBase64(uint8Array).replace(/\+/g, "-").replace(/\//g, "_");
+export function encodeUnpaddedBase64Url(uint8Array: Uint8Array): string {
+    return toBase64(uint8Array, { alphabet: "base64url", omitPadding: true });
+}
+
+function fromBase64(base64: string, options: Uint8ArrayFromBase64Options): Uint8Array {
+    if (typeof Uint8Array.fromBase64 === "function") {
+        // Currently this is only supported in Firefox,
+        // but we match the options in the hope in the future we can rely on it for all environments.
+        // https://tc39.es/proposal-arraybuffer-base64/spec/#sec-uint8array.frombase64
+        return Uint8Array.fromBase64(base64, options);
+    }
+
+    return Uint8Array.from(atob(base64), (c) => c.charCodeAt(0));
 }
 
 /**
@@ -68,21 +81,6 @@ export function encodeUnpaddedBase64Url(uint8Array: ArrayBuffer | Uint8Array): s
  * @returns The decoded data.
  */
 export function decodeBase64(base64: string): Uint8Array {
-    // See encodeBase64 for a short treatise on base64 en/decoding in JS
-    if (typeof Buffer === "function") {
-        return Buffer.from(base64, "base64");
-    } else if (typeof atob === "function") {
-        const itFunc = function* (): Generator<number> {
-            const decoded = atob(
-                // built-in atob doesn't support base64url: convert so we support either
-                base64.replace(/-/g, "+").replace(/_/g, "/"),
-            );
-            for (let i = 0; i < decoded.length; ++i) {
-                yield decoded.charCodeAt(i);
-            }
-        };
-        return Uint8Array.from(itFunc());
-    } else {
-        throw new Error("No base64 impl found!");
-    }
+    // The function requires us to select an alphabet, but we don't know if base64url was used so we convert.
+    return fromBase64(base64.replace(/-/g, "+").replace(/_/g, "/"), { alphabet: "base64", lastChunkHandling: "loose" });
 }
diff --git a/src/client.ts b/src/client.ts
@@ -3899,28 +3899,28 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
     }
 
     private async restoreKeyBackup(
-        privKey: ArrayLike<number>,
+        privKey: Uint8Array,
         targetRoomId: undefined,
         targetSessionId: undefined,
         backupInfo: IKeyBackupInfo,
         opts?: IKeyBackupRestoreOpts,
     ): Promise<IKeyBackupRestoreResult>;
     private async restoreKeyBackup(
-        privKey: ArrayLike<number>,
+        privKey: Uint8Array,
         targetRoomId: string,
         targetSessionId: undefined,
         backupInfo: IKeyBackupInfo,
         opts?: IKeyBackupRestoreOpts,
     ): Promise<IKeyBackupRestoreResult>;
     private async restoreKeyBackup(
-        privKey: ArrayLike<number>,
+        privKey: Uint8Array,
         targetRoomId: string,
         targetSessionId: string,
         backupInfo: IKeyBackupInfo,
         opts?: IKeyBackupRestoreOpts,
     ): Promise<IKeyBackupRestoreResult>;
     private async restoreKeyBackup(
-        privKey: ArrayLike<number>,
+        privKey: Uint8Array,
         targetRoomId: string | undefined,
         targetSessionId: string | undefined,
         backupInfo: IKeyBackupInfo,

diff --git a/src/common-crypto/CryptoBackend.ts b/src/common-crypto/CryptoBackend.ts
@@ -108,7 +108,7 @@ export interface CryptoBackend extends SyncCryptoCallbacks, CryptoApi {
      * @param backupInfo - The backup information
      * @param privKey - The private decryption key.
      */
-    getBackupDecryptor(backupInfo: KeyBackupInfo, privKey: ArrayLike<number>): Promise<BackupDecryptor>;
+    getBackupDecryptor(backupInfo: KeyBackupInfo, privKey: Uint8Array): Promise<BackupDecryptor>;
 
     /**
      * Import a list of room keys restored from backup

diff --git a/src/crypto-api/recovery-key.ts b/src/crypto-api/recovery-key.ts
@@ -26,7 +26,7 @@ const KEY_SIZE = 32;
  * @param key
  */
 export function encodeRecoveryKey(key: ArrayLike<number>): string | undefined {
-    const buf = Buffer.alloc(OLM_RECOVERY_KEY_PREFIX.length + key.length + 1);
+    const buf = new Uint8Array(OLM_RECOVERY_KEY_PREFIX.length + key.length + 1);
     buf.set(OLM_RECOVERY_KEY_PREFIX, 0);
     buf.set(key, OLM_RECOVERY_KEY_PREFIX.length);
 

diff --git a/src/crypto-api/verification.ts b/src/crypto-api/verification.ts
@@ -155,7 +155,7 @@ export interface VerificationRequest
      * @param qrCodeData - the decoded QR code.
      * @returns A verifier; call `.verify()` on it to wait for the other side to complete the verification flow.
      */
-    scanQRCode(qrCodeData: Uint8Array): Promise<Verifier>;
+    scanQRCode(qrCodeData: Uint8ClampedArray): Promise<Verifier>;
 
     /**
      * The verifier which is doing the actual verification, once the method has been established.
@@ -170,15 +170,15 @@ export interface VerificationRequest
      *
      * @deprecated Not supported in Rust Crypto. Use {@link VerificationRequest#generateQRCode} instead.
      */
-    getQRCodeBytes(): Buffer | undefined;
+    getQRCodeBytes(): Uint8ClampedArray | undefined;
 
     /**
      * Generate the data for a QR code allowing the other device to verify this one, if it supports it.
      *
      * Only returns data once `phase` is {@link VerificationPhase.Ready} and the other party can scan a QR code;
      * otherwise returns `undefined`.
      */
-    generateQRCode(): Promise<Buffer | undefined>;
+    generateQRCode(): Promise<Uint8ClampedArray | undefined>;
 
     /**
      * If this request has been cancelled, the cancellation code (e.g `m.user`) which is responsible for cancelling

diff --git a/src/crypto/index.ts b/src/crypto/index.ts
@@ -1846,7 +1846,7 @@ export class Crypto extends TypedEventEmitter<CryptoEvent, CryptoEventHandlerMap
     /**
      * Implementation of {@link CryptoBackend#getBackupDecryptor}.
      */
-    public async getBackupDecryptor(backupInfo: KeyBackupInfo, privKey: ArrayLike<number>): Promise<BackupDecryptor> {
+    public async getBackupDecryptor(backupInfo: KeyBackupInfo, privKey: Uint8Array): Promise<BackupDecryptor> {
         if (!(privKey instanceof Uint8Array)) {
             throw new Error(`getBackupDecryptor expects Uint8Array`);
         }