diff --git a/changelog.d/15190.bugfix b/changelog.d/15190.bugfix
new file mode 100644
index 0000000000..5c3a86320e
--- /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.
diff --git a/rust/src/push/base_rules.rs b/rust/src/push/base_rules.rs
index ec8d96656a..d7c73c1f25 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(EventPropertyIsTypeCondition {
- 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::EventPropertyIs(EventPropertyIsCondition {
- 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 9ff382ccc3..7e05f78f70 100644
--- a/synapse/config/experimental.py
+++ b/synapse/config/experimental.py
@@ -166,11 +166,6 @@ class ExperimentalConfig(Config):
# MSC3391: Removing account data.
self.msc3391_enabled = experimental.get("msc3391_enabled", False)
- # MSC3873: Disambiguate event_match keys.
- self.msc3873_escape_event_match_key = experimental.get(
- "msc3873_escape_event_match_key", False
- )
-
# MSC3952: Intentional mentions, this depends on MSC3966.
self.msc3952_intentional_mentions = experimental.get(
"msc3952_intentional_mentions", False
@@ -181,10 +176,5 @@ class ExperimentalConfig(Config):
"msc3958_supress_edit_notifs", False
)
- # MSC3966: exact_event_property_contains push rule condition.
- self.msc3966_exact_event_property_contains = experimental.get(
- "msc3966_exact_event_property_contains", False
- )
-
# MSC3967: Do not require UIA when first uploading cross signing keys
self.msc3967_enabled = experimental.get("msc3967_enabled", False)
diff --git a/synapse/push/bulk_push_rule_evaluator.py b/synapse/push/bulk_push_rule_evaluator.py
index 45622a9e9b..199337673f 100644
--- a/synapse/push/bulk_push_rule_evaluator.py
+++ b/synapse/push/bulk_push_rule_evaluator.py
@@ -273,10 +273,7 @@ class BulkPushRuleEvaluator:
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 @@ class BulkPushRuleEvaluator:
)
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 @@ class BulkPushRuleEvaluator:
)
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,
@@ -494,8 +485,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,
@@ -524,11 +513,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
@@ -536,12 +524,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 6deee0fd02..52c4aafea6 100644
--- a/tests/push/test_push_rule_evaluator.py
+++ b/tests/push/test_push_rule_evaluator.py
@@ -51,11 +51,7 @@ class FlattenDictTestCase(unittest.TestCase):
# 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 @@ class FlattenDictTestCase(unittest.TestCase):
"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 @@ class FlattenDictTestCase(unittest.TestCase):
"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))
|
