From 9356656e67ba6ed23f8f43d03fedaf559241aa84 Mon Sep 17 00:00:00 2001
From: Patrick Cloke <clokep@users.noreply.github.com>
Date: Wed, 2 Sep 2020 07:59:39 -0400
Subject: Do not try to store invalid data in the stats table (#8226)

---
 changelog.d/8226.bugfix                 |  1 +
 synapse/storage/databases/main/stats.py | 34 +++++++++++++++++++++++++--------
 2 files changed, 27 insertions(+), 8 deletions(-)
 create mode 100644 changelog.d/8226.bugfix

diff --git a/changelog.d/8226.bugfix b/changelog.d/8226.bugfix
new file mode 100644
index 0000000000..60bdff576d
--- /dev/null
+++ b/changelog.d/8226.bugfix
@@ -0,0 +1 @@
+Fix a longstanding bug where stats updates could break when unexpected profile data was included in events.
diff --git a/synapse/storage/databases/main/stats.py b/synapse/storage/databases/main/stats.py
index 9b9bc304a8..55a250ef06 100644
--- a/synapse/storage/databases/main/stats.py
+++ b/synapse/storage/databases/main/stats.py
@@ -224,14 +224,32 @@ class StatsStore(StateDeltasStore):
         )
 
     async def update_room_state(self, room_id: str, fields: Dict[str, Any]) -> None:
-        """
+        """Update the state of a room.
+
+        fields can contain the following keys with string values:
+        * join_rules
+        * history_visibility
+        * encryption
+        * name
+        * topic
+        * avatar
+        * canonical_alias
+
+        A is_federatable key can also be included with a boolean value.
+
         Args:
-            room_id
-            fields
+            room_id: The room ID to update the state of.
+            fields: The fields to update. This can include a partial list of the
+                above fields to only update some room information.
         """
-
-        # For whatever reason some of the fields may contain null bytes, which
-        # postgres isn't a fan of, so we replace those fields with null.
+        # Ensure that the values to update are valid, they should be strings and
+        # not contain any null bytes.
+        #
+        # Invalid data gets overwritten with null.
+        #
+        # Note that a missing value should not be overwritten (it keeps the
+        # previous value).
+        sentinel = object()
         for col in (
             "join_rules",
             "history_visibility",
@@ -241,8 +259,8 @@ class StatsStore(StateDeltasStore):
             "avatar",
             "canonical_alias",
         ):
-            field = fields.get(col)
-            if field and "\0" in field:
+            field = fields.get(col, sentinel)
+            if field is not sentinel and (not isinstance(field, str) or "\0" in field):
                 fields[col] = None
 
         await self.db_pool.simple_upsert(
-- 
cgit 1.4.1