From 24407852fd2495780e2405ec3a3d1014eb1b63e3 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Mar 2023 11:59:35 -0500 Subject: [PATCH 1/2] Stabilize support for MSC3873: disambuguated event push keys. --- rust/src/push/base_rules.rs | 6 ++--- synapse/config/experimental.py | 5 ---- synapse/push/bulk_push_rule_evaluator.py | 33 ++++++------------------ tests/push/test_push_rule_evaluator.py | 10 +++---- 4 files changed, 14 insertions(+), 40 deletions(-) diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs index 3d72a4a4c36e..eb20d8a440f7 100644 --- a/rust/src/push/base_rules.rs +++ b/rust/src/push/base_rules.rs @@ -71,7 +71,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ priority_class: 5, conditions: Cow::Borrowed(&[Condition::Known(KnownCondition::EventMatch( EventMatchCondition { - key: Cow::Borrowed("content.m.relates_to.rel_type"), + key: Cow::Borrowed("content.m\\.relates_to.rel_type"), pattern: Cow::Borrowed("m.replace"), }, ))]), @@ -146,7 +146,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ priority_class: 5, conditions: Cow::Borrowed(&[Condition::Known( KnownCondition::ExactEventPropertyContainsType(ExactEventMatchTypeCondition { - key: Cow::Borrowed("content.org.matrix.msc3952.mentions.user_ids"), + key: Cow::Borrowed("content.org\\.matrix\\.msc3952\\.mentions.user_ids"), value_type: Cow::Borrowed(&EventMatchPatternType::UserId), }), )]), @@ -167,7 +167,7 @@ pub const BASE_APPEND_OVERRIDE_RULES: &[PushRule] = &[ priority_class: 5, conditions: Cow::Borrowed(&[ Condition::Known(KnownCondition::ExactEventMatch(ExactEventMatchCondition { - key: Cow::Borrowed("content.org.matrix.msc3952.mentions.room"), + key: Cow::Borrowed("content.org\\.matrix\\.msc3952\\.mentions.room"), value: Cow::Borrowed(&SimpleJsonValue::Bool(true)), })), Condition::Known(KnownCondition::SenderNotificationPermission { diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index fc64f2bda11d..c536a9f50f34 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -174,11 +174,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: "msc3758_exact_event_match", False ) - # MSC3873: Disambiguate event_match keys. - self.msc3873_escape_event_match_key = experimental.get( - "msc3873_escape_event_match_key", False - ) - # MSC3966: exact_event_property_contains push rule condition. self.msc3966_exact_event_property_contains = experimental.get( "msc3966_exact_event_property_contains", False diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py index abcf687f0528..1cb36742d745 100644 --- a/synapse/push/bulk_push_rule_evaluator.py +++ b/synapse/push/bulk_push_rule_evaluator.py @@ -273,10 +273,7 @@ async def _related_events( related_event_id, allow_none=True ) if related_event is not None: - related_events[relation_type] = _flatten_dict( - related_event, - msc3873_escape_event_match_key=self.hs.config.experimental.msc3873_escape_event_match_key, - ) + related_events[relation_type] = _flatten_dict(related_event) reply_event_id = ( event.content.get("m.relates_to", {}) @@ -291,10 +288,7 @@ async def _related_events( ) if related_event is not None: - related_events["m.in_reply_to"] = _flatten_dict( - related_event, - msc3873_escape_event_match_key=self.hs.config.experimental.msc3873_escape_event_match_key, - ) + related_events["m.in_reply_to"] = _flatten_dict(related_event) # indicate that this is from a fallback relation. if relation_type == "m.thread" and event.content.get( @@ -401,10 +395,7 @@ async def _action_for_event_by_user( ) evaluator = PushRuleEvaluator( - _flatten_dict( - event, - msc3873_escape_event_match_key=self.hs.config.experimental.msc3873_escape_event_match_key, - ), + _flatten_dict(event), has_mentions, room_member_count, sender_power_level, @@ -496,8 +487,6 @@ def _flatten_dict( d: Union[EventBase, Mapping[str, Any]], prefix: Optional[List[str]] = None, result: Optional[Dict[str, JsonValue]] = None, - *, - msc3873_escape_event_match_key: bool = False, ) -> Dict[str, JsonValue]: """ Given a JSON dictionary (or event) which might contain sub dictionaries, @@ -526,11 +515,10 @@ def _flatten_dict( if result is None: result = {} for key, value in d.items(): - if msc3873_escape_event_match_key: - # Escape periods in the key with a backslash (and backslashes with an - # extra backslash). This is since a period is used as a separator between - # nested fields. - key = key.replace("\\", "\\\\").replace(".", "\\.") + # Escape periods in the key with a backslash (and backslashes with an + # extra backslash). This is since a period is used as a separator between + # nested fields. + key = key.replace("\\", "\\\\").replace(".", "\\.") if _is_simple_value(value): result[".".join(prefix + [key])] = value @@ -538,12 +526,7 @@ def _flatten_dict( result[".".join(prefix + [key])] = [v for v in value if _is_simple_value(v)] elif isinstance(value, Mapping): # do not set `room_version` due to recursion considerations below - _flatten_dict( - value, - prefix=(prefix + [key]), - result=result, - msc3873_escape_event_match_key=msc3873_escape_event_match_key, - ) + _flatten_dict(value, prefix=(prefix + [key]), result=result) # `room_version` should only ever be set when looking at the top level of an event if ( diff --git a/tests/push/test_push_rule_evaluator.py b/tests/push/test_push_rule_evaluator.py index d4a4bc4d93cb..b993a3106f95 100644 --- a/tests/push/test_push_rule_evaluator.py +++ b/tests/push/test_push_rule_evaluator.py @@ -51,11 +51,7 @@ def test_nested(self) -> None: # If a field has a dot in it, escape it. input = {"m.foo": {"b\\ar": "abc"}} - self.assertEqual({"m.foo.b\\ar": "abc"}, _flatten_dict(input)) - self.assertEqual( - {"m\\.foo.b\\\\ar": "abc"}, - _flatten_dict(input, msc3873_escape_event_match_key=True), - ) + self.assertEqual({"m\\.foo.b\\\\ar": "abc"}, _flatten_dict(input)) def test_non_string(self) -> None: """String, booleans, ints, nulls and list of those should be kept while other items are dropped.""" @@ -125,7 +121,7 @@ def test_extensible_events(self) -> None: "room_id": "!test:test", "sender": "@alice:test", "type": "m.room.message", - "content.org.matrix.msc1767.markup": [], + "content.org\\.matrix\\.msc1767\\.markup": [], } self.assertEqual(expected, _flatten_dict(event)) @@ -137,7 +133,7 @@ def test_extensible_events(self) -> None: "room_id": "!test:test", "sender": "@alice:test", "type": "m.room.message", - "content.org.matrix.msc1767.markup": [], + "content.org\\.matrix\\.msc1767\\.markup": [], } self.assertEqual(expected, _flatten_dict(event)) From 70619f05a5bfffeadfd899f368be5f8fdb546378 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 2 Mar 2023 12:02:27 -0500 Subject: [PATCH 2/2] Newsfragment --- changelog.d/15190.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15190.bugfix diff --git a/changelog.d/15190.bugfix b/changelog.d/15190.bugfix new file mode 100644 index 000000000000..5c3a86320ec8 --- /dev/null +++ b/changelog.d/15190.bugfix @@ -0,0 +1 @@ +Implement [MSC3873](https://github.com/matrix-org/matrix-spec-proposals/pull/3873) to fix a long-standing bug where properties with dots were handled ambiguously in push rules.