This removes the experimental configuration option and always escapes the push rule condition keys. Also escapes any (experimental) push rule condition keys in the base rules which contain dot in a field name.tags/v1.80.0rc1
@@ -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. |
@@ -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 { | |||
@@ -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) |
@@ -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 ( | |||
@@ -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)) | |||