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

2 files changed, 53 insertions, 22 deletions

diff --git a/synapse/events/utils.py b/synapse/events/utils.py
index 026dcde8d8..ac91c5eb57 100644
--- a/synapse/events/utils.py
+++ b/synapse/events/utils.py
@@ -22,6 +22,7 @@ from typing import (
     Iterable,
     List,
     Mapping,
+    MutableMapping,
     Optional,
     Union,
 )
@@ -580,10 +581,20 @@ class EventClientSerializer:
         ]
 
 
-def copy_power_levels_contents(
-    old_power_levels: Mapping[str, Union[int, Mapping[str, int]]]
+_PowerLevel = Union[str, int]
+
+
+def copy_and_fixup_power_levels_contents(
+    old_power_levels: Mapping[str, Union[_PowerLevel, Mapping[str, _PowerLevel]]]
 ) -> Dict[str, Union[int, Dict[str, int]]]:
-    """Copy the content of a power_levels event, unfreezing frozendicts along the way
+    """Copy the content of a power_levels event, unfreezing frozendicts along the way.
+
+    We accept as input power level values which are strings, provided they represent an
+    integer, e.g. `"`100"` instead of 100. Such strings are converted to integers
+    in the returned dictionary (hence "fixup" in the function name).
+
+    Note that future room versions will outlaw such stringy power levels (see
+    https://github.com/matrix-org/matrix-spec/issues/853).
 
     Raises:
         TypeError if the input does not look like a valid power levels event content
@@ -592,29 +603,47 @@ def copy_power_levels_contents(
         raise TypeError("Not a valid power-levels content: %r" % (old_power_levels,))
 
     power_levels: Dict[str, Union[int, Dict[str, int]]] = {}
-    for k, v in old_power_levels.items():
-
-        if isinstance(v, int):
-            power_levels[k] = v
-            continue
 
+    for k, v in old_power_levels.items():
         if isinstance(v, collections.abc.Mapping):
             h: Dict[str, int] = {}
             power_levels[k] = h
             for k1, v1 in v.items():
-                # we should only have one level of nesting
-                if not isinstance(v1, int):
-                    raise TypeError(
-                        "Invalid power_levels value for %s.%s: %r" % (k, k1, v1)
-                    )
-                h[k1] = v1
-            continue
+                _copy_power_level_value_as_integer(v1, h, k1)
 
-        raise TypeError("Invalid power_levels value for %s: %r" % (k, v))
+        else:
+            _copy_power_level_value_as_integer(v, power_levels, k)
 
     return power_levels
 
 
+def _copy_power_level_value_as_integer(
+    old_value: object,
+    power_levels: MutableMapping[str, Any],
+    key: str,
+) -> None:
+    """Set `power_levels[key]` to the integer represented by `old_value`.
+
+    :raises TypeError: if `old_value` is not an integer, nor a base-10 string
+        representation of an integer.
+    """
+    if isinstance(old_value, int):
+        power_levels[key] = old_value
+        return
+
+    if isinstance(old_value, str):
+        try:
+            parsed_value = int(old_value, base=10)
+        except ValueError:
+            # Fall through to the final TypeError.
+            pass
+        else:
+            power_levels[key] = parsed_value
+            return
+
+    raise TypeError(f"Invalid power_levels value for {key}: {old_value}")
+
+
 def validate_canonicaljson(value: Any) -> None:
     """
     Ensure that the JSON object is valid according to the rules of canonical JSON.
diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py
index b31f00b517..604eb6ec15 100644
--- a/synapse/handlers/room.py
+++ b/synapse/handlers/room.py
@@ -57,7 +57,7 @@ from synapse.api.filtering import Filter
 from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion
 from synapse.event_auth import validate_event_for_room_version
 from synapse.events import EventBase
-from synapse.events.utils import copy_power_levels_contents
+from synapse.events.utils import copy_and_fixup_power_levels_contents
 from synapse.federation.federation_client import InvalidResponseError
 from synapse.handlers.federation import get_domains_from_state
 from synapse.handlers.relations import BundledAggregations
@@ -337,13 +337,13 @@ class RoomCreationHandler:
         # 50, but if the default PL in a room is 50 or more, then we set the
         # required PL above that.
 
-        pl_content = dict(old_room_pl_state.content)
-        users_default = int(pl_content.get("users_default", 0))
+        pl_content = copy_and_fixup_power_levels_contents(old_room_pl_state.content)
+        users_default: int = pl_content.get("users_default", 0)  # type: ignore[assignment]
         restricted_level = max(users_default + 1, 50)
 
         updated = False
         for v in ("invite", "events_default"):
-            current = int(pl_content.get(v, 0))
+            current: int = pl_content.get(v, 0)  # type: ignore[assignment]
             if current < restricted_level:
                 logger.debug(
                     "Setting level for %s in %s to %i (was %i)",
@@ -380,7 +380,9 @@ class RoomCreationHandler:
                 "state_key": "",
                 "room_id": new_room_id,
                 "sender": requester.user.to_string(),
-                "content": old_room_pl_state.content,
+                "content": copy_and_fixup_power_levels_contents(
+                    old_room_pl_state.content
+                ),
             },
             ratelimit=False,
         )
@@ -471,7 +473,7 @@ class RoomCreationHandler:
         # dict so we can't just copy.deepcopy it.
         initial_state[
             (EventTypes.PowerLevels, "")
-        ] = power_levels = copy_power_levels_contents(
+        ] = power_levels = copy_and_fixup_power_levels_contents(
             initial_state[(EventTypes.PowerLevels, "")]
         )