From c22314c4e8a3e9637810d78508bfe15dcdfe50b6 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Wed, 18 May 2022 11:28:14 +0100 Subject: Discard null-containing strings before updating the user directory (#12762) --- changelog.d/12762.misc | 1 + synapse/rest/client/room.py | 4 ++-- synapse/storage/databases/main/events.py | 4 +--- synapse/storage/databases/main/user_directory.py | 9 ++++---- synapse/util/stringutils.py | 10 ++++++++- tests/handlers/test_user_directory.py | 28 ++++++++++++++++++++++++ 6 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 changelog.d/12762.misc diff --git a/changelog.d/12762.misc b/changelog.d/12762.misc new file mode 100644 index 0000000000..990fb6fe74 --- /dev/null +++ b/changelog.d/12762.misc @@ -0,0 +1 @@ +Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. diff --git a/synapse/rest/client/room.py b/synapse/rest/client/room.py index 906fe09e97..12fed856a8 100644 --- a/synapse/rest/client/room.py +++ b/synapse/rest/client/room.py @@ -109,10 +109,10 @@ class RoomStateEventRestServlet(TransactionRestServlet): self.auth = hs.get_auth() def register(self, http_server: HttpServer) -> None: - # /room/$roomid/state/$eventtype + # /rooms/$roomid/state/$eventtype no_state_key = "/rooms/(?P[^/]*)/state/(?P[^/]*)$" - # /room/$roomid/state/$eventtype/$statekey + # /rooms/$roomid/state/$eventtype/$statekey state_key = ( "/rooms/(?P[^/]*)/state/" "(?P[^/]*)/(?P[^/]*)$" diff --git a/synapse/storage/databases/main/events.py b/synapse/storage/databases/main/events.py index ed29a0a5e2..2c86a870cf 100644 --- a/synapse/storage/databases/main/events.py +++ b/synapse/storage/databases/main/events.py @@ -53,6 +53,7 @@ from synapse.storage.util.sequence import SequenceGenerator from synapse.types import StateMap, get_domain_from_id from synapse.util import json_encoder from synapse.util.iterutils import batch_iter, sorted_topologically +from synapse.util.stringutils import non_null_str_or_none if TYPE_CHECKING: from synapse.server import HomeServer @@ -1737,9 +1738,6 @@ class PersistEventsStore: not affect the current local state. """ - 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, table="room_memberships", diff --git a/synapse/storage/databases/main/user_directory.py b/synapse/storage/databases/main/user_directory.py index df772d4721..028db69af3 100644 --- a/synapse/storage/databases/main/user_directory.py +++ b/synapse/storage/databases/main/user_directory.py @@ -29,6 +29,7 @@ from typing import ( from typing_extensions import TypedDict from synapse.api.errors import StoreError +from synapse.util.stringutils import non_null_str_or_none if TYPE_CHECKING: from synapse.server import HomeServer @@ -469,11 +470,9 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore): """ Update or add a user's profile in the user directory. """ - # If the display name or avatar URL are unexpected types, overwrite them. - if not isinstance(display_name, str): - display_name = None - if not isinstance(avatar_url, str): - avatar_url = None + # If the display name or avatar URL are unexpected types, replace with None. + display_name = non_null_str_or_none(display_name) + avatar_url = non_null_str_or_none(avatar_url) def _update_profile_in_user_dir_txn(txn: LoggingTransaction) -> None: self.db_pool.simple_upsert_txn( diff --git a/synapse/util/stringutils.py b/synapse/util/stringutils.py index b26546aecd..27a363d7e5 100644 --- a/synapse/util/stringutils.py +++ b/synapse/util/stringutils.py @@ -16,7 +16,7 @@ import itertools import re import secrets import string -from typing import Iterable, Optional, Tuple +from typing import Any, Iterable, Optional, Tuple from netaddr import valid_ipv6 @@ -247,3 +247,11 @@ def base62_encode(num: int, minwidth: int = 1) -> str: # pad to minimum width pad = "0" * (minwidth - len(res)) return pad + res + + +def non_null_str_or_none(val: Any) -> Optional[str]: + """Check that the arg is a string containing no null (U+0000) codepoints. + + If so, returns the given string unmodified; otherwise, returns None. + """ + return val if isinstance(val, str) and "\u0000" not in val else None diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 96e2e3039b..4d658d29ca 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -1007,6 +1007,34 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): self.assertEqual(in_public, {(bob, room1), (bob, room2)}) self.assertEqual(in_private, set()) + def test_ignore_display_names_with_null_codepoints(self) -> None: + MXC_DUMMY = "mxc://dummy" + + # Alice creates a public room. + alice = self.register_user("alice", "pass") + + # Alice has a user directory entry to start with. + self.assertIn( + alice, + self.get_success(self.user_dir_helper.get_profiles_in_user_directory()), + ) + + # Alice changes her name to include a null codepoint. + self.get_success( + self.hs.get_user_directory_handler().handle_local_profile_change( + alice, + ProfileInfo( + display_name="abcd\u0000efgh", + avatar_url=MXC_DUMMY, + ), + ) + ) + # Alice's profile should be updated with the new avatar, but no display name. + self.assertEqual( + self.get_success(self.user_dir_helper.get_profiles_in_user_directory()), + {alice: ProfileInfo(display_name=None, avatar_url=MXC_DUMMY)}, + ) + class TestUserDirSearchDisabled(unittest.HomeserverTestCase): servlets = [ -- cgit 1.4.1 From 1aa30f7b3e7767d2845858f37427282710ee94cc Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 18 May 2022 11:41:53 +0100 Subject: 1.59.1 --- CHANGES.md | 9 +++++++++ changelog.d/12762.misc | 1 - debian/changelog | 6 ++++++ pyproject.toml | 2 +- 4 files changed, 16 insertions(+), 2 deletions(-) delete mode 100644 changelog.d/12762.misc diff --git a/CHANGES.md b/CHANGES.md index 56d1a5a7d7..115f4b35ff 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,12 @@ +Synapse 1.59.1 (2022-05-18) +=========================== + +Internal Changes +---------------- + +- Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. ([\#12762](https://github.com/matrix-org/synapse/issues/12762)) + + Synapse 1.59.0 (2022-05-17) =========================== diff --git a/changelog.d/12762.misc b/changelog.d/12762.misc deleted file mode 100644 index 990fb6fe74..0000000000 --- a/changelog.d/12762.misc +++ /dev/null @@ -1 +0,0 @@ -Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. diff --git a/debian/changelog b/debian/changelog index cc6152d5b3..dda342a630 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (1.59.1) stable; urgency=medium + + * New Synapse release 1.59.1. + + -- Synapse Packaging team Wed, 18 May 2022 11:41:46 +0100 + matrix-synapse-py3 (1.59.0) stable; urgency=medium * New Synapse release 1.59.0. diff --git a/pyproject.toml b/pyproject.toml index e600a1d52e..5a5a2eaba7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ skip_gitignore = true [tool.poetry] name = "matrix-synapse" -version = "1.59.0" +version = "1.59.1" description = "Homeserver for the Matrix decentralised comms protocol" authors = ["Matrix.org Team and Contributors "] license = "Apache-2.0" -- cgit 1.4.1 From d24a1486e5c5d69a8798a9d159fd9e06dfc8c3e3 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 18 May 2022 11:46:05 +0100 Subject: Fixup changelog --- CHANGES.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index 115f4b35ff..e10ac0314a 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,10 +1,12 @@ Synapse 1.59.1 (2022-05-18) =========================== -Internal Changes +This release fixes a long-standing issue which could prevent Synapse's user directory for updating properly. + +Bugfixes ---------------- -- Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. ([\#12762](https://github.com/matrix-org/synapse/issues/12762)) +- Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. Contributed by Nick @ Beeper. ([\#12762](https://github.com/matrix-org/synapse/issues/12762)) Synapse 1.59.0 (2022-05-17) -- cgit 1.4.1