From 433b7afd7118bdf49917c247e63ce799884ecda5 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Fri, 4 Nov 2022 05:54:23 -0500
Subject: [PATCH] Extra insurance that we don't mix events in the wrong
 timelines (#2848)

Add checks to `addEventToTimeline` as extra insurance that we don't mix events in the wrong timelines (main timeline vs thread timeline).

Split out from https://github.com/matrix-org/matrix-js-sdk/pull/2521

Previously, we just relied on the callers to make sure they're doing the right thing and since it's easy to get it wrong, we mixed and bugs happened.

Call stacks for how events get added to a timeline:

 - `TimelineSet.addEventsToTimeline` -> `TimelineSet.addEventToTimeline` -> `Timeline.addEvent`
 - `TimelineSet.addEventToTimeline` -> `Timeline.addEvent`
 - `TimelineSet.addLiveEvent` -> `TimelineSet.addEventToTimeline` -> `Timeline.addEvent`
---
 .DS_Store                            | Bin 0 -> 6148 bytes
 spec/unit/event-timeline-set.spec.ts |  59 +++++++++++++++++++++++++++
 src/models/event-timeline-set.ts     |  13 ++++++
 src/models/event.ts                  |   2 +-
 src/models/room.ts                   |   2 +-
 5 files changed, 74 insertions(+), 2 deletions(-)
 create mode 100644 .DS_Store

diff --git a/.DS_Store b/.DS_Store
new file mode 100644
index 0000000000000000000000000000000000000000..8c6875c741d7013ebf68f1a62758ebca25db1fed
GIT binary patch
literal 6148
zcmeHK%}T>S5Z<*_6N-?7LXQhx3%1l&ikDF93mDOZN=;1BV9b^zHHT8jSzpK}@p+ut
z-GIfMMeGdhe)GGV{UH0p7~|tb*kjCLj9JhSIVv@R?%L3nNk-&2Mm7&(8G!W>%uVdC
z1AcphWh`Y6LGk_j<0#9!-A}&NXm0Q9T9(za?z|^ic)6c1vaz4upmix_98|g=Tt%~D
z>g=D%B=@6ama2j%oI%R%b(Dm%7|TTxW~$cH0n4`SsncFAhbR4x=nV&}j#!RHqmCH#
zk5?<(IygK!y_i17FNu89baG%@$)3Rq-a#>|dG*pHmdPV{s_ZI@kQg8ahyh|?vl%ew
zg4Nn=8ff*z05MR*0PYV08lr2k)Tp)&=<xcC@g^b)==hdE6b4;`rACN=a9s+hOSySs
za9s|5Ve(vqrAA%OxSAQpF*8?>7p`UpzfkFnyBet{28e-m1{&IQ@cciAU#9kvzg|KX
zF+dFbGX{8b;!Vb|D08;{SRS6W0@^(^6wE780ResO5&#D7BW)Gbae+GIxduy(I12hz
QIUrpG6d}|R1HZt)7X)QVng9R*

literal 0
HcmV?d00001

diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts
index ce8fbf32d..d4b0f6213 100644
--- a/spec/unit/event-timeline-set.spec.ts
+++ b/spec/unit/event-timeline-set.spec.ts
@@ -55,6 +55,23 @@ describe('EventTimelineSet', () => {
         });
     };
 
+    const mkThreadResponse = (root: MatrixEvent) => utils.mkEvent({
+        event: true,
+        type: EventType.RoomMessage,
+        user: userA,
+        room: roomId,
+        content: {
+            "body": "Thread response :: " + Math.random(),
+            "m.relates_to": {
+                "event_id": root.getId(),
+                "m.in_reply_to": {
+                    "event_id": root.getId(),
+                },
+                "rel_type": "m.thread",
+            },
+        },
+    }, room.client);
+
     beforeEach(() => {
         client = utils.mock(MatrixClient, 'MatrixClient');
         client.reEmitter = utils.mock(ReEmitter, 'ReEmitter');
@@ -117,6 +134,13 @@ describe('EventTimelineSet', () => {
     });
 
     describe('addEventToTimeline', () => {
+        let thread: Thread;
+
+        beforeEach(() => {
+            (client.supportsExperimentalThreads as jest.Mock).mockReturnValue(true);
+            thread = new Thread("!thread_id:server", messageEvent, { room, client });
+        });
+
         it("Adds event to timeline", () => {
             const liveTimeline = eventTimelineSet.getLiveTimeline();
             expect(liveTimeline.getEvents().length).toStrictEqual(0);
@@ -144,6 +168,41 @@ describe('EventTimelineSet', () => {
                 );
             }).not.toThrow();
         });
+
+        it("should not add an event to a timeline that does not belong to the timelineSet", () => {
+            const eventTimelineSet2 = new EventTimelineSet(room);
+            const liveTimeline2 = eventTimelineSet2.getLiveTimeline();
+            expect(liveTimeline2.getEvents().length).toStrictEqual(0);
+
+            expect(() => {
+                eventTimelineSet.addEventToTimeline(messageEvent, liveTimeline2, {
+                    toStartOfTimeline: true,
+                });
+            }).toThrowError();
+        });
+
+        it("should not add a threaded reply to the main room timeline", () => {
+            const liveTimeline = eventTimelineSet.getLiveTimeline();
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+
+            const threadedReplyEvent = mkThreadResponse(messageEvent);
+
+            eventTimelineSet.addEventToTimeline(threadedReplyEvent, liveTimeline, {
+                toStartOfTimeline: true,
+            });
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+        });
+
+        it("should not add a normal message to the timelineSet representing a thread", () => {
+            const eventTimelineSetForThread = new EventTimelineSet(room, {}, client, thread);
+            const liveTimeline = eventTimelineSetForThread.getLiveTimeline();
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+
+            eventTimelineSetForThread.addEventToTimeline(messageEvent, liveTimeline, {
+                toStartOfTimeline: true,
+            });
+            expect(liveTimeline.getEvents().length).toStrictEqual(0);
+        });
     });
 
     describe('aggregateRelations', () => {
diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts
index e41eec158..fd9dc1ff8 100644
--- a/src/models/event-timeline-set.ts
+++ b/src/models/event-timeline-set.ts
@@ -702,6 +702,19 @@ export class EventTimelineSet extends TypedEventEmitter<EmittedEvents, EventTime
             );
         }
 
+        if (timeline.getTimelineSet() !== this) {
+            throw new Error(`EventTimelineSet.addEventToTimeline: Timeline=${timeline.toString()} does not belong " + 
+                "in timelineSet(threadId=${this.thread?.id})`);
+        }
+
+        // Make sure events don't get mixed in timelines they shouldn't be in
+        // (e.g. a threaded message should not be in the main timeline).
+        if (!this.canContain(event)) {
+            logger.warn(`EventTimelineSet.addEventToTimeline: Ignoring event=${event.getId()} that does not belong " + 
+                "in timeline=${timeline.toString()} timelineSet(threadId=${this.thread?.id})`);
+            return;
+        }
+
         const eventId = event.getId()!;
         timeline.addEvent(event, {
             toStartOfTimeline,
diff --git a/src/models/event.ts b/src/models/event.ts
index 6ba8a6511..21c5c4cc2 100644
--- a/src/models/event.ts
+++ b/src/models/event.ts
@@ -107,7 +107,7 @@ export interface IEventRelation {
     event_id?: string;
     is_falling_back?: boolean;
     "m.in_reply_to"?: {
-        event_id: string;
+        event_id?: string;
     };
     key?: string;
 }
diff --git a/src/models/room.ts b/src/models/room.ts
index 3bfa308c5..0079aba9d 100644
--- a/src/models/room.ts
+++ b/src/models/room.ts
@@ -1862,7 +1862,7 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
         shouldLiveInThread: boolean;
         threadId?: string;
     } {
-        if (!this.client.supportsExperimentalThreads()) {
+        if (!this.client?.supportsExperimentalThreads()) {
             return {
                 shouldLiveInRoom: true,
                 shouldLiveInThread: false,