Add m.direct filter / validation (#10436)

weeman1337 · web-flow · commit ba2608ec7461 · 2023-03-27T08:37:35.000+02:00
diff --git a/src/utils/DMRoomMap.ts b/src/utils/DMRoomMap.ts
@@ -23,6 +23,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event";
 import { Optional } from "matrix-events-sdk";
 
 import { MatrixClientPeg } from "../MatrixClientPeg";
+import { filterValidMDirect } from "./dm/filterValidMDirect";
 
 /**
  * Class that takes a Matrix Client and flips the m.direct map
@@ -44,8 +45,8 @@ export default class DMRoomMap {
         // see onAccountData
         this.hasSentOutPatchDirectAccountDataPatch = false;
 
-        const mDirectEvent = matrixClient.getAccountData(EventType.Direct)?.getContent() ?? {};
-        this.mDirectEvent = { ...mDirectEvent }; // copy as we will mutate
+        const mDirectRawContent = matrixClient.getAccountData(EventType.Direct)?.getContent() ?? {};
+        this.setMDirectFromContent(mDirectRawContent);
     }
 
     /**
@@ -84,9 +85,28 @@ export default class DMRoomMap {
         this.matrixClient.removeListener(ClientEvent.AccountData, this.onAccountData);
     }
 
+    /**
+     * Filter m.direct content to contain only valid data and then sets it.
+     * Logs if invalid m.direct content occurs.
+     * {@link filterValidMDirect}
+     *
+     * @param content - Raw m.direct content
+     */
+    private setMDirectFromContent(content: unknown): void {
+        const { valid, filteredContent } = filterValidMDirect(content);
+
+        if (!valid) {
+            logger.warn("Invalid m.direct content occurred", content);
+        }
+
+        this.mDirectEvent = filteredContent;
+    }
+
     private onAccountData = (ev: MatrixEvent): void => {
+        console.log("onAccountData");
+
         if (ev.getType() == EventType.Direct) {
-            this.mDirectEvent = { ...ev.getContent() }; // copy as we will mutate
+            this.setMDirectFromContent(ev.getContent());
             this.userToRooms = null;
             this.roomToUser = null;
         }
diff --git a/src/utils/dm/filterValidMDirect.ts b/src/utils/dm/filterValidMDirect.ts
@@ -0,0 +1,69 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+interface FilterValidMDirectResult {
+    /** Whether the entire content is valid */
+    valid: boolean;
+    /** Filtered content with only the valid parts */
+    filteredContent: Record<string, string[]>;
+}
+
+/**
+ * Filter m.direct content to be compliant to https://spec.matrix.org/v1.6/client-server-api/#mdirect.
+ *
+ * @param content - Raw event content to be filerted
+ * @returns value as a flag whether to content was valid.
+ *          filteredContent with only values from the content that are spec compliant.
+ */
+export const filterValidMDirect = (content: unknown): FilterValidMDirectResult => {
+    if (content === null || typeof content !== "object") {
+        return {
+            valid: false,
+            filteredContent: {},
+        };
+    }
+
+    const filteredContent = new Map();
+    let valid = true;
+
+    for (const [userId, roomIds] of Object.entries(content)) {
+        if (typeof userId !== "string") {
+            valid = false;
+            continue;
+        }
+
+        if (!Array.isArray(roomIds)) {
+            valid = false;
+            continue;
+        }
+
+        const filteredRoomIds: string[] = [];
+        filteredContent.set(userId, filteredRoomIds);
+
+        for (const roomId of roomIds) {
+            if (typeof roomId === "string") {
+                filteredRoomIds.push(roomId);
+            } else {
+                valid = false;
+            }
+        }
+    }
+
+    return {
+        valid,
+        filteredContent: Object.fromEntries(filteredContent.entries()),
+    };
+};
diff --git a/test/utils/DMRoomMap-test.ts b/test/utils/DMRoomMap-test.ts
@@ -15,18 +15,18 @@ limitations under the License.
 */
 
 import { mocked, Mocked } from "jest-mock";
-import { EventType, IContent, MatrixClient } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
+import { ClientEvent, EventType, IContent, MatrixClient, MatrixEvent } from "matrix-js-sdk/src/matrix";
 
 import DMRoomMap from "../../src/utils/DMRoomMap";
 import { mkEvent, stubClient } from "../test-utils";
-
 describe("DMRoomMap", () => {
     const roomId1 = "!room1:example.com";
     const roomId2 = "!room2:example.com";
     const roomId3 = "!room3:example.com";
     const roomId4 = "!room4:example.com";
 
-    const mDirectContent = {
+    const validMDirectContent = {
         "user@example.com": [roomId1, roomId2],
         "@user:example.com": [roomId1, roomId3, roomId4],
         "@user2:example.com": [] as string[],
@@ -35,20 +35,106 @@ describe("DMRoomMap", () => {
     let client: Mocked<MatrixClient>;
     let dmRoomMap: DMRoomMap;
 
+    const mkMDirectEvent = (content: any): MatrixEvent => {
+        return mkEvent({
+            event: true,
+            type: EventType.Direct,
+            user: client.getSafeUserId(),
+            content: content,
+        });
+    };
+
     beforeEach(() => {
         client = mocked(stubClient());
+        jest.spyOn(logger, "warn");
+    });
 
-        const mDirectEvent = mkEvent({
-            event: true,
+    describe("when m.direct has valid content", () => {
+        beforeEach(() => {
+            client.getAccountData.mockReturnValue(mkMDirectEvent(validMDirectContent));
+            dmRoomMap = new DMRoomMap(client);
+            dmRoomMap.start();
+        });
+
+        it("getRoomIds should return the room Ids", () => {
+            expect(dmRoomMap.getRoomIds()).toEqual(new Set([roomId1, roomId2, roomId3, roomId4]));
+        });
+
+        describe("and there is an update with valid data", () => {
+            beforeEach(() => {
+                client.emit(
+                    ClientEvent.AccountData,
+                    mkMDirectEvent({
+                        "@user:example.com": [roomId1, roomId3],
+                    }),
+                );
+            });
+
+            it("getRoomIds should return the new room Ids", () => {
+                expect(dmRoomMap.getRoomIds()).toEqual(new Set([roomId1, roomId3]));
+            });
+        });
+
+        describe("and there is an update with invalid data", () => {
+            const partiallyInvalidContent = {
+                "@user1:example.com": [roomId1, roomId3],
+                "@user2:example.com": "room2, room3",
+            };
+
+            beforeEach(() => {
+                client.emit(ClientEvent.AccountData, mkMDirectEvent(partiallyInvalidContent));
+            });
+
+            it("getRoomIds should return the valid room Ids", () => {
+                expect(dmRoomMap.getRoomIds()).toEqual(new Set([roomId1, roomId3]));
+            });
+
+            it("should log the invalid content", () => {
+                expect(logger.warn).toHaveBeenCalledWith("Invalid m.direct content occurred", partiallyInvalidContent);
+            });
+        });
+    });
+
+    describe("when m.direct content contains the entire event", () => {
+        const mDirectContentContent = {
             type: EventType.Direct,
-            user: client.getSafeUserId(),
-            content: mDirectContent,
+            content: validMDirectContent,
+        };
+
+        beforeEach(() => {
+            client.getAccountData.mockReturnValue(mkMDirectEvent(mDirectContentContent));
+            dmRoomMap = new DMRoomMap(client);
+        });
+
+        it("should log the invalid content", () => {
+            expect(logger.warn).toHaveBeenCalledWith("Invalid m.direct content occurred", mDirectContentContent);
+        });
+
+        it("getRoomIds should return an empty list", () => {
+            expect(dmRoomMap.getRoomIds()).toEqual(new Set([]));
         });
-        client.getAccountData.mockReturnValue(mDirectEvent);
-        dmRoomMap = new DMRoomMap(client);
     });
 
-    it("getRoomIds should return the room Ids", () => {
-        expect(dmRoomMap.getRoomIds()).toEqual(new Set([roomId1, roomId2, roomId3, roomId4]));
+    describe("when partially crap m.direct content appears", () => {
+        const partiallyCrapContent = {
+            "hello": 23,
+            "@user1:example.com": [] as string[],
+            "@user2:example.com": [roomId1, roomId2],
+            "@user3:example.com": "room1, room2, room3",
+            "@user4:example.com": [roomId4],
+        };
+
+        beforeEach(() => {
+            client.getAccountData.mockReturnValue(mkMDirectEvent(partiallyCrapContent));
+            dmRoomMap = new DMRoomMap(client);
+        });
+
+        it("should log the invalid content", () => {
+            expect(logger.warn).toHaveBeenCalledWith("Invalid m.direct content occurred", partiallyCrapContent);
+        });
+
+        it("getRoomIds should only return the valid items", () => {
+            expect(dmRoomMap.getRoomIds()).toEqual(new Set([roomId1, roomId2, roomId4]));
+        });
     });
 });
diff --git a/test/utils/dm/filterValidMDirect-test.ts b/test/utils/dm/filterValidMDirect-test.ts
@@ -0,0 +1,77 @@
+/*
+Copyright 2023 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import { filterValidMDirect } from "../../../src/utils/dm/filterValidMDirect";
+
+const roomId1 = "!room1:example.com";
+const roomId2 = "!room2:example.com";
+const userId1 = "@user1:example.com";
+const userId2 = "@user2:example.com";
+const userId3 = "@user3:example.com";
+
+describe("filterValidMDirect", () => {
+    it("should return an empty object as valid content", () => {
+        expect(filterValidMDirect({})).toEqual({
+            valid: true,
+            filteredContent: {},
+        });
+    });
+
+    it("should return valid content", () => {
+        expect(
+            filterValidMDirect({
+                [userId1]: [roomId1, roomId2],
+                [userId2]: [roomId1],
+            }),
+        ).toEqual({
+            valid: true,
+            filteredContent: {
+                [userId1]: [roomId1, roomId2],
+                [userId2]: [roomId1],
+            },
+        });
+    });
+
+    it("should return an empy object for null", () => {
+        expect(filterValidMDirect(null)).toEqual({
+            valid: false,
+            filteredContent: {},
+        });
+    });
+
+    it("should return an empy object for a non-object", () => {
+        expect(filterValidMDirect(23)).toEqual({
+            valid: false,
+            filteredContent: {},
+        });
+    });
+
+    it("should only return valid content", () => {
+        const invalidContent = {
+            [userId1]: [23],
+            [userId2]: [roomId2],
+            [userId3]: "room1",
+        };
+
+        expect(filterValidMDirect(invalidContent)).toEqual({
+            valid: false,
+            filteredContent: {
+                [userId1]: [],
+                [userId2]: [roomId2],
+            },
+        });
+    });
+});
