Fix behaviour of isRelation with relation m.replace for state events (#…

…2389) * Add some short-circuits to skip async code * Fix behaviour of `isRelation` with relation `m.replace` for state events
matrix-org · May 20, 2022 · e81d845 · e81d845
1 parent 81d884f
commit e81d845
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 7 deletions.
diff --git a/spec/unit/relations.spec.ts b/spec/unit/relations.spec.ts
@@ -168,6 +168,8 @@ describe("Relations", function() {
         await relations.setTargetEvent(originalTopic);
         expect(originalTopic.replacingEvent()).toBe(null);
         expect(originalTopic.getContent().topic).toBe("orig");
+        expect(badlyEditedTopic.isRelation()).toBe(false);
+        expect(badlyEditedTopic.isRelation("m.replace")).toBe(false);
 
         await relations.addEvent(badlyEditedTopic);
         expect(originalTopic.replacingEvent()).toBe(null);

diff --git a/src/models/event.ts b/src/models/event.ts
@@ -1290,7 +1290,7 @@ export class MatrixEvent extends TypedEventEmitter<EmittedEvents, MatrixEventHan
 
     /**
      * Get whether the event is a relation event, and of a given type if
-     * `relType` is passed in.
+     * `relType` is passed in. State events cannot be relation events
      *
      * @param {string?} relType if given, checks that the relation is of the
      * given type
@@ -1300,8 +1300,11 @@ export class MatrixEvent extends TypedEventEmitter<EmittedEvents, MatrixEventHan
         // Relation info is lifted out of the encrypted content when sent to
         // encrypted rooms, so we have to check `getWireContent` for this.
         const relation = this.getWireContent()?.["m.relates_to"];
-        return relation && relation.rel_type && relation.event_id &&
-            ((relType && relation.rel_type === relType) || !relType);
+        if (this.isState() && relation?.rel_type === RelationType.Replace) {
+            // State events cannot be m.replace relations
+            return false;
+        }
+        return relation?.rel_type && relation.event_id && (relType ? relation.rel_type === relType : true);
     }
 
     /**

diff --git a/src/models/relations.ts b/src/models/relations.ts
@@ -103,7 +103,7 @@ export class Relations extends TypedEventEmitter<RelationsEvent, EventHandlerMap
 
         if (this.relationType === RelationType.Annotation) {
             this.addAnnotationToAggregation(event);
-        } else if (this.relationType === RelationType.Replace && this.targetEvent) {
+        } else if (this.relationType === RelationType.Replace && this.targetEvent && !this.targetEvent.isState()) {
             const lastReplacement = await this.getLastReplacement();
             this.targetEvent.makeReplaced(lastReplacement);
         }
@@ -144,7 +144,7 @@ export class Relations extends TypedEventEmitter<RelationsEvent, EventHandlerMap
 
         if (this.relationType === RelationType.Annotation) {
             this.removeAnnotationFromAggregation(event);
-        } else if (this.relationType === RelationType.Replace && this.targetEvent) {
+        } else if (this.relationType === RelationType.Replace && this.targetEvent && !this.targetEvent.isState()) {
             const lastReplacement = await this.getLastReplacement();
             this.targetEvent.makeReplaced(lastReplacement);
         }
@@ -261,7 +261,7 @@ export class Relations extends TypedEventEmitter<RelationsEvent, EventHandlerMap
         if (this.relationType === RelationType.Annotation) {
             // Remove the redacted annotation from aggregation by key
             this.removeAnnotationFromAggregation(redactedEvent);
-        } else if (this.relationType === RelationType.Replace && this.targetEvent) {
+        } else if (this.relationType === RelationType.Replace && this.targetEvent && !this.targetEvent.isState()) {
             const lastReplacement = await this.getLastReplacement();
             this.targetEvent.makeReplaced(lastReplacement);
         }
@@ -364,7 +364,7 @@ export class Relations extends TypedEventEmitter<RelationsEvent, EventHandlerMap
         }
         this.targetEvent = event;
 
-        if (this.relationType === RelationType.Replace) {
+        if (this.relationType === RelationType.Replace && !this.targetEvent.isState()) {
             const replacement = await this.getLastReplacement();
             // this is the initial update, so only call it if we already have something
             // to not emit Event.replaced needlessly