Better fallback for unavailable event age (#3854)

* Age fallback using origin_server_ts instead of 0 Signed-off-by: Timo K <[email protected]> * use getMsUntilExpiry for isExpired Signed-off-by: Timo K <[email protected]> * fix tests tests now also rely on localTimestamp. So this need to be mocked as well Signed-off-by: Timo K <[email protected]> * better fallback for unavailable unsigned Signed-off-by: Timo K <[email protected]> --------- Signed-off-by: Timo K <[email protected]>
matrix-org · Nov 6, 2023 · 84bd8ab · 84bd8ab
1 parent a25ba7b
commit 84bd8ab
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 5 deletions.
diff --git a/spec/unit/matrixrtc/CallMembership.spec.ts b/spec/unit/matrixrtc/CallMembership.spec.ts
@@ -85,7 +85,9 @@ describe("CallMembership", () => {
 
     it("considers memberships expired when local age large", () => {
         const fakeEvent = makeMockEvent(1000);
-        fakeEvent.getLocalAge = jest.fn().mockReturnValue(6000);
+        const evAge = 6000;
+        fakeEvent.getLocalAge = jest.fn().mockReturnValue(evAge);
+        fakeEvent.localTimestamp = Date.now() - evAge;
         const membership = new CallMembership(fakeEvent, membershipTemplate);
         expect(membership.isExpired()).toEqual(true);
     });

diff --git a/spec/unit/matrixrtc/mocks.ts b/spec/unit/matrixrtc/mocks.ts
@@ -61,7 +61,7 @@ export function mockRTCEvent(
         getSender: jest.fn().mockReturnValue("@mock:user.example"),
         getTs: jest.fn().mockReturnValue(1000),
         getLocalAge: getLocalAgeFn,
-        localTimestamp: Date.now(),
+        localTimestamp: Date.now() - getLocalAgeFn(),
         getRoomId: jest.fn().mockReturnValue(roomId),
         sender: {
             userId: "@mock:user.example",

diff --git a/src/matrixrtc/CallMembership.ts b/src/matrixrtc/CallMembership.ts
@@ -91,7 +91,7 @@ export class CallMembership {
     }
 
     public isExpired(): boolean {
-        return this.getAbsoluteExpiry() < this.parentEvent.getTs() + this.parentEvent.getLocalAge();
+        return this.getMsUntilExpiry() <= 0;
     }
 
     public getActiveFoci(): Focus[] {

diff --git a/src/models/event.ts b/src/models/event.ts
@@ -392,7 +392,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
         });
 
         this.txnId = event.txn_id;
-        this.localTimestamp = Date.now() - (this.getAge() ?? 0);
+        this.localTimestamp = Date.now() - (this.getAge() ?? this.fallbackAge());
         this.reEmitter = new TypedReEmitter(this);
     }
 
@@ -663,6 +663,21 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
         return this.getUnsigned().age || this.event.age; // v2 / v1
     }
 
+    /**
+     * The fallbackAge is computed by using the origin_server_ts. So it is not adjusted
+     * to the local device clock. It should never be used.
+     * If there is no unsigned field in the event this is a better fallback then 0.
+     * It is supposed to only be used like this: `ev.getAge() ?? ev.fallbackAge()`
+     */
+    private fallbackAge(): number {
+        if (!this.getAge()) {
+            logger.warn(
+                "Age for event was not available, using `now - origin_server_ts` as a fallback. If the device clock is not correct issues might occur.",
+            );
+        }
+        return Math.max(Date.now() - this.getTs(), 0);
+    }
+
     /**
      * Get the age of the event when this function was called.
      * This is the 'age' field adjusted according to how long this client has
@@ -1384,7 +1399,7 @@ export class MatrixEvent extends TypedEventEmitter<MatrixEventEmittedEvents, Mat
             this.emit(MatrixEventEvent.LocalEventIdReplaced, this);
         }
 
-        this.localTimestamp = Date.now() - this.getAge()!;
+        this.localTimestamp = Date.now() - (this.getAge() ?? this.fallbackAge());
     }
 
     /**