Implement MSC3873 to handle escaped dots in push rule keys

spec/unit/pushprocessor.spec.ts

@@ -1,6 +1,6 @@
 import * as utils from "../test-utils/test-utils";
 import { IActionsObject, PushProcessor } from "../../src/pushprocessor";
-import { EventType, IContent, MatrixClient, MatrixEvent } from "../../src";
+import { ConditionKind, EventType, IContent, MatrixClient, MatrixEvent } from "../../src";
 
 describe("NotificationService", function () {
     const testUserId = "@ali:matrix.org";
@@ -287,6 +287,13 @@ describe("NotificationService", function () {
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
+    // TODO: This is not spec compliant behaviour.
+    //
+    // See https://spec.matrix.org/v1.5/client-server-api/#conditions-1 which
+    // describes pattern should glob:
+    //
+    // 1. * matches 0 or ore characters;
+    // 2. ? matches exactly one character
     it("should bing on character group ([abc]) bing words.", function () {
         testEvent.event.content!.body = "Ping!";
         let actions = pushProcessor.actionsForEvent(testEvent);
@@ -296,12 +303,14 @@ describe("NotificationService", function () {
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
+    // TODO: This is not spec compliant behaviour. (See above.)
     it("should bing on character range ([a-z]) bing words.", function () {
         testEvent.event.content!.body = "I ate 6 pies";
         const actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(true);
     });
 
+    // TODO: This is not spec compliant behaviour. (See above.)
     it("should bing on character negation ([!a]) bing words.", function () {
         testEvent.event.content!.body = "boke";
         let actions = pushProcessor.actionsForEvent(testEvent);
@@ -330,6 +339,8 @@ describe("NotificationService", function () {
     // invalid
 
     it("should gracefully handle bad input.", function () {
+        // The following body is an object (not a string) and thus is invalid
+        // for matching against.
         testEvent.event.content!.body = { foo: "bar" };
         const actions = pushProcessor.actionsForEvent(testEvent);
         expect(actions.tweaks.highlight).toEqual(false);
@@ -493,4 +504,83 @@ describe("NotificationService", function () {
             });
         });
     });
+
+    it.each([
+        // The properly escaped key works.
+        { key: "content.m\\.test.foo", pattern: "bar", expected: true },
+        // An unescaped version does not match.
+        { key: "content.m.test.foo", pattern: "bar", expected: false },
+        // Over escaping does not match.
+        { key: "content.m\\.test\\.foo", pattern: "bar", expected: false },
+        // Escaping backslashes should match.
+        { key: "content.m\\\\example", pattern: "baz", expected: true },
+        // An unnecessary escape sequence leaves the backslash and still matches.
+        { key: "content.m\\example", pattern: "baz", expected: true },
+    ])("test against escaped dotted paths '$key'", ({ key, pattern, expected }) => {
+        testEvent = utils.mkEvent({
+            type: "m.room.message",
+            room: testRoomId,
+            user: "@alfred:localhost",
+            event: true,
+            content: {
+                // A dot in the field name.
+                "m.test": { foo: "bar" },
+                // A backslash in a field name.
+                "m\\example": "baz",
+            },
+        });
+
+        expect(
+            pushProcessor.ruleMatchesEvent(
+                {
+                    rule_id: "rule1",
+                    actions: [],
+                    conditions: [
+                        {
+                            kind: ConditionKind.EventMatch,
+                            key: key,
+                            keyParts: PushProcessor.partsForDottedKey(key),
+                            pattern: pattern,
+                        },
+                    ],
+                    default: false,
+                    enabled: true,
+                },
+                testEvent,
+            ),
+        ).toBe(expected);
+    });
+});
+
+describe("Test PushProcessor.partsForDottedKey", function () {
+    it.each([
+        // A field with no dots.
+        ["m", ["m"]],
+        // Simple dotted fields.
+        ["m.foo", ["m", "foo"]],
+        ["m.foo.bar", ["m", "foo", "bar"]],
+        // Backslash is used as an escape character.
+        ["m\\.foo", ["m.foo"]],
+        ["m\\\\.foo", ["m\\", "foo"]],
+        ["m\\\\\\.foo", ["m\\.foo"]],
+        ["m\\\\\\\\.foo", ["m\\\\", "foo"]],
+        ["m\\foo", ["m\\foo"]],
+        ["m\\\\foo", ["m\\foo"]],
+        ["m\\\\\\foo", ["m\\\\foo"]],
+        ["m\\\\\\\\foo", ["m\\\\foo"]],
+        // Ensure that escapes at the end don't cause issues.
+        ["m.foo\\", ["m", "foo\\"]],
+        ["m.foo\\\\", ["m", "foo\\"]],
+        ["m.foo\\.", ["m", "foo."]],
+        ["m.foo\\\\.", ["m", "foo\\", ""]],
+        ["m.foo\\\\\\.", ["m", "foo\\."]],
+        // Empty parts (corresponding to properties which are an empty string) are allowed.
+        [".m", ["", "m"]],
+        ["..m", ["", "", "m"]],
+        ["m.", ["m", ""]],
+        ["m..", ["m", "", ""]],
+        ["m..foo", ["m", "", "foo"]],
+    ])("partsFotDottedKey for %s", (path: string, expected: string[]) => {
+        expect(PushProcessor.partsForDottedKey(path)).toStrictEqual(expected);
+    });
 });

src/@types/PushRules.ts

@@ -76,6 +76,11 @@ export interface IPushRuleCondition<N extends ConditionKind | string> {
 
 export interface IEventMatchCondition extends IPushRuleCondition<ConditionKind.EventMatch> {
     key: string;
+    /**
+     * keyParts is calculated by rewriteDefaultRules by unescaping and spitting
+     * key.
+     */
+    keyParts: string[];
     pattern?: string;
     value?: string;
 }

src/pushprocessor.ts

@@ -63,6 +63,7 @@ const DEFAULT_OVERRIDE_RULES: IPushRule[] = [
             {
                 kind: ConditionKind.EventMatch,
                 key: "type",
+                keyParts: ["type"],
                 pattern: "m.reaction",
             },
         ],
@@ -77,11 +78,13 @@ const DEFAULT_OVERRIDE_RULES: IPushRule[] = [
             {
                 kind: ConditionKind.EventMatch,
                 key: "type",
+                keyParts: ["type"],
                 pattern: EventType.RoomServerAcl,
             },
             {
                 kind: ConditionKind.EventMatch,
                 key: "state_key",
+                keyParts: ["state_key"],
                 pattern: "",
             },
         ],
@@ -99,6 +102,7 @@ const DEFAULT_UNDERRIDE_RULES: IPushRule[] = [
             {
                 kind: ConditionKind.EventMatch,
                 key: "type",
+                keyParts: ["type"],
                 pattern: "org.matrix.msc3401.call",
             },
             {
@@ -162,6 +166,8 @@ export class PushProcessor {
         if (!newRules) newRules = {} as IPushRules;
         if (!newRules.global) newRules.global = {} as PushRuleSet;
         if (!newRules.global.override) newRules.global.override = [];
+        if (!newRules.global.room) newRules.global.room = [];
+        if (!newRules.global.sender) newRules.global.sender = [];
         if (!newRules.global.override) newRules.global.underride = [];
 
         // Merge the client-level defaults with the ones from the server
@@ -199,6 +205,26 @@ export class PushProcessor {
             }
         }
 
+        // Process the 'key' property on event_match conditions to unescape and
+        // split it.
+        for (const ruleset of [globalOverrides, newRules.global.room, newRules.global.sender, globalUnderrides]) {
+            for (const rule of ruleset) {
+                if (!rule.conditions) {
+                    continue;
+                }
+
+                for (const condition of rule.conditions) {
+                    if (condition.kind !== ConditionKind.EventMatch) {
+                        continue;
+                    }
+
+                    // The homeserver might have returned an object with this key
+                    // already, but we want to always stomp over it.
+                    condition.keyParts = PushProcessor.partsForDottedKey(condition.key);
+                }
+            }
+        }
+
         return newRules;
     }
 
@@ -253,6 +279,7 @@ export class PushProcessor {
                 rawrule.conditions!.push({
                     kind: ConditionKind.EventMatch,
                     key: "room_id",
+                    keyParts: ["room_id"],
                     value: tprule.rule_id,
                 });
                 break;
@@ -263,6 +290,7 @@ export class PushProcessor {
                 rawrule.conditions!.push({
                     kind: ConditionKind.EventMatch,
                     key: "user_id",
+                    keyParts: ["user_id"],
                     value: tprule.rule_id,
                 });
                 break;
@@ -273,6 +301,7 @@ export class PushProcessor {
                 rawrule.conditions!.push({
                     kind: ConditionKind.EventMatch,
                     key: "content.body",
+                    keyParts: ["content", "body"],
                     pattern: tprule.pattern,
                 });
                 break;
@@ -383,11 +412,11 @@ export class PushProcessor {
     }
 
     private eventFulfillsEventMatchCondition(cond: IEventMatchCondition, ev: MatrixEvent): boolean {
-        if (!cond.key) {
+        if (!cond.key || !cond.keyParts) {
             return false;
         }
 
-        const val = this.valueForDottedKey(cond.key, ev);
+        const val = this.valueForDottedKey(cond.keyParts, ev);
         if (typeof val !== "string") {
             return false;
         }
@@ -433,25 +462,91 @@ export class PushProcessor {
         return PushProcessor.cachedGlobToRegex[glob];
     }
 
-    private valueForDottedKey(key: string, ev: MatrixEvent): any {
-        const parts = key.split(".");
+    /**
+     * Parse the key into the separate fields to search by splitting on
+     * unescaped ".", and then removing any escape characters.
+     *
+     * @param str - The key of the push rule condition: a dotted field.
+     * @returns The unescaped parts to fetch.
+     * @internal
+     */
+    public static partsForDottedKey(str: string): string[] {
+        const result = [];
+
+        // The current field and whether the previous character was the escape
+        // character (a backslash).
+        let part = "";
+        let escaped = false;
+
+        // Iterate over each character, and decide whether to append to the current
+        // part (following the escape rules) or to start a new part (based on the
+        // field separator).
+        for (const c of str) {
+            // If the previous character was the escape character (a backslash)
+            // then decide what to append to the current part.
+            if (escaped) {
+                if (c === "\\" || c === ".") {
+                    // An escaped backslash or dot just gets added.
+                    part += c;
+                } else {
+                    // A character that shouldn't be escaped gets the backslash prepended.
+                    part += "\\" + c;
+                }
+                // This always resets being escaped.
+                escaped = false;
+                continue;
+            }
+
+            if (c == ".") {
+                // The field separator creates a new part.
+                result.push(part);
+                part = "";
+            } else if (c == "\\") {
+                // A backslash adds no characters, but starts an escape sequence.
+                escaped = true;
+            } else {
+                // Otherwise, just add the current character.
+                part += c;
+            }
+        }
+
+        // Ensure the final part is included. If there's an open escape sequence
+        // it should be included.
+        if (escaped) {
+            part += "\\";
+        }
+        result.push(part);
+
+        return result;
+    }
+
+    /**
+     * For a dotted field and event, fetch the value at that position, if one
+     * exists.
+     *
+     * @param parts - The pre-split key of the push rule condition to fetch.
+     * @param ev - The matrix event to fetch the field from.
+     * @returns The value at the dotted path given by key.
+     */
+    private valueForDottedKey(parts: string[], ev: MatrixEvent): any {
         let val: any;
 
         // special-case the first component to deal with encrypted messages
         const firstPart = parts[0];
+        let currentIndex = 0;
         if (firstPart === "content") {
             val = ev.getContent();
-            parts.shift();
+            ++currentIndex;
         } else if (firstPart === "type") {
             val = ev.getType();
-            parts.shift();
+            ++currentIndex;
         } else {
             // use the raw event for any other fields
             val = ev.event;
         }
 
-        while (parts.length > 0) {
-            const thisPart = parts.shift()!;
+        for (; currentIndex < parts.length; ++currentIndex) {
+            const thisPart = parts[currentIndex];
             if (isNullOrUndefined(val[thisPart])) {
                 return null;
             }