From 1646ea05bc4764d7bb10b2b6c1f06680eb1d01d3 Mon Sep 17 00:00:00 2001
From: Eric Eastwood <erice@element.io>
Date: Mon, 7 Nov 2022 14:24:43 -0600
Subject: [PATCH] Extra insurance that we don't mix events in the wrong
 timelines - v2 (#2856)

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

This PR is a v2 of https://github.com/matrix-org/matrix-js-sdk/pull/2848 since it was reverted in https://github.com/matrix-org/matrix-js-sdk/pull/2853

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`
---
 spec/unit/event-timeline-set.spec.ts | 76 ++++++++++++++++++++++++++++
 src/models/event-timeline-set.ts     | 22 ++++++++
 src/models/event.ts                  |  2 +-
 src/models/room.ts                   |  2 +-
 4 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/spec/unit/event-timeline-set.spec.ts b/spec/unit/event-timeline-set.spec.ts
index ce8fbf32d..2bc068af6 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,58 @@ 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('non-room timeline', () => {
+            it('Adds event to timeline', () => {
+                const nonRoomEventTimelineSet = new EventTimelineSet(
+                    // This is what we're specifically testing against, a timeline
+                    // without a `room` defined
+                    undefined,
+                );
+                const nonRoomEventTimeline = new EventTimeline(nonRoomEventTimelineSet);
+
+                expect(nonRoomEventTimeline.getEvents().length).toStrictEqual(0);
+                nonRoomEventTimelineSet.addEventToTimeline(messageEvent, nonRoomEventTimeline, {
+                    toStartOfTimeline: true,
+                });
+                expect(nonRoomEventTimeline.getEvents().length).toStrictEqual(1);
+            });
+        });
     });
 
     describe('aggregateRelations', () => {
diff --git a/src/models/event-timeline-set.ts b/src/models/event-timeline-set.ts
index d51fb0dbd..a69715dc3 100644
--- a/src/models/event-timeline-set.ts
+++ b/src/models/event-timeline-set.ts
@@ -701,6 +701,28 @@ 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).
+        //
+        // We can only run this check for timelines with a `room` because `canContain`
+        // requires it
+        if (this.room && !this.canContain(event)) {
+            let eventDebugString = `event=${event.getId()}`;
+            if (event.threadRootId) {
+                eventDebugString += `(belongs to thread=${event.threadRootId})`;
+            }
+            logger.warn(
+                `EventTimelineSet.addEventToTimeline: Ignoring ${eventDebugString} 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 ef684c247..1055a4c51 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,