From 9df8009ae3fd02ce3622e227e16376d76afe6696 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 21:07:20 +0200 Subject: [PATCH 1/9] Implement the current spec for event match conditions This fixes that people randomly get pinged on every reply to a user names @roomba:server.tld. fixes #2541 Signed-off-by: Nicolas Werner --- .../session/pushrules/EventMatchCondition.kt | 31 ++++++++----------- .../session/pushrules/rest/PushCondition.kt | 2 +- .../pushrules/PushRulesConditionTest.kt | 19 ++++++++++-- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt index 8875807b8a..4b11bfcdd7 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt @@ -31,18 +31,14 @@ class EventMatchCondition( * The glob-style pattern to match against. Patterns with no special glob characters should * be treated as having asterisks prepended and appended when testing the condition. */ - val pattern: String, - /** - * true to match only words. In this case pattern will not be considered as a glob - */ - val wordsOnly: Boolean + val pattern: String ) : Condition { override fun isSatisfied(event: Event, conditionResolver: ConditionResolver): Boolean { return conditionResolver.resolveEventMatchCondition(event, this) } - override fun technicalDescription() = "'$key' matches '$pattern', words only '$wordsOnly'" + override fun technicalDescription() = "'$key' matches '$pattern'" fun isSatisfied(event: Event): Boolean { // TODO encrypted events? @@ -50,21 +46,20 @@ class EventMatchCondition( ?: return false val value = extractField(rawJson, key) ?: return false - // Patterns with no special glob characters should be treated as having asterisks prepended - // and appended when testing the condition. + // The match is performed case-insensitively, and must match the entire value of + // the event field given by `key` (though see below regarding `content.body`). The + // exact meaning of "case insensitive" is defined by the implementation of the + // homeserver. + // + // As a special case, if `key` is `content.body`, then `pattern` must instead + // match any substring of the value of the property which starts and ends at a + // word boundary. return try { - if (wordsOnly) { + if (key == 'content.body') { value.caseInsensitiveFind(pattern) } else { - val modPattern = if (pattern.hasSpecialGlobChar()) { - // Regex.containsMatchIn() is way faster without leading and trailing - // stars, that don't make any difference for the evaluation result - pattern.removePrefix("*").removeSuffix("*").simpleGlobToRegExp() - } else { - pattern.simpleGlobToRegExp() - } - val regex = Regex(modPattern, RegexOption.DOT_MATCHES_ALL) - regex.containsMatchIn(value) + val regex = Regex(pattern.simpleGlobToRegExp(), setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) + regex.matches(value) } } catch (e: Throwable) { // e.g PatternSyntaxException diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt index ec0936e4c8..c2b45619e5 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt @@ -63,7 +63,7 @@ data class PushCondition( return when (Kind.fromString(kind)) { Kind.EventMatch -> { if (key != null && pattern != null) { - EventMatchCondition(key, pattern, rule.ruleId == RuleIds.RULE_ID_CONTAIN_USER_NAME) + EventMatchCondition(key, pattern) } else { Timber.e("Malformed Event match condition") null diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt index c4a3404e80..ef6230dbbc 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt @@ -67,7 +67,7 @@ class PushRulesConditionTest : MatrixTest { ) assert(condition.isSatisfied(simpleTextEvent)) - assert(!condition.isSatisfied(simpleRoomMemberEvent)) + assertFalse(condition.isSatisfied(simpleRoomMemberEvent)) } @Test @@ -98,7 +98,7 @@ class PushRulesConditionTest : MatrixTest { val condition = EventMatchCondition("content.body", "cake", false) assert(condition.isSatisfied(createSimpleTextEvent("How was the cake?"))) - assert(condition.isSatisfied(createSimpleTextEvent("Howwasthecake?"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("Howwasthecake?"))) } @Test @@ -124,6 +124,21 @@ class PushRulesConditionTest : MatrixTest { assert(condition.isSatisfied(createSimpleTextEvent("BEN"))) } + @Test + fun test_eventmatch_at_room_condition() { + val condition = EventMatchCondition("content.body", "@room", true) + + assertFalse(condition.isSatisfied(createSimpleTextEvent("@roomba"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("room benoit"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("abc@roomba"))) + + assert(condition.isSatisfied(createSimpleTextEvent("@room"))) + assert(condition.isSatisfied(createSimpleTextEvent("@room, ben"))) + assert(condition.isSatisfied(createSimpleTextEvent("@ROOM"))) + assert(condition.isSatisfied(createSimpleTextEvent("Use:@room"))) + assert(condition.isSatisfied(createSimpleTextEvent("Don't ping @room!"))) + } + @Test fun test_notice_condition() { val conditionEqual = EventMatchCondition("content.msgtype", "m.notice", false) From 48fc634825422e9eca5b4b839ff60d06148f2414 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 21:33:21 +0200 Subject: [PATCH 2/9] Add changelog and fix condition not matching globs on body Signed-off-by: Nicolas Werner --- changelog.d/6457.bugfix | 1 + .../android/sdk/api/session/pushrules/EventMatchCondition.kt | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog.d/6457.bugfix diff --git a/changelog.d/6457.bugfix b/changelog.d/6457.bugfix new file mode 100644 index 0000000000..89ba075378 --- /dev/null +++ b/changelog.d/6457.bugfix @@ -0,0 +1 @@ +Fix that replies to @roomba would be highlighted as a room ping. Contributed by Nico. diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt index 4b11bfcdd7..fbedb0b2a0 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt @@ -56,7 +56,8 @@ class EventMatchCondition( // word boundary. return try { if (key == 'content.body') { - value.caseInsensitiveFind(pattern) + val regex = Regex("(\\W|^)"+pattern.simpleGlobToRegExp() + "(\\W|$)", setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) + regex.containsMatchIn(value) } else { val regex = Regex(pattern.simpleGlobToRegExp(), setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) regex.matches(value) From b24b1a188490115f767c48f61c79d4fe8ca40863 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 21:35:59 +0200 Subject: [PATCH 3/9] Add negative test for globs on body only matching full words Signed-off-by: Nicolas Werner --- .../android/sdk/api/session/pushrules/PushRulesConditionTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt index ef6230dbbc..7d64217843 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt @@ -106,6 +106,7 @@ class PushRulesConditionTest : MatrixTest { val condition = EventMatchCondition("content.body", "cake*lie", false) assert(condition.isSatisfied(createSimpleTextEvent("How was the cakeisalie?"))) + assertFalse(condition.isSatisfied(createSimpleTextEvent("How was the notcakeisalie?"))) } @Test From e980f6bb2f8bc1a54074c49754a2a014b7d44424 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 21:51:49 +0200 Subject: [PATCH 4/9] kotlin is not dart Signed-off-by: Nicolas Werner --- .../android/sdk/api/session/pushrules/EventMatchCondition.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt index fbedb0b2a0..293ae6c043 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt @@ -55,8 +55,8 @@ class EventMatchCondition( // match any substring of the value of the property which starts and ends at a // word boundary. return try { - if (key == 'content.body') { - val regex = Regex("(\\W|^)"+pattern.simpleGlobToRegExp() + "(\\W|$)", setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) + if (key == "content.body") { + val regex = Regex("(\\W|^)" + pattern.simpleGlobToRegExp() + "(\\W|$)", setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) regex.containsMatchIn(value) } else { val regex = Regex(pattern.simpleGlobToRegExp(), setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) From 365ec8ef722f1efb769c3e79e6825ae812ff4e07 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 21:57:56 +0200 Subject: [PATCH 5/9] Remove unused imports Signed-off-by: Nicolas Werner --- .../android/sdk/api/session/pushrules/EventMatchCondition.kt | 2 -- .../android/sdk/api/session/pushrules/rest/PushCondition.kt | 1 - 2 files changed, 3 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt index 293ae6c043..e5448322e6 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt @@ -17,8 +17,6 @@ package org.matrix.android.sdk.api.session.pushrules import org.matrix.android.sdk.api.session.events.model.Event import org.matrix.android.sdk.internal.di.MoshiProvider -import org.matrix.android.sdk.internal.util.caseInsensitiveFind -import org.matrix.android.sdk.internal.util.hasSpecialGlobChar import org.matrix.android.sdk.internal.util.simpleGlobToRegExp import timber.log.Timber diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt index c2b45619e5..0e175b47b9 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt @@ -22,7 +22,6 @@ import org.matrix.android.sdk.api.session.pushrules.ContainsDisplayNameCondition import org.matrix.android.sdk.api.session.pushrules.EventMatchCondition import org.matrix.android.sdk.api.session.pushrules.Kind import org.matrix.android.sdk.api.session.pushrules.RoomMemberCountCondition -import org.matrix.android.sdk.api.session.pushrules.RuleIds import org.matrix.android.sdk.api.session.pushrules.SenderNotificationPermissionCondition import timber.log.Timber From 144d6c99a6b46989419abe2443e125a356746c05 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 22:07:52 +0200 Subject: [PATCH 6/9] Fix rule parameter not needed anymore --- .../android/sdk/api/session/pushrules/rest/PushCondition.kt | 2 +- .../android/sdk/internal/session/pushrules/PushRuleFinder.kt | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt index 0e175b47b9..1b53801d0b 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/rest/PushCondition.kt @@ -58,7 +58,7 @@ data class PushCondition( val iz: String? = null ) { - fun asExecutableCondition(rule: PushRule): Condition? { + fun asExecutableCondition(): Condition? { return when (Kind.fromString(kind)) { Kind.EventMatch -> { if (key != null && pattern != null) { diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt index b9d06a934d..15e803bb91 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/internal/session/pushrules/PushRuleFinder.kt @@ -28,7 +28,7 @@ internal class PushRuleFinder @Inject constructor( return rules.firstOrNull { rule -> // All conditions must hold true for an event in order to apply the action for the event. rule.enabled && rule.conditions?.all { - it.asExecutableCondition(rule)?.isSatisfied(event, conditionResolver) ?: false + it.asExecutableCondition()?.isSatisfied(event, conditionResolver) ?: false } ?: false } } From f4f9851edd88a618c3a44f187f7cde29e4dc7c3b Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 22:21:22 +0200 Subject: [PATCH 7/9] Remove rule param also in files outside of the sdk --- .../java/im/vector/app/features/settings/push/PushRuleItem.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt b/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt index 5a1dd055bd..57b2595590 100644 --- a/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt +++ b/vector/src/main/java/im/vector/app/features/settings/push/PushRuleItem.kt @@ -73,7 +73,7 @@ abstract class PushRuleItem : VectorEpoxyModel(R.layout.ite pushRule.conditions?.forEachIndexed { i, condition -> if (i > 0) description.append("\n") description.append( - condition.asExecutableCondition(pushRule)?.technicalDescription() + condition.asExecutableCondition()?.technicalDescription() ?: "UNSUPPORTED" ) } From bc20ad5cf1614bc2478e3287c04a7e269d771065 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Mon, 4 Jul 2022 22:53:24 +0200 Subject: [PATCH 8/9] Fix tests still passing the word match bool Signed-off-by: Nicolas Werner --- .../session/pushrules/PushRulesConditionTest.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt index 7d64217843..3ddf940241 100644 --- a/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt +++ b/matrix-sdk-android/src/test/java/org/matrix/android/sdk/api/session/pushrules/PushRulesConditionTest.kt @@ -49,7 +49,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_eventmatch_type_condition() { - val condition = EventMatchCondition("type", "m.room.message", false) + val condition = EventMatchCondition("type", "m.room.message") val simpleTextEvent = createSimpleTextEvent("Yo wtf?") @@ -72,7 +72,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_eventmatch_path_condition() { - val condition = EventMatchCondition("content.msgtype", "m.text", false) + val condition = EventMatchCondition("content.msgtype", "m.text") val simpleTextEvent = createSimpleTextEvent("Yo wtf?") @@ -89,13 +89,13 @@ class PushRulesConditionTest : MatrixTest { ).toContent(), originServerTs = 0 ).apply { - assert(EventMatchCondition("content.membership", "invite", false).isSatisfied(this)) + assert(EventMatchCondition("content.membership", "invite").isSatisfied(this)) } } @Test fun test_eventmatch_cake_condition() { - val condition = EventMatchCondition("content.body", "cake", false) + val condition = EventMatchCondition("content.body", "cake") assert(condition.isSatisfied(createSimpleTextEvent("How was the cake?"))) assertFalse(condition.isSatisfied(createSimpleTextEvent("Howwasthecake?"))) @@ -103,7 +103,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_eventmatch_cakelie_condition() { - val condition = EventMatchCondition("content.body", "cake*lie", false) + val condition = EventMatchCondition("content.body", "cake*lie") assert(condition.isSatisfied(createSimpleTextEvent("How was the cakeisalie?"))) assertFalse(condition.isSatisfied(createSimpleTextEvent("How was the notcakeisalie?"))) @@ -111,7 +111,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_eventmatch_words_only_condition() { - val condition = EventMatchCondition("content.body", "ben", true) + val condition = EventMatchCondition("content.body", "ben") assertFalse(condition.isSatisfied(createSimpleTextEvent("benoit"))) assertFalse(condition.isSatisfied(createSimpleTextEvent("Hello benoit"))) @@ -127,7 +127,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_eventmatch_at_room_condition() { - val condition = EventMatchCondition("content.body", "@room", true) + val condition = EventMatchCondition("content.body", "@room") assertFalse(condition.isSatisfied(createSimpleTextEvent("@roomba"))) assertFalse(condition.isSatisfied(createSimpleTextEvent("room benoit"))) @@ -142,7 +142,7 @@ class PushRulesConditionTest : MatrixTest { @Test fun test_notice_condition() { - val conditionEqual = EventMatchCondition("content.msgtype", "m.notice", false) + val conditionEqual = EventMatchCondition("content.msgtype", "m.notice") Event( type = "m.room.message", From 4a383523e5d4d782ab8f8d8214f742c5c66919f8 Mon Sep 17 00:00:00 2001 From: Nicolas Werner Date: Thu, 21 Jul 2022 12:18:55 +0200 Subject: [PATCH 9/9] Bring back the body match optimization Signed-off-by: Nicolas Werner --- .../sdk/api/session/pushrules/EventMatchCondition.kt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt index e5448322e6..15d5cd3153 100644 --- a/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt +++ b/matrix-sdk-android/src/main/java/org/matrix/android/sdk/api/session/pushrules/EventMatchCondition.kt @@ -54,7 +54,14 @@ class EventMatchCondition( // word boundary. return try { if (key == "content.body") { - val regex = Regex("(\\W|^)" + pattern.simpleGlobToRegExp() + "(\\W|$)", setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) + val modPattern = if (pattern.startsWith("*") && pattern.endsWith("*")) { + // Regex.containsMatchIn() is way faster without leading and trailing + // stars, that don't make any difference for the evaluation result + pattern.removePrefix("*").removeSuffix("*").simpleGlobToRegExp() + } else { + "(\\W|^)" + pattern.simpleGlobToRegExp() + "(\\W|$)" + } + val regex = Regex(modPattern, setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE)) regex.containsMatchIn(value) } else { val regex = Regex(pattern.simpleGlobToRegExp(), setOf(RegexOption.DOT_MATCHES_ALL, RegexOption.IGNORE_CASE))