Convert stringy power levels to integers on room upgrade (#12657)

2 files changed, 83 insertions, 2 deletions

diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py
index 00ad19e446..b1c47efac7 100644
--- a/tests/events/test_utils.py
+++ b/tests/events/test_utils.py
@@ -17,7 +17,7 @@ from synapse.api.room_versions import RoomVersions
 from synapse.events import make_event_from_dict
 from synapse.events.utils import (
     SerializeEventConfig,
-    copy_power_levels_contents,
+    copy_and_fixup_power_levels_contents,
     prune_event,
     serialize_event,
 )
@@ -529,7 +529,7 @@ class CopyPowerLevelsContentTestCase(unittest.TestCase):
         }
 
     def _test(self, input):
-        a = copy_power_levels_contents(input)
+        a = copy_and_fixup_power_levels_contents(input)
 
         self.assertEqual(a["ban"], 50)
         self.assertEqual(a["events"]["m.room.name"], 100)
@@ -547,3 +547,40 @@ class CopyPowerLevelsContentTestCase(unittest.TestCase):
     def test_frozen(self):
         input = freeze(self.test_content)
         self._test(input)
+
+    def test_stringy_integers(self):
+        """String representations of decimal integers are converted to integers."""
+        input = {
+            "a": "100",
+            "b": {
+                "foo": 99,
+                "bar": "-98",
+            },
+            "d": "0999",
+        }
+        output = copy_and_fixup_power_levels_contents(input)
+        expected_output = {
+            "a": 100,
+            "b": {
+                "foo": 99,
+                "bar": -98,
+            },
+            "d": 999,
+        }
+
+        self.assertEqual(output, expected_output)
+
+    def test_strings_that_dont_represent_decimal_integers(self) -> None:
+        """Should raise when given inputs `s` for which `int(s, base=10)` raises."""
+        for invalid_string in ["0x123", "123.0", "123.45", "hello", "0b1", "0o777"]:
+            with self.assertRaises(TypeError):
+                copy_and_fixup_power_levels_contents({"a": invalid_string})
+
+    def test_invalid_types_raise_type_error(self) -> None:
+        with self.assertRaises(TypeError):
+            copy_and_fixup_power_levels_contents({"a": ["hello", "grandma"]})  # type: ignore[arg-type]
+            copy_and_fixup_power_levels_contents({"a": None})  # type: ignore[arg-type]
+
+    def test_invalid_nesting_raises_type_error(self) -> None:
+        with self.assertRaises(TypeError):
+            copy_and_fixup_power_levels_contents({"a": {"b": {"c": 1}}})
diff --git a/tests/rest/client/test_upgrade_room.py b/tests/rest/client/test_upgrade_room.py
index b7d0f42daf..c86fc5df0d 100644
--- a/tests/rest/client/test_upgrade_room.py
+++ b/tests/rest/client/test_upgrade_room.py
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 from typing import Optional
+from unittest.mock import patch
 
 from twisted.test.proto_helpers import MemoryReactor
 
@@ -167,6 +168,49 @@ class UpgradeRoomTest(unittest.HomeserverTestCase):
         )
         self.assertNotIn(self.other, power_levels["users"])
 
+    def test_stringy_power_levels(self) -> None:
+        """The room upgrade converts stringy power levels to proper integers."""
+        # Retrieve the room's current power levels.
+        power_levels = self.helper.get_state(
+            self.room_id,
+            "m.room.power_levels",
+            tok=self.creator_token,
+        )
+
+        # Set creator's power level to the string "100" instead of the integer `100`.
+        power_levels["users"][self.creator] = "100"
+
+        # Synapse refuses to accept new stringy power level events. Bypass this by
+        # neutering the validation.
+        with patch("synapse.events.validator.jsonschema.validate"):
+            # Note: https://github.com/matrix-org/matrix-spec/issues/853 plans to forbid
+            # string power levels in new rooms. For this test to have a clean
+            # conscience, we ought to ensure it's upgrading from a sufficiently old
+            # version of room.
+            self.helper.send_state(
+                self.room_id,
+                "m.room.power_levels",
+                body=power_levels,
+                tok=self.creator_token,
+            )
+
+        # Upgrade the room. Check the homeserver reports success.
+        channel = self._upgrade_room()
+        self.assertEqual(200, channel.code, channel.result)
+
+        # Extract the new room ID.
+        new_room_id = channel.json_body["replacement_room"]
+
+        # Fetch the new room's power level event.
+        new_power_levels = self.helper.get_state(
+            new_room_id,
+            "m.room.power_levels",
+            tok=self.creator_token,
+        )
+
+        # We should now have an integer power level.
+        self.assertEqual(new_power_levels["users"][self.creator], 100, new_power_levels)
+
     def test_space(self) -> None:
         """Test upgrading a space."""