Compare commits

...

7 Commits

Author SHA1 Message Date
David Robertson
0953cad3e4 docstringggggg 2022-05-18 10:19:30 +01:00
David Robertson
ad6a6675bf go away flake8 2022-05-17 14:14:40 +01:00
David Robertson
21d1347f2c Changelog 2022-05-17 14:00:56 +01:00
David Robertson
a9fe3350f8 Drive-by typo fix I spotted while debugging 2022-05-17 13:55:58 +01:00
David Robertson
a1adede444 discard strings with nulls before user dir update 2022-05-17 13:55:58 +01:00
David Robertson
79f1cef5e4 Move non_null_str_or_none to s.utils.stringutils 2022-05-17 13:55:58 +01:00
David Robertson
8c977edec8 Reproduce #12755 2022-05-17 13:55:44 +01:00
6 changed files with 45 additions and 11 deletions

1
changelog.d/12762.misc Normal file
View File

@@ -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.

View File

@@ -109,10 +109,10 @@ class RoomStateEventRestServlet(TransactionRestServlet):
self.auth = hs.get_auth() self.auth = hs.get_auth()
def register(self, http_server: HttpServer) -> None: def register(self, http_server: HttpServer) -> None:
# /room/$roomid/state/$eventtype # /rooms/$roomid/state/$eventtype
no_state_key = "/rooms/(?P<room_id>[^/]*)/state/(?P<event_type>[^/]*)$" no_state_key = "/rooms/(?P<room_id>[^/]*)/state/(?P<event_type>[^/]*)$"
# /room/$roomid/state/$eventtype/$statekey # /rooms/$roomid/state/$eventtype/$statekey
state_key = ( state_key = (
"/rooms/(?P<room_id>[^/]*)/state/" "/rooms/(?P<room_id>[^/]*)/state/"
"(?P<event_type>[^/]*)/(?P<state_key>[^/]*)$" "(?P<event_type>[^/]*)/(?P<state_key>[^/]*)$"

View File

@@ -52,6 +52,7 @@ from synapse.storage.util.sequence import SequenceGenerator
from synapse.types import JsonDict, StateMap, get_domain_from_id from synapse.types import JsonDict, StateMap, get_domain_from_id
from synapse.util import json_encoder from synapse.util import json_encoder
from synapse.util.iterutils import batch_iter, sorted_topologically from synapse.util.iterutils import batch_iter, sorted_topologically
from synapse.util.stringutils import non_null_str_or_none
if TYPE_CHECKING: if TYPE_CHECKING:
from synapse.server import HomeServer from synapse.server import HomeServer
@@ -1728,9 +1729,6 @@ class PersistEventsStore:
not affect the current local state. 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( self.db_pool.simple_insert_many_txn(
txn, txn,
table="room_memberships", table="room_memberships",

View File

@@ -29,6 +29,7 @@ from typing import (
from typing_extensions import TypedDict from typing_extensions import TypedDict
from synapse.api.errors import StoreError from synapse.api.errors import StoreError
from synapse.util.stringutils import non_null_str_or_none
if TYPE_CHECKING: if TYPE_CHECKING:
from synapse.server import HomeServer from synapse.server import HomeServer
@@ -469,11 +470,9 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
""" """
Update or add a user's profile in the user directory. Update or add a user's profile in the user directory.
""" """
# If the display name or avatar URL are unexpected types, overwrite them. # If the display name or avatar URL are unexpected types, replace with None.
if not isinstance(display_name, str): display_name = non_null_str_or_none(display_name)
display_name = None avatar_url = non_null_str_or_none(avatar_url)
if not isinstance(avatar_url, str):
avatar_url = None
def _update_profile_in_user_dir_txn(txn: LoggingTransaction) -> None: def _update_profile_in_user_dir_txn(txn: LoggingTransaction) -> None:
self.db_pool.simple_upsert_txn( self.db_pool.simple_upsert_txn(

View File

@@ -16,7 +16,7 @@ import itertools
import re import re
import secrets import secrets
import string import string
from typing import Iterable, Optional, Tuple from typing import Any, Iterable, Optional, Tuple
from netaddr import valid_ipv6 from netaddr import valid_ipv6
@@ -247,3 +247,11 @@ def base62_encode(num: int, minwidth: int = 1) -> str:
# pad to minimum width # pad to minimum width
pad = "0" * (minwidth - len(res)) pad = "0" * (minwidth - len(res))
return pad + 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

View File

@@ -1007,6 +1007,34 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
self.assertEqual(in_public, {(bob, room1), (bob, room2)}) self.assertEqual(in_public, {(bob, room1), (bob, room2)})
self.assertEqual(in_private, set()) 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): class TestUserDirSearchDisabled(unittest.HomeserverTestCase):
servlets = [ servlets = [