From 1facb897573b26b33b8aca9baabea7cc0377bad2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Feb 2023 11:20:44 -0500 Subject: [PATCH 01/22] Add comments. --- src/pushprocessor.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 0d7338f8fb1..58c00c310b2 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -433,6 +433,14 @@ export class PushProcessor { return PushProcessor.cachedGlobToRegex[glob]; } + /** + * 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("."); let val: any; From 06005230a53a629c9c1badc64c6cb08f3eb7562e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Feb 2023 12:25:08 -0500 Subject: [PATCH 02/22] Implment MSC3873 to handle escaped dots in keys. --- spec/unit/pushprocessor.spec.ts | 75 ++++++++++++++++++++++++++++++++- src/pushprocessor.ts | 60 +++++++++++++++++++++++++- 2 files changed, 133 insertions(+), 2 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index dd84f03820a..f12a0fc9fb4 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"; @@ -493,4 +493,77 @@ describe("NotificationService", function () { }); }); }); + + it("test against escaped dotted paths", function () { + 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: "content.m\\.test.foo", + pattern: "bar", + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(true); + + expect( + pushProcessor.ruleMatchesEvent( + { + rule_id: "rule1", + actions: [], + conditions: [ + { + kind: ConditionKind.EventMatch, + key: "content.m\\\\example", + pattern: "baz", + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(true); + + expect( + pushProcessor.ruleMatchesEvent( + { + rule_id: "rule1", + actions: [], + conditions: [ + { + kind: ConditionKind.EventMatch, + // An incorrect escape sequence leaves the backslash. + key: "content.m\\example", + pattern: "baz", + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(true); + }); }); diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 58c00c310b2..57cea8ddceb 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -433,6 +433,62 @@ export class PushProcessor { return PushProcessor.cachedGlobToRegex[glob]; } + /** + * Parse a dotted key into parts following the escape rules. + * + * @param str - The key of the push rule condition: a dotted field. + * @returns The unescaped parts to fetch. + */ + private partsFotDottedKey(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 (let i = 0; i < str.length; ++i) { + const c = str[i]; + + // 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 is one. + if (part.length) { + result.push(part); + } + + return result; + } + /** * For a dotted field and event, fetch the value at that position, if one * exists. @@ -442,7 +498,9 @@ export class PushProcessor { * @returns The value at the dotted path given by key. */ 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. + const parts = this.partsFotDottedKey(key); let val: any; // special-case the first component to deal with encrypted messages From 0ac33c89457136e294fa2132c7f91b324122ed06 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Feb 2023 14:12:28 -0500 Subject: [PATCH 03/22] Add some comments about tests. --- spec/unit/pushprocessor.spec.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index f12a0fc9fb4..682a863594c 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -287,6 +287,7 @@ describe("NotificationService", function () { expect(actions.tweaks.highlight).toEqual(true); }); + // TODO: This is not spec compliant behaviour. it("should bing on character group ([abc]) bing words.", function () { testEvent.event.content!.body = "Ping!"; let actions = pushProcessor.actionsForEvent(testEvent); @@ -296,12 +297,14 @@ describe("NotificationService", function () { expect(actions.tweaks.highlight).toEqual(true); }); + // TODO: This is not spec compliant behaviour. 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. it("should bing on character negation ([!a]) bing words.", function () { testEvent.event.content!.body = "boke"; let actions = pushProcessor.actionsForEvent(testEvent); @@ -330,6 +333,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); From f680ad80b0f36b3c3eb152634cc563dfce23eb6b Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Feb 2023 10:08:55 -0500 Subject: [PATCH 04/22] Clarify spec behavior. --- spec/unit/pushprocessor.spec.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 682a863594c..ffc6a422eff 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -288,6 +288,12 @@ describe("NotificationService", function () { }); // 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); @@ -297,14 +303,14 @@ describe("NotificationService", function () { expect(actions.tweaks.highlight).toEqual(true); }); - // TODO: This is not spec compliant behaviour. + // 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. + // 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); From a23ed415d9da54040311b97eac8f5ae427c661d6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Feb 2023 10:19:21 -0500 Subject: [PATCH 05/22] Fix typo. --- src/pushprocessor.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 57cea8ddceb..a88e8c868d7 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -439,7 +439,7 @@ export class PushProcessor { * @param str - The key of the push rule condition: a dotted field. * @returns The unescaped parts to fetch. */ - private partsFotDottedKey(str: string): string[] { + private partsForDottedKey(str: string): string[] { const result = []; // The current field and whether the previous character was the escape @@ -500,7 +500,7 @@ export class PushProcessor { private valueForDottedKey(key: string, ev: MatrixEvent): any { // Parse the key into the separate fields to search by splitting on // unescaped ".", and then removing any escape characters. - const parts = this.partsFotDottedKey(key); + const parts = this.partsForDottedKey(key); let val: any; // special-case the first component to deal with encrypted messages From 704ed20b60d9a504f45348c07670713e5b3fa387 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Feb 2023 10:25:05 -0500 Subject: [PATCH 06/22] Don't manually iterate string. --- src/pushprocessor.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index a88e8c868d7..e922d121a7b 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -450,9 +450,7 @@ export class PushProcessor { // 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 (let i = 0; i < str.length; ++i) { - const c = str[i]; - + 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) { From 0b9d896cbedbe6a4ea30e5e9d73d5c3040fa1a6d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Feb 2023 10:32:05 -0500 Subject: [PATCH 07/22] Clean-up tests. --- spec/unit/pushprocessor.spec.ts | 44 ++++++++++++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index ffc6a422eff..95552d865a9 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -519,6 +519,7 @@ describe("NotificationService", function () { }, }); + // The properly escaped key works. expect( pushProcessor.ruleMatchesEvent( { @@ -538,6 +539,47 @@ describe("NotificationService", function () { ), ).toBe(true); + // An unescaped version does not match. + expect( + pushProcessor.ruleMatchesEvent( + { + rule_id: "rule1", + actions: [], + conditions: [ + { + kind: ConditionKind.EventMatch, + key: "content.m.test.foo", + pattern: "bar", + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(false); + + // Over escaping does not match. + expect( + pushProcessor.ruleMatchesEvent( + { + rule_id: "rule1", + actions: [], + conditions: [ + { + kind: ConditionKind.EventMatch, + key: "content.m\\.test\\.foo", + pattern: "bar", + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(false); + + // Escaping backslashes should match. expect( pushProcessor.ruleMatchesEvent( { @@ -557,6 +599,7 @@ describe("NotificationService", function () { ), ).toBe(true); + // An unnecessary escape sequence leaves the backslash and still matches. expect( pushProcessor.ruleMatchesEvent( { @@ -565,7 +608,6 @@ describe("NotificationService", function () { conditions: [ { kind: ConditionKind.EventMatch, - // An incorrect escape sequence leaves the backslash. key: "content.m\\example", pattern: "baz", }, From 08144fa347dbe469b28a4181b4e72b175a76a471 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Feb 2023 11:14:28 -0500 Subject: [PATCH 08/22] Simplify tests. --- spec/unit/pushprocessor.spec.ts | 133 ++++++++------------------------ 1 file changed, 34 insertions(+), 99 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 95552d865a9..9ac041e9528 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -519,104 +519,39 @@ describe("NotificationService", function () { }, }); - // The properly escaped key works. - expect( - pushProcessor.ruleMatchesEvent( - { - rule_id: "rule1", - actions: [], - conditions: [ - { - kind: ConditionKind.EventMatch, - key: "content.m\\.test.foo", - pattern: "bar", - }, - ], - default: false, - enabled: true, - }, - testEvent, - ), - ).toBe(true); - - // An unescaped version does not match. - expect( - pushProcessor.ruleMatchesEvent( - { - rule_id: "rule1", - actions: [], - conditions: [ - { - kind: ConditionKind.EventMatch, - key: "content.m.test.foo", - pattern: "bar", - }, - ], - default: false, - enabled: true, - }, - testEvent, - ), - ).toBe(false); - - // Over escaping does not match. - expect( - pushProcessor.ruleMatchesEvent( - { - rule_id: "rule1", - actions: [], - conditions: [ - { - kind: ConditionKind.EventMatch, - key: "content.m\\.test\\.foo", - pattern: "bar", - }, - ], - default: false, - enabled: true, - }, - testEvent, - ), - ).toBe(false); - - // Escaping backslashes should match. - expect( - pushProcessor.ruleMatchesEvent( - { - rule_id: "rule1", - actions: [], - conditions: [ - { - kind: ConditionKind.EventMatch, - key: "content.m\\\\example", - pattern: "baz", - }, - ], - default: false, - enabled: true, - }, - testEvent, - ), - ).toBe(true); - - // An unnecessary escape sequence leaves the backslash and still matches. - expect( - pushProcessor.ruleMatchesEvent( - { - rule_id: "rule1", - actions: [], - conditions: [ - { - kind: ConditionKind.EventMatch, - key: "content.m\\example", - pattern: "baz", - }, - ], - default: false, - enabled: true, - }, - testEvent, - ), - ).toBe(true); + // Each test is a (key, pattern, expected match) tuple. + const testCases = [ + // 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 }, + ]; + + for (const testCase of testCases) { + expect( + pushProcessor.ruleMatchesEvent( + { + rule_id: "rule1", + actions: [], + conditions: [ + { + kind: ConditionKind.EventMatch, + key: testCase.key, + pattern: testCase.pattern, + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(testCase.expected); + } }); }); From b724cf1547cc52743757d30778041f6cf7c8ada3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Feb 2023 09:37:17 -0500 Subject: [PATCH 09/22] Add more tests & fix bug with empty parts. --- spec/unit/pushprocessor.spec.ts | 18 ++++++++++++++++++ src/pushprocessor.ts | 11 +++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 9ac041e9528..992466b0ee6 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -555,3 +555,21 @@ describe("NotificationService", function () { } }); }); + +describe("Test PushProcessor.partsForDottedKey", function () { + it.each([ + ["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"]], + // Extraneous escapes leave the backslash. + ["m\\foo", ["m\\foo"]], + // Empty parts (corresponding to properties which are an empty string) are allowed. + ["m.", ["m", ""]], + ["m..", ["m", "", ""]], + ])("partsFotDottedKey for %s", (path: string, expected: string[]) => { + expect(PushProcessor.partsForDottedKey(path)).toStrictEqual(expected); + }); +}); diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index e922d121a7b..48ce0869ca4 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -438,8 +438,9 @@ export class PushProcessor { * * @param str - The key of the push rule condition: a dotted field. * @returns The unescaped parts to fetch. + * @internal */ - private partsForDottedKey(str: string): string[] { + public static partsForDottedKey(str: string): string[] { const result = []; // The current field and whether the previous character was the escape @@ -479,10 +480,8 @@ export class PushProcessor { } } - // Ensure the final part is included, if there is one. - if (part.length) { - result.push(part); - } + // Ensure the final part is included. + result.push(part); return result; } @@ -498,7 +497,7 @@ export class PushProcessor { private valueForDottedKey(key: string, ev: MatrixEvent): any { // Parse the key into the separate fields to search by splitting on // unescaped ".", and then removing any escape characters. - const parts = this.partsForDottedKey(key); + const parts = PushProcessor.partsForDottedKey(key); let val: any; // special-case the first component to deal with encrypted messages From 7e11af82506d0393233047276c24784496b0ee60 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Feb 2023 12:51:56 -0500 Subject: [PATCH 10/22] Add more edge cases. --- spec/unit/pushprocessor.spec.ts | 19 +++++++++++++++++-- src/pushprocessor.ts | 6 +++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 992466b0ee6..6215fe6114b 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -558,17 +558,32 @@ describe("NotificationService", function () { 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"]], - // Extraneous escapes leave the backslash. + ["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/pushprocessor.ts b/src/pushprocessor.ts index 48ce0869ca4..95b451770d9 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -480,7 +480,11 @@ export class PushProcessor { } } - // Ensure the final part is included. + // 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; From 97fe8e944506be01c95437c76617186fff3d79e8 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Feb 2023 14:55:04 -0500 Subject: [PATCH 11/22] Add a regular expression solution. This is ~80% slower than the basic split("."). --- src/pushprocessor.ts | 72 +++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 45 deletions(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 95b451770d9..62bd282687e 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -441,53 +441,35 @@ export class PushProcessor { * @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; - } + // This complicated regular expression matches: + // + // (?<=^|\.) Lookbehind to ensure we're starting with the beginning of + // the string or at a dot. This avoids capturing empty groups + // per dot. + // + // (?: A non-capturing group which repeats 0 or more times + // + // \\.? An escape character followed by any character (or nothing, + // for the end of strings). + // + // | Or + // + // [^.\\] Any character except a dot or backslash. + // + // )* + const regexp = /(?<=^|\.)(?:\\.?|[^.\\])*/g; + const parts = str.match(regexp); + if (parts === null) { + // Or error? Should this happen? + return []; } - - // Ensure the final part is included. If there's an open escape sequence - // it should be included. - if (escaped) { - part += "\\"; + // If the last entry is empty & the second to last entry ends with a . + // then the regular expression creates an extra final group. + if (parts.length > 2 && parts[parts.length - 1] === "" && parts[parts.length - 2].endsWith(".")) { + parts.pop(); } - result.push(part); - - return result; + // Unescape each part individually. + return parts.map((p) => p.replace(/\\([\\.])/g, "$1")); } /** From 322913ec114aa91533dbaffe5c614b857b124e15 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Feb 2023 15:02:24 -0500 Subject: [PATCH 12/22] Split on a simpler regular expression. This is ~50% slower than a simple split("."). --- src/pushprocessor.ts | 31 +++---------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 62bd282687e..b12907b5101 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -441,35 +441,10 @@ export class PushProcessor { * @internal */ public static partsForDottedKey(str: string): string[] { - // This complicated regular expression matches: - // - // (?<=^|\.) Lookbehind to ensure we're starting with the beginning of - // the string or at a dot. This avoids capturing empty groups - // per dot. - // - // (?: A non-capturing group which repeats 0 or more times - // - // \\.? An escape character followed by any character (or nothing, - // for the end of strings). - // - // | Or - // - // [^.\\] Any character except a dot or backslash. - // - // )* - const regexp = /(?<=^|\.)(?:\\.?|[^.\\])*/g; - const parts = str.match(regexp); - if (parts === null) { - // Or error? Should this happen? - return []; - } - // If the last entry is empty & the second to last entry ends with a . - // then the regular expression creates an extra final group. - if (parts.length > 2 && parts[parts.length - 1] === "" && parts[parts.length - 2].endsWith(".")) { - parts.pop(); - } + // Split on a dot that is after any 0 or even sets of backslashes. + const regexp = /(?<=[^\\](?:\\\\)+|[^\\]|^)\./g; // Unescape each part individually. - return parts.map((p) => p.replace(/\\([\\.])/g, "$1")); + return str.split(regexp).map((p) => p.replace(/\\([\\.])/g, "$1")); } /** From d7fcd36d6e904edc7ff5e07a2916cb236e93b232 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Feb 2023 15:24:58 -0500 Subject: [PATCH 13/22] Remove redundant case in regex. --- src/pushprocessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index b12907b5101..5e1809f80d6 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -442,7 +442,7 @@ export class PushProcessor { */ public static partsForDottedKey(str: string): string[] { // Split on a dot that is after any 0 or even sets of backslashes. - const regexp = /(?<=[^\\](?:\\\\)+|[^\\]|^)\./g; + const regexp = /(?<=[^\\](?:\\\\)*|^)\./g; // Unescape each part individually. return str.split(regexp).map((p) => p.replace(/\\([\\.])/g, "$1")); } From c8a8f09c431ef71b99fb2d1872e96ee336ffa119 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 15 Feb 2023 15:25:08 -0500 Subject: [PATCH 14/22] Enable sticky regex. --- src/pushprocessor.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 5e1809f80d6..ee61e11e0ae 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -442,7 +442,7 @@ export class PushProcessor { */ public static partsForDottedKey(str: string): string[] { // Split on a dot that is after any 0 or even sets of backslashes. - const regexp = /(?<=[^\\](?:\\\\)*|^)\./g; + const regexp = /(?<=[^\\](?:\\\\)*|^)\./gy; // Unescape each part individually. return str.split(regexp).map((p) => p.replace(/\\([\\.])/g, "$1")); } From 256cc9278879544cc01e67ec2010940e4e2c8124 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 08:08:04 -0500 Subject: [PATCH 15/22] Rollback use of regex. --- src/pushprocessor.ts | 51 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index ee61e11e0ae..95b451770d9 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -441,10 +441,53 @@ export class PushProcessor { * @internal */ public static partsForDottedKey(str: string): string[] { - // Split on a dot that is after any 0 or even sets of backslashes. - const regexp = /(?<=[^\\](?:\\\\)*|^)\./gy; - // Unescape each part individually. - return str.split(regexp).map((p) => p.replace(/\\([\\.])/g, "$1")); + 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; } /** From bdde4efeef68910824cb7efcc1db45ae43db57e4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 08:28:56 -0500 Subject: [PATCH 16/22] Cache values in the PushProcessor. --- src/pushprocessor.ts | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 95b451770d9..59dea3f7311 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 @@ -434,7 +440,8 @@ export class PushProcessor { } /** - * Parse a dotted key into parts following the escape rules. + * 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. @@ -499,26 +506,31 @@ export class PushProcessor { * @returns The value at the dotted path given by key. */ private valueForDottedKey(key: string, ev: MatrixEvent): any { - // Parse the key into the separate fields to search by splitting on - // unescaped ".", and then removing any escape characters. - const parts = PushProcessor.partsForDottedKey(key); + // 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; } From d04d2a62884186570235ac71d9557f5b38459c71 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 16 Feb 2023 08:43:40 -0500 Subject: [PATCH 17/22] Use more each in tests. --- spec/unit/pushprocessor.spec.ts | 65 +++++++++++++++------------------ 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 6215fe6114b..b747556aed2 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -505,7 +505,18 @@ describe("NotificationService", function () { }); }); - it("test against escaped dotted paths", 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, @@ -519,40 +530,24 @@ describe("NotificationService", function () { }, }); - // Each test is a (key, pattern, expected match) tuple. - const testCases = [ - // 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 }, - ]; - - for (const testCase of testCases) { - expect( - pushProcessor.ruleMatchesEvent( - { - rule_id: "rule1", - actions: [], - conditions: [ - { - kind: ConditionKind.EventMatch, - key: testCase.key, - pattern: testCase.pattern, - }, - ], - default: false, - enabled: true, - }, - testEvent, - ), - ).toBe(testCase.expected); - } + expect( + pushProcessor.ruleMatchesEvent( + { + rule_id: "rule1", + actions: [], + conditions: [ + { + kind: ConditionKind.EventMatch, + key: key, + pattern: pattern, + }, + ], + default: false, + enabled: true, + }, + testEvent, + ), + ).toBe(expected); }); }); From e673f363ea4b4f2de3c0b8833d80cda020564ded Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 22 Feb 2023 14:22:03 -0500 Subject: [PATCH 18/22] Pre-calculate the key parts instead of caching them. --- spec/unit/pushprocessor.spec.ts | 1 + src/@types/PushRules.ts | 5 ++++ src/pushprocessor.ts | 50 ++++++++++++++++++++++----------- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index b747556aed2..897b3ff99c0 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -539,6 +539,7 @@ describe("NotificationService", function () { { kind: ConditionKind.EventMatch, key: key, + keyParts: PushProcessor.partsForDottedKey(key), pattern: pattern, }, ], diff --git a/src/@types/PushRules.ts b/src/@types/PushRules.ts index 56a93df9fb3..a48e7804cfd 100644 --- a/src/@types/PushRules.ts +++ b/src/@types/PushRules.ts @@ -76,6 +76,11 @@ export interface IPushRuleCondition { export interface IEventMatchCondition extends IPushRuleCondition { key: string; + /** + * keyParts is calculated by rewriteDefaultRules by unescaping and spitting + * key. + */ + keyParts: string[]; pattern?: string; value?: string; } diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index 59dea3f7311..84141ec632d 100644 --- a/src/pushprocessor.ts +++ b/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", }, { @@ -123,12 +127,6 @@ 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 @@ -168,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 @@ -205,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; } @@ -259,6 +279,7 @@ export class PushProcessor { rawrule.conditions!.push({ kind: ConditionKind.EventMatch, key: "room_id", + keyParts: ["room_id"], value: tprule.rule_id, }); break; @@ -269,6 +290,7 @@ export class PushProcessor { rawrule.conditions!.push({ kind: ConditionKind.EventMatch, key: "user_id", + keyParts: ["user_id"], value: tprule.rule_id, }); break; @@ -279,6 +301,7 @@ export class PushProcessor { rawrule.conditions!.push({ kind: ConditionKind.EventMatch, key: "content.body", + keyParts: ["content", "body"], pattern: tprule.pattern, }); break; @@ -389,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; } @@ -501,18 +524,11 @@ export class PushProcessor { * 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 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(key: string, ev: MatrixEvent): any { - // 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); - } + private valueForDottedKey(parts: string[], ev: MatrixEvent): any { let val: any; // special-case the first component to deal with encrypted messages From 3c445f68c66df78502c4e95cdd36c53a881292fe Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 23 Feb 2023 13:00:45 -0500 Subject: [PATCH 19/22] Fix typo. --- spec/unit/pushprocessor.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index 897b3ff99c0..c089266d65f 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -292,7 +292,7 @@ describe("NotificationService", function () { // See https://spec.matrix.org/v1.5/client-server-api/#conditions-1 which // describes pattern should glob: // - // 1. * matches 0 or ore characters; + // 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!"; From ab9f21247184bdbe16d93c0fc69d4a217cab5299 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 27 Feb 2023 09:04:06 -0500 Subject: [PATCH 20/22] Switch back to external cache, but clean out obsolete cached values. --- src/client.ts | 14 ++++++++- src/pushprocessor.ts | 69 ++++++++++++++++++++++++++++++++--------- src/sliding-sync-sdk.ts | 3 +- src/sync.ts | 3 +- 4 files changed, 69 insertions(+), 20 deletions(-) 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 84141ec632d..a5236c25a5f 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -127,6 +127,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 @@ -166,9 +172,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.room) newRules.global.room = []; - if (!newRules.global.sender) newRules.global.sender = []; - 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; @@ -205,9 +209,33 @@ 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]) { + 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; @@ -218,14 +246,17 @@ export class PushProcessor { 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); + // Ensure we keep this key. + toRemoveKeys.delete(condition.key); + + // Pre-process the key. + this.parsedKeys.set(condition.key, PushProcessor.partsForDottedKey(condition.key)); } } } - - return newRules; + // 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 @@ -412,11 +443,11 @@ export class PushProcessor { } private eventFulfillsEventMatchCondition(cond: IEventMatchCondition, ev: MatrixEvent): boolean { - if (!cond.key || !cond.keyParts) { + if (!cond.key) { return false; } - const val = this.valueForDottedKey(cond.keyParts, ev); + const val = this.valueForDottedKey(cond.key, ev); if (typeof val !== "string") { return false; } @@ -524,11 +555,19 @@ export class PushProcessor { * 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 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(parts: string[], ev: MatrixEvent): any { + private valueForDottedKey(key: string, ev: MatrixEvent): any { + // 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 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); From fc66ceca496e2a9eb2ae50190198f0d2f2307b44 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 27 Feb 2023 09:06:00 -0500 Subject: [PATCH 21/22] Remove obsolete property. --- src/@types/PushRules.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/@types/PushRules.ts b/src/@types/PushRules.ts index a48e7804cfd..56a93df9fb3 100644 --- a/src/@types/PushRules.ts +++ b/src/@types/PushRules.ts @@ -76,11 +76,6 @@ export interface IPushRuleCondition { export interface IEventMatchCondition extends IPushRuleCondition { key: string; - /** - * keyParts is calculated by rewriteDefaultRules by unescaping and spitting - * key. - */ - keyParts: string[]; pattern?: string; value?: string; } From 9e7c3c9315a6b5d171aeb880f8a95f1cc4cb75f5 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 27 Feb 2023 09:07:26 -0500 Subject: [PATCH 22/22] Remove more obsolete properties. --- spec/unit/pushprocessor.spec.ts | 1 - src/pushprocessor.ts | 7 ------- 2 files changed, 8 deletions(-) diff --git a/spec/unit/pushprocessor.spec.ts b/spec/unit/pushprocessor.spec.ts index c089266d65f..f43d5f8aa6c 100644 --- a/spec/unit/pushprocessor.spec.ts +++ b/spec/unit/pushprocessor.spec.ts @@ -539,7 +539,6 @@ describe("NotificationService", function () { { kind: ConditionKind.EventMatch, key: key, - keyParts: PushProcessor.partsForDottedKey(key), pattern: pattern, }, ], diff --git a/src/pushprocessor.ts b/src/pushprocessor.ts index a5236c25a5f..2bbf8fbdb9c 100644 --- a/src/pushprocessor.ts +++ b/src/pushprocessor.ts @@ -63,7 +63,6 @@ const DEFAULT_OVERRIDE_RULES: IPushRule[] = [ { kind: ConditionKind.EventMatch, key: "type", - keyParts: ["type"], pattern: "m.reaction", }, ], @@ -78,13 +77,11 @@ 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: "", }, ], @@ -102,7 +99,6 @@ const DEFAULT_UNDERRIDE_RULES: IPushRule[] = [ { kind: ConditionKind.EventMatch, key: "type", - keyParts: ["type"], pattern: "org.matrix.msc3401.call", }, { @@ -310,7 +306,6 @@ export class PushProcessor { rawrule.conditions!.push({ kind: ConditionKind.EventMatch, key: "room_id", - keyParts: ["room_id"], value: tprule.rule_id, }); break; @@ -321,7 +316,6 @@ export class PushProcessor { rawrule.conditions!.push({ kind: ConditionKind.EventMatch, key: "user_id", - keyParts: ["user_id"], value: tprule.rule_id, }); break; @@ -332,7 +326,6 @@ export class PushProcessor { rawrule.conditions!.push({ kind: ConditionKind.EventMatch, key: "content.body", - keyParts: ["content", "body"], pattern: tprule.pattern, }); break;