Merge pull request #1961 from keepassxreboot/fix/refactor_messaging

Refactor extension messaging

keepassxc-browser/background/client.js

@@ -51,79 +51,90 @@ const kpErrors = {
 const messageBuffer = {
     buffer: [],
 
-    addMessage(msg) {
-        if (!this.buffer.includes(msg)) {
-            this.buffer.push(msg);
-        }
+    addMessage(message) {
+        this.buffer.push(message);
     },
 
-    matchAndRemove(msg) {
-        for (let i = 0; i < this.buffer.length; ++i) {
-            if (msg.nonce && msg.nonce === keepassClient.incrementedNonce(this.buffer[i].nonce)) {
-                this.buffer.splice(i, 1);
-                return true;
-            }
+    removeMessageFromIndex(index) {
+        if (index >= 0 && index < this.buffer.length) {
+            this.buffer.splice(index, 1);
         }
-
-        return false;
     }
 };
 
+// Basic class for a message to be sent. The Promise inside the class will be resolved when
+// the response to the message is received.
+class Message {
+    constructor(request, enableTimeout, timeoutValue) {
+        this.promise = new Promise((resolve, reject) => {
+            this.reject = reject;
+            this.resolve = resolve;
+            this.enableTimeout = enableTimeout;
+
+            const messageTimeout = timeoutValue || keepassClient.messageTimeout;
+
+            // Handle timeout
+            if (enableTimeout) {
+                this.timeout = setTimeout(() => {
+                    const errorMessage = {
+                        action: request.action,
+                        error: kpErrors.getError(kpErrors.TIMEOUT_OR_NOT_CONNECTED),
+                        errorCode: kpErrors.TIMEOUT_OR_NOT_CONNECTED
+                    };
+
+                    keepass.isKeePassXCAvailable = false;
+                    resolve(errorMessage);
+                }, messageTimeout);
+            }
+        });
+    }
+}
+
 //--------------------------------------------------------------------------
 // Messaging
 //--------------------------------------------------------------------------
 
-keepassClient.sendNativeMessage = function(request, enableTimeout = false, timeoutValue) {
-    return new Promise((resolve, reject) => {
-        let timeout;
-        const requestAction = request.action;
-        const ev = keepassClient.nativePort.onMessage;
+keepassClient.sendNativeMessage = async function(request, enableTimeout = false, timeoutValue) {
+    if (!keepassClient.nativePort) {
+        logError('No native messaging port defined.');
+        return;
+    }
 
-        const listener = ((port, action) => {
-            const handler = (msg) => {
-                if (msg && msg?.action === action) {
-                    // If the request has a separate requestID, check if it matches when there's no nonce (an error message)
-                    const isNotificationOrError = !msg.nonce && (request.requestID === msg.requestID || (msg.error && msg.errorCode));
+    const message = new Message(request, enableTimeout, timeoutValue);
+    await navigator.locks.request('messageBuffer', async (lock) => {
+        messageBuffer.addMessage({ request: request, message: message });
+    });
 
-                    // Only resolve a matching response or a notification (without nonce)
-                    if (isNotificationOrError || messageBuffer.matchAndRemove(msg)) {
-                        port.removeListener(handler);
-                        if (enableTimeout) {
-                            clearTimeout(timeout);
-                        }
+    keepassClient.nativePort.postMessage(request);
+    return await message.promise;
+};
 
-                        resolve(msg);
-                        return;
-                    }
+keepassClient.handleNativeMessage = async function(response) {
+    const isError = Boolean(!response.nonce && response.error && response.errorCode);
+
+    // Parse through the message buffer to find the corresponding Promise.
+    await navigator.locks.request('messageBuffer', async (lock) => {
+        for (let i = 0; i < messageBuffer.buffer.length; ++i) {
+            if (!messageBuffer.buffer[i]) {
+                continue;
+            }
+
+            const request = messageBuffer.buffer[i]?.request;
+            const message = messageBuffer.buffer[i]?.message;
+            const errorFound = isError && request?.action === response?.action;
+
+            if ((response.nonce && response.nonce === keepassClient.incrementedNonce(request.nonce)) || errorFound) {
+                if (message.enableTimeout) {
+                    clearTimeout(message.timeout);
                 }
-            };
-            return handler;
-        })(ev, requestAction);
-        ev.addListener(listener);
 
-        const messageTimeout = timeoutValue || keepassClient.messageTimeout;
-
-        // Handle timeouts
-        if (enableTimeout) {
-            timeout = setTimeout(() => {
-                const errorMessage = {
-                    action: requestAction,
-                    error: kpErrors.getError(kpErrors.TIMEOUT_OR_NOT_CONNECTED),
-                    errorCode: kpErrors.TIMEOUT_OR_NOT_CONNECTED
-                };
-                keepass.isKeePassXCAvailable = false;
-                ev.removeListener(listener.handler);
-                resolve(errorMessage);
-            }, messageTimeout);
+                message.resolve(response);
+                messageBuffer.removeMessageFromIndex(i);
+                return;
+            }
         }
 
-        // Store the request to the buffer
-        messageBuffer.addMessage(request);
-
-        // Send the request
-        if (keepassClient.nativePort) {
-            keepassClient.nativePort.postMessage(request);
-        }
+        debugLogMessage('Corresponding request not found in the message buffer for response: ', response);
     });
 };
 
@@ -341,5 +352,9 @@ keepassClient.onNativeMessage = function(response) {
     // Handle database lock/unlock status
     if (response.action === kpActions.DATABASE_LOCKED || response.action === kpActions.DATABASE_UNLOCKED) {
         keepass.updateDatabase();
+        return;
     }
+
+    // Generic response handling
+    keepassClient.handleNativeMessage(response);
 };

keepassxc-browser/background/keepass.js

@@ -168,18 +168,18 @@ keepass.retrieveCredentials = async function(tab, args = []) {
 
 keepass.generatePassword = async function(tab) {
     if (!keepass.isConnected) {
-        return [];
+        return undefined;
     }
 
     try {
         const taResponse = await keepass.testAssociation(tab);
         if (!taResponse) {
             browserAction.showDefault(tab);
-            return [];
+            return '';
         }
 
         if (!keepass.compareVersion(keepass.requiredKeePassXC, keepass.currentKeePassXC)) {
-            return [];
+            return '';
         }
 
         let password;
@@ -204,7 +204,7 @@ keepass.generatePassword = async function(tab) {
         return password;
     } catch (err) {
         logError(`generatePassword failed: ${err}`);
-        return [];
+        return undefined;
     }
 };
 

keepassxc-browser/manifest.json

@@ -159,7 +159,7 @@
     "applications": {
         "gecko": {
             "id": "keepassxc-browser@keepassxc.org",
-            "strict_min_version": "93.0"
+            "strict_min_version": "96.0"
         }
     },
     "default_locale": "en"