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,82 @@ 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,
+                            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/pushprocessor.ts

@@ -123,6 +123,12 @@ export class PushProcessor {
      */
     public constructor(private readonly client: MatrixClient) {}
 
+    /**
+     * Maps the original key from the push rules to a list of property names
+     * after unescaping.
+     */
+    private readonly parsedKeys = new Map<string, string[]>();
+
     /**
      * Convert a list of actions into a object with the actions as keys and their values
      * @example
@@ -433,25 +439,98 @@ export class PushProcessor {
         return PushProcessor.cachedGlobToRegex[glob];
     }
 
+    /**
+     * 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 key - The key of the push rule condition: a dotted field to fetch.
+     * @param ev - The matrix event to fetch the field from.
+     * @returns The value at the dotted path given by key.
+     */
     private valueForDottedKey(key: string, ev: MatrixEvent): any {
-        const parts = key.split(".");
+        // Cache the parsed key since it is per rule, not per event (and there's
+        // no reason to keep calculating it).
+        let parts = this.parsedKeys.get(key);
+        if (parts === undefined) {
+            parts = PushProcessor.partsForDottedKey(key);
+            this.parsedKeys.set(key, parts);
+        }
         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;
             }