Ensure the body is a string before comparing push rules. (#7701)

changelog.d/7701.bugfix

+Do not break push rule evaluation when receiving an event with a non-string body. This is a long-standing bug.

synapse/push/push_rule_evaluator.py

@@ -131,7 +131,7 @@ class PushRuleEvaluatorForEvent(object):
         # XXX: optimisation: cache our pattern regexps
         if condition["key"] == "content.body":
             body = self._event.content.get("body", None)
-            if not body:
+            if not body or not isinstance(body, str):
                 return False
 
             return _glob_matches(pattern, body, word_boundary=True)
@@ -147,7 +147,7 @@ class PushRuleEvaluatorForEvent(object):
             return False
 
         body = self._event.content.get("body", None)
-        if not body:
+        if not body or not isinstance(body, str):
             return False
 
         # Similar to _glob_matches, but do not treat display_name as a glob.

tests/push/test_push_rule_evaluator.py

@@ -21,7 +21,7 @@ from tests import unittest
 
 
 class PushRuleEvaluatorTestCase(unittest.TestCase):
-    def setUp(self):
+    def _get_evaluator(self, content):
         event = FrozenEvent(
             {
                 "event_id": "$event_id",
@@ -29,37 +29,58 @@ class PushRuleEvaluatorTestCase(unittest.TestCase):
                 "sender": "@user:test",
                 "state_key": "",
                 "room_id": "@room:test",
-                "content": {"body": "foo bar baz"},
+                "content": content,
             },
             RoomVersions.V1,
         )
         room_member_count = 0
         sender_power_level = 0
         power_levels = {}
-        self.evaluator = PushRuleEvaluatorForEvent(
+        return PushRuleEvaluatorForEvent(
             event, room_member_count, sender_power_level, power_levels
         )
 
     def test_display_name(self):
         """Check for a matching display name in the body of the event."""
+        evaluator = self._get_evaluator({"body": "foo bar baz"})
+
         condition = {
             "kind": "contains_display_name",
         }
 
         # Blank names are skipped.
-        self.assertFalse(self.evaluator.matches(condition, "@user:test", ""))
+        self.assertFalse(evaluator.matches(condition, "@user:test", ""))
 
         # Check a display name that doesn't match.
-        self.assertFalse(self.evaluator.matches(condition, "@user:test", "not found"))
+        self.assertFalse(evaluator.matches(condition, "@user:test", "not found"))
 
         # Check a display name which matches.
-        self.assertTrue(self.evaluator.matches(condition, "@user:test", "foo"))
+        self.assertTrue(evaluator.matches(condition, "@user:test", "foo"))
 
         # A display name that matches, but not a full word does not result in a match.
-        self.assertFalse(self.evaluator.matches(condition, "@user:test", "ba"))
+        self.assertFalse(evaluator.matches(condition, "@user:test", "ba"))
 
         # A display name should not be interpreted as a regular expression.
-        self.assertFalse(self.evaluator.matches(condition, "@user:test", "ba[rz]"))
+        self.assertFalse(evaluator.matches(condition, "@user:test", "ba[rz]"))
 
         # A display name with spaces should work fine.
-        self.assertTrue(self.evaluator.matches(condition, "@user:test", "foo bar"))
+        self.assertTrue(evaluator.matches(condition, "@user:test", "foo bar"))
+
+    def test_no_body(self):
+        """Not having a body shouldn't break the evaluator."""
+        evaluator = self._get_evaluator({})
+
+        condition = {
+            "kind": "contains_display_name",
+        }
+        self.assertFalse(evaluator.matches(condition, "@user:test", "foo"))
+
+    def test_invalid_body(self):
+        """A non-string body should not break the evaluator."""
+        condition = {
+            "kind": "contains_display_name",
+        }
+
+        for body in (1, True, {"foo": "bar"}):
+            evaluator = self._get_evaluator({"body": body})
+            self.assertFalse(evaluator.matches(condition, "@user:test", "foo"))