Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement MSC3873 to handle escaped dots in push rule keys #3134

Merged
merged 26 commits into from
Mar 1, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
1facb89
Add comments.
clokep Feb 7, 2023
0600523
Implment MSC3873 to handle escaped dots in keys.
clokep Feb 7, 2023
0ac33c8
Add some comments about tests.
clokep Feb 7, 2023
9da576c
Merge remote-tracking branch 'upstream/develop' into dotted-keys
clokep Feb 14, 2023
f680ad8
Clarify spec behavior.
clokep Feb 14, 2023
a23ed41
Fix typo.
clokep Feb 14, 2023
704ed20
Don't manually iterate string.
clokep Feb 14, 2023
0b9d896
Clean-up tests.
clokep Feb 14, 2023
08144fa
Simplify tests.
clokep Feb 14, 2023
b724cf1
Add more tests & fix bug with empty parts.
clokep Feb 15, 2023
7e11af8
Add more edge cases.
clokep Feb 15, 2023
97fe8e9
Add a regular expression solution.
clokep Feb 15, 2023
322913e
Split on a simpler regular expression.
clokep Feb 15, 2023
7016e10
Merge remote-tracking branch 'upstream/develop' into dotted-keys
clokep Feb 15, 2023
d7fcd36
Remove redundant case in regex.
clokep Feb 15, 2023
c8a8f09
Enable sticky regex.
clokep Feb 15, 2023
256cc92
Rollback use of regex.
clokep Feb 16, 2023
bdde4ef
Cache values in the PushProcessor.
clokep Feb 16, 2023
d04d2a6
Use more each in tests.
clokep Feb 16, 2023
e673f36
Pre-calculate the key parts instead of caching them.
clokep Feb 22, 2023
d85806e
Merge remote-tracking branch 'upstream/develop' into dotted-keys
clokep Feb 22, 2023
3c445f6
Fix typo.
clokep Feb 23, 2023
ab9f212
Switch back to external cache, but clean out obsolete cached values.
clokep Feb 27, 2023
fc66cec
Remove obsolete property.
clokep Feb 27, 2023
cdb8760
Merge remote-tracking branch 'upstream/develop' into dotted-keys
clokep Feb 27, 2023
9e7c3c9
Remove more obsolete properties.
clokep Feb 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 79 additions & 1 deletion spec/unit/pushprocessor.spec.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -287,6 +287,7 @@ describe("NotificationService", function () {
expect(actions.tweaks.highlight).toEqual(true);
});

// TODO: This is not spec compliant behaviour.
clokep marked this conversation as resolved.
Show resolved Hide resolved
it("should bing on character group ([abc]) bing words.", function () {
testEvent.event.content!.body = "Ping!";
let actions = pushProcessor.actionsForEvent(testEvent);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -493,4 +498,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",
clokep marked this conversation as resolved.
Show resolved Hide resolved
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);
});
});
68 changes: 67 additions & 1 deletion src/pushprocessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -433,8 +433,74 @@ 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[] {
clokep marked this conversation as resolved.
Show resolved Hide resolved
clokep marked this conversation as resolved.
Show resolved Hide resolved
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).
clokep marked this conversation as resolved.
Show resolved Hide resolved
for (let i = 0; i < str.length; ++i) {
clokep marked this conversation as resolved.
Show resolved Hide resolved
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) {
clokep marked this conversation as resolved.
Show resolved Hide resolved
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(".");
// 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
Expand Down