From ba2608ec7461d1611224c4b27b8c47b8c57edc18 Mon Sep 17 00:00:00 2001 From: Michael Weimann Date: Mon, 27 Mar 2023 08:37:35 +0200 Subject: [PATCH] Add `m.direct` filter / validation (#10436) --- src/utils/DMRoomMap.ts | 26 +++++- src/utils/dm/filterValidMDirect.ts | 69 ++++++++++++++ test/utils/DMRoomMap-test.ts | 110 ++++++++++++++++++++--- test/utils/dm/filterValidMDirect-test.ts | 77 ++++++++++++++++ 4 files changed, 267 insertions(+), 15 deletions(-) create mode 100644 src/utils/dm/filterValidMDirect.ts create mode 100644 test/utils/dm/filterValidMDirect-test.ts diff --git a/src/utils/DMRoomMap.ts b/src/utils/DMRoomMap.ts index 97c4b6b11c..1f95f5be61 100644 --- 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 new file mode 100644 index 0000000000..a0cfabae02 --- /dev/null +++ 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; +} + +/** + * 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 index 528bd0778c..03591ae54a 100644 --- 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; let dmRoomMap: DMRoomMap; - beforeEach(() => { - client = mocked(stubClient()); - - const mDirectEvent = mkEvent({ + const mkMDirectEvent = (content: any): MatrixEvent => { + return mkEvent({ event: true, type: EventType.Direct, user: client.getSafeUserId(), - content: mDirectContent, + content: content, }); - client.getAccountData.mockReturnValue(mDirectEvent); - dmRoomMap = new DMRoomMap(client); + }; + + beforeEach(() => { + client = mocked(stubClient()); + jest.spyOn(logger, "warn"); }); - it("getRoomIds should return the room Ids", () => { - expect(dmRoomMap.getRoomIds()).toEqual(new Set([roomId1, roomId2, roomId3, roomId4])); + 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, + 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([])); + }); + }); + + 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 new file mode 100644 index 0000000000..333b1feb4a --- /dev/null +++ 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], + }, + }); + }); +});