Always pass recent decryption retry successes to widgets

Potential fix for element-hq/element-call#2561
matrix-org · Aug 14, 2024 · cbc55bd · cbc55bd
1 parent d9846dc
commit cbc55bd
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 10 deletions.
diff --git a/src/stores/widgets/StopGapWidget.ts b/src/stores/widgets/StopGapWidget.ts
@@ -472,14 +472,16 @@ export class StopGapWidget extends EventEmitter {
     }
 
     private onEvent = (ev: MatrixEvent): void => {
+        // It looks like we don't await this because if it does later succeed then we assume that
+        // a MatrixEventEvent.Decrypted will be emitted and we'll handle it there.
         this.client.decryptEventIfNeeded(ev);
         if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) return;
-        this.feedEvent(ev);
+        this.feedEvent(ev, ClientEvent.Event);
     };
 
     private onEventDecrypted = (ev: MatrixEvent): void => {
         if (ev.isDecryptionFailure()) return;
-        this.feedEvent(ev);
+        this.feedEvent(ev, MatrixEventEvent.Decrypted);
     };
 
     private onToDeviceEvent = async (ev: MatrixEvent): Promise<void> => {
@@ -488,7 +490,7 @@ export class StopGapWidget extends EventEmitter {
         await this.messaging?.feedToDevice(ev.getEffectiveEvent() as IRoomEvent, ev.isEncrypted());
     };
 
-    private feedEvent(ev: MatrixEvent): void {
+    private feedEvent(ev: MatrixEvent, reason: ClientEvent | MatrixEventEvent): void {
         if (!this.messaging) return;
 
         // Check to see if this event would be before or after our "read up to" marker. If it's
@@ -519,12 +521,21 @@ export class StopGapWidget extends EventEmitter {
             const timeline = room.getLiveTimeline();
             const events = arrayFastClone(timeline.getEvents()).reverse().slice(0, 100);
 
-            for (const timelineEvent of events) {
-                if (timelineEvent.getId() === upToEventId) {
-                    break;
-                } else if (timelineEvent.getId() === ev.getId()) {
-                    shouldForward = true;
-                    break;
+            if (reason === MatrixEventEvent.Decrypted) {
+                // If the event has just been decrypted, we should forward it if it appears anywhere in
+                // the timeline
+                shouldForward = events.some((timelineEvent) => timelineEvent.getId() === ev.getId());
+            } else {
+                // otherwise we search backwards in time until we either find the last event we have seen
+                // or the event we are looking for, or the events are exhausted.
+                for (const timelineEvent of events) {
+                    if (timelineEvent.getId() === upToEventId) {
+                        // no need to do shouldForward = false as set above
+                        break;
+                    } else if (timelineEvent.getId() === ev.getId()) {
+                        shouldForward = true;
+                        break;
+                    }
                 }
             }
 

diff --git a/test/stores/widgets/StopGapWidget-test.ts b/test/stores/widgets/StopGapWidget-test.ts
@@ -16,7 +16,7 @@ limitations under the License.
 
 import { mocked, MockedObject } from "jest-mock";
 import { last } from "lodash";
-import { MatrixEvent, MatrixClient, ClientEvent, EventTimeline } from "matrix-js-sdk/src/matrix";
+import { MatrixEvent, MatrixClient, ClientEvent, EventTimeline, MatrixEventEvent } from "matrix-js-sdk/src/matrix";
 import { ClientWidgetApi, WidgetApiFromWidgetAction } from "matrix-widget-api";
 import { waitFor } from "@testing-library/react";
 
@@ -185,6 +185,43 @@ describe("StopGapWidget", () => {
             expect(messaging.feedEvent).toHaveBeenCalledTimes(2);
             expect(messaging.feedEvent).toHaveBeenLastCalledWith(event.getEffectiveEvent(), "!1:example.org");
         });
+
+        describe("e2ee", () => {
+            it("should not feed events that failed decryption", async () => {
+                event1.isDecryptionFailure = jest.fn().mockReturnValue(true);
+                client.emit(ClientEvent.Event, event1);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(0);
+
+                client.emit(MatrixEventEvent.Decrypted, event1);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(0);
+            });
+
+            it("should feed event after decryption retry success", async () => {
+                event1.isDecryptionFailure = jest.fn().mockReturnValue(true);
+                client.emit(ClientEvent.Event, event1);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(0);
+
+                event1.isDecryptionFailure = jest.fn().mockReturnValue(false);
+                client.emit(MatrixEventEvent.Decrypted, event1);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(1);
+                expect(messaging.feedEvent).toHaveBeenLastCalledWith(event1.getEffectiveEvent(), "!1:example.org");
+            });
+
+            it("should feed event after decryption success even if older", async () => {
+                event1.isDecryptionFailure = jest.fn().mockReturnValue(true);
+                client.emit(ClientEvent.Event, event1);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(0);
+
+                client.emit(ClientEvent.Event, event2);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(1);
+                expect(messaging.feedEvent).toHaveBeenLastCalledWith(event2.getEffectiveEvent(), "!1:example.org");
+
+                event1.isDecryptionFailure = jest.fn().mockReturnValue(false);
+                client.emit(MatrixEventEvent.Decrypted, event1);
+                expect(messaging.feedEvent).toHaveBeenCalledTimes(2);
+                expect(messaging.feedEvent).toHaveBeenLastCalledWith(event1.getEffectiveEvent(), "!1:example.org");
+            });
+        });
     });
 
     describe("when there is a voice broadcast recording", () => {