diff --git a/changelog.d/11230.bugfix b/changelog.d/11230.bugfix
new file mode 100644
index 0000000000..b2d6d4d024
--- /dev/null
+++ b/changelog.d/11230.bugfix
@@ -0,0 +1,2 @@
+Fix a long-standing bug wherein display names or avatar URLs containing null bytes cause an internal server error
+when stored in the DB.
\ No newline at end of file
diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py
index 596275c23c..120e4807d1 100644
--- a/synapse/storage/databases/main/events.py
+++ b/synapse/storage/databases/main/events.py
@@ -1641,8 +1641,8 @@ class PersistEventsStore:
def _store_room_members_txn(self, txn, events, backfilled):
"""Store a room member in the database."""
- def str_or_none(val: Any) -> Optional[str]:
- return val if isinstance(val, str) else None
+ def non_null_str_or_none(val: Any) -> Optional[str]:
+ return val if isinstance(val, str) and "\u0000" not in val else None
self.db_pool.simple_insert_many_txn(
txn,
@@ -1654,8 +1654,10 @@ class PersistEventsStore:
"sender": event.user_id,
"room_id": event.room_id,
"membership": event.membership,
- "display_name": str_or_none(event.content.get("displayname")),
- "avatar_url": str_or_none(event.content.get("avatar_url")),
+ "display_name": non_null_str_or_none(
+ event.content.get("displayname")
+ ),
+ "avatar_url": non_null_str_or_none(event.content.get("avatar_url")),
}
for event in events
],
diff --git a/tests/storage/test_roommember.py b/tests/storage/test_roommember.py
index 2873e22ccf..fccab733c0 100644
--- a/tests/storage/test_roommember.py
+++ b/tests/storage/test_roommember.py
@@ -161,6 +161,54 @@ class RoomMemberStoreTestCase(unittest.HomeserverTestCase):
)
self.assertEqual(users.keys(), {self.u_alice, self.u_bob})
+ def test__null_byte_in_display_name_properly_handled(self):
+ room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)
+
+ res = self.get_success(
+ self.store.db_pool.simple_select_list(
+ "room_memberships",
+ {"user_id": "@alice:test"},
+ ["display_name", "event_id"],
+ )
+ )
+ # Check that we only got one result back
+ self.assertEqual(len(res), 1)
+
+ # Check that alice's display name is "alice"
+ self.assertEqual(res[0]["display_name"], "alice")
+
+ # Grab the event_id to use later
+ event_id = res[0]["event_id"]
+
+ # Create a profile with the offending null byte in the display name
+ new_profile = {"displayname": "ali\u0000ce"}
+
+ # Ensure that the change goes smoothly and does not fail due to the null byte
+ self.helper.change_membership(
+ room,
+ self.u_alice,
+ self.u_alice,
+ "join",
+ extra_data=new_profile,
+ tok=self.t_alice,
+ )
+
+ res2 = self.get_success(
+ self.store.db_pool.simple_select_list(
+ "room_memberships",
+ {"user_id": "@alice:test"},
+ ["display_name", "event_id"],
+ )
+ )
+ # Check that we only have two results
+ self.assertEqual(len(res2), 2)
+
+ # Filter out the previous event using the event_id we grabbed above
+ row = [row for row in res2 if row["event_id"] != event_id]
+
+ # Check that alice's display name is now None
+ self.assertEqual(row[0]["display_name"], None)
+
class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase):
def prepare(self, reactor, clock, homeserver):
|
