diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index dd84f03820a..f43d5f8aa6c 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/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 more 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); + }); }); diff --git a/src/client.ts b/src/client.ts index d46116675ee..d59f5cc5d68 100644 --- a/src/client.ts +++ b/src/client.ts @@ -8499,10 +8499,22 @@ export class MatrixClient extends TypedEventEmitter { return this.http.authedRequest(Method.Get, "/pushrules/").then((rules: IPushRules) => { - return PushProcessor.rewriteDefaultRules(rules); + this.setPushRules(rules); + return this.pushRules!; }); } + /** + * Update the push rules for the account. This should be called whenever + * updated push rules are available. + */ + public setPushRules(rules: IPushRules): void { + // Fix-up defaults, if applicable. + this.pushRules = PushProcessor.rewriteDefaultRules(rules); + // Pre-calculate any necessary caches. + this.pushProcessor.updateCachedPushRuleKeys(this.pushRules); + } + /** * @returns Promise which resolves: an empty object `{}` * @returns Rejects: with an error response. diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 0d7338f8fb1..2bbf8fbdb9c 100644 --- a/src/pushprocessor.ts +++ b/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(); + /** * Convert a list of actions into a object with the actions as keys and their values * @example @@ -162,7 +168,7 @@ 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.override) newRules.global.underride = []; + if (!newRules.global.underride) newRules.global.underride = []; // Merge the client-level defaults with the ones from the server const globalOverrides = newRules.global.override; @@ -202,6 +208,53 @@ export class PushProcessor { return newRules; } + /** + * Pre-caches the parsed keys for push rules and cleans out any obsolete cache + * entries. Should be called after push rules are updated. + * @param newRules - The new push rules. + */ + public updateCachedPushRuleKeys(newRules: IPushRules): void { + // These lines are mostly to make the tests happy. We shouldn't run into these + // properties missing in practice. + 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.underride) newRules.global.underride = []; + + // Process the 'key' property on event_match conditions pre-cache the + // values and clean-out any unused values. + const toRemoveKeys = new Set(this.parsedKeys.keys()); + for (const ruleset of [ + newRules.global.override, + newRules.global.room, + newRules.global.sender, + newRules.global.underride, + ]) { + for (const rule of ruleset) { + if (!rule.conditions) { + continue; + } + + for (const condition of rule.conditions) { + if (condition.kind !== ConditionKind.EventMatch) { + continue; + } + + // Ensure we keep this key. + toRemoveKeys.delete(condition.key); + + // Pre-process the key. + this.parsedKeys.set(condition.key, PushProcessor.partsForDottedKey(condition.key)); + } + } + } + // Any keys that were previously cached, but are no longer needed should + // be removed. + toRemoveKeys.forEach((k) => this.parsedKeys.delete(k)); + } + private static cachedGlobToRegex: Record = {}; // $glob: RegExp private matchingRuleFromKindSet(ev: MatrixEvent, kindset: PushRuleSet): IAnnotatedPushRule | null { @@ -433,25 +486,99 @@ 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("."); + // The key should already have been parsed via updateCachedPushRuleKeys, + // but if it hasn't (maybe via an old consumer of the SDK which hasn't + // been updated?) then lazily calculate it here. + 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; } diff --git a/src/sliding-sync-sdk.ts b/src/sliding-sync-sdk.ts index f63a3046002..93e29e0baa3 100644 --- a/src/sliding-sync-sdk.ts +++ b/src/sliding-sync-sdk.ts @@ -43,7 +43,6 @@ import { } from "./sliding-sync"; import { EventType } from "./@types/event"; import { IPushRules } from "./@types/PushRules"; -import { PushProcessor } from "./pushprocessor"; import { RoomStateEvent } from "./models/room-state"; import { RoomMemberEvent } from "./models/room-member"; @@ -262,7 +261,7 @@ class ExtensionAccountData implements Extension(); - this.client.pushRules = PushProcessor.rewriteDefaultRules(rules); + this.client.setPushRules(rules); } const prevEvent = prevEventsMap[accountDataEvent.getType()]; this.client.emit(ClientEvent.AccountData, accountDataEvent, prevEvent); diff --git a/src/sync.ts b/src/sync.ts index bfd6029f0b3..057ee1540da 100644 --- a/src/sync.ts +++ b/src/sync.ts @@ -32,7 +32,6 @@ import * as utils from "./utils"; import { IDeferred } from "./utils"; import { Filter } from "./filter"; import { EventTimeline } from "./models/event-timeline"; -import { PushProcessor } from "./pushprocessor"; import { logger } from "./logger"; import { InvalidStoreError, InvalidStoreState } from "./errors"; import { ClientEvent, IStoredClientOpts, MatrixClient, PendingEventOrdering, ResetTimelineCallback } from "./client"; @@ -1162,7 +1161,7 @@ export class SyncApi { // (see sync) before syncing over the network. if (accountDataEvent.getType() === EventType.PushRules) { const rules = accountDataEvent.getContent(); - client.pushRules = PushProcessor.rewriteDefaultRules(rules); + client.setPushRules(rules); } const prevEvent = prevEventsMap[accountDataEvent.getType()!]; client.emit(ClientEvent.AccountData, accountDataEvent, prevEvent);