From 8bace9f97ba56386250bc5a477fefafb4096f5fb Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Thu, 25 Jun 2026 22:29:11 +0300 Subject: [PATCH] Fix some types in various places regarding profile field values I believe the correct type is `JsonValue | dict[str, JsonValue]`, except deep down in the displayname/avatar_url store methods. Removes a dubious cast by correctly casting in `get_profile_data_for_users` --- synapse/handlers/profile.py | 14 +++++++++--- synapse/handlers/sync.py | 22 ++++++++++-------- synapse/replication/http/profile.py | 4 ++-- synapse/rest/client/profile.py | 4 +++- synapse/storage/databases/main/profile.py | 27 ++++++++++++++++------- 5 files changed, 48 insertions(+), 23 deletions(-) diff --git a/synapse/handlers/profile.py b/synapse/handlers/profile.py index 4a47df2fdb..c9203b915f 100644 --- a/synapse/handlers/profile.py +++ b/synapse/handlers/profile.py @@ -632,7 +632,7 @@ class ProfileHandler: async def get_profile_field( self, target_user: UserID, field_name: str - ) -> JsonValue: + ) -> JsonValue | dict[str, JsonValue]: """ Fetch a user's profile from the database for local users and over federation for remote users. @@ -676,12 +676,16 @@ class ProfileHandler: target_user: UserID, requester: Requester, field_name: str, - new_value: str, + new_value: JsonValue | dict[str, JsonValue], by_admin: bool = False, propagate: bool = False, ) -> None: """Wrapper function for setting any profile field for a user.""" if field_name == ProfileFields.DISPLAYNAME: + if not isinstance(new_value, str): + raise SynapseError( + 400, "'displayname' must be a string", errcode=Codes.INVALID_PARAM + ) await self.set_displayname( target_user=target_user, requester=requester, @@ -690,6 +694,10 @@ class ProfileHandler: propagate=propagate, ) elif field_name == ProfileFields.AVATAR_URL: + if not isinstance(new_value, str): + raise SynapseError( + 400, "'avatar_url' must be a string", errcode=Codes.INVALID_PARAM + ) await self.set_avatar_url( target_user=target_user, requester=requester, @@ -721,7 +729,7 @@ class ProfileHandler: target_user: UserID, requester: Requester, field_name: str, - new_value: JsonValue, + new_value: JsonValue | dict[str, JsonValue], *, by_admin: bool = False, ) -> None: diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index ce09ebf357..f349d93ae0 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -252,7 +252,7 @@ class SyncResult: presence: list[UserPresenceState] account_data: list[JsonDict] # user ID -> {profile field -> value | null if unset } - profile_updates: dict[str, dict[str, JsonValue | None] | None] + profile_updates: dict[str, dict[str, JsonValue | dict[str, JsonValue]] | None] joined: list[JoinedSyncResult] invited: list[InvitedSyncResult] knocked: list[KnockedSyncResult] @@ -2213,7 +2213,9 @@ class SyncHandler: profile_data_by_user = await self.store.get_profile_data_for_users(user_ids) # Serialise the profile updates into the sync response format. - profile_updates: dict[str, dict[str, JsonValue | None] | None] = {} + profile_updates: dict[ + str, dict[str, JsonValue | dict[str, JsonValue]] | None + ] = {} for other_user_id in user_ids: profile_data = profile_data_by_user.get(other_user_id) if profile_data is None: @@ -2221,12 +2223,10 @@ class SyncHandler: # in initial sync. continue - per_user_updates: dict[str, JsonValue] = {} + per_user_updates: dict[str, JsonValue | dict[str, JsonValue]] = {} for field_name in profile_fields: if profile_data.get(field_name): - per_user_updates[field_name] = cast( - JsonValue, profile_data[field_name] - ) + per_user_updates[field_name] = profile_data[field_name] if per_user_updates: profile_updates[other_user_id] = per_user_updates @@ -2321,7 +2321,9 @@ class SyncHandler: # Serialise the profile updates into the sync response format. # user ID -> {profile field -> value | null if unset } - profile_updates: dict[str, dict[str, JsonValue | None] | None] = {} + profile_updates: dict[ + str, dict[str, JsonValue | dict[str, JsonValue]] | None + ] = {} # Process field updates and users who have events in the sync response if users: @@ -2351,7 +2353,7 @@ class SyncHandler: profile_updates[other_user_id] = None continue - per_user_updates: dict[str, JsonValue] = {} + per_user_updates: dict[str, JsonValue | dict[str, JsonValue]] = {} if include_users and other_user_id in include_users: # Include the full profile as this user has events in # a lazy loaded sync response, except for fields we've recently @@ -3443,7 +3445,9 @@ class SyncResultBuilder: presence: list[UserPresenceState] = attr.Factory(list) account_data: list[JsonDict] = attr.Factory(list) - profile_updates: dict[str, dict[str, JsonValue | None] | None] = attr.Factory(dict) + profile_updates: dict[str, dict[str, JsonValue | dict[str, JsonValue]] | None] = ( + attr.Factory(dict) + ) joined: list[JoinedSyncResult] = attr.Factory(list) invited: list[InvitedSyncResult] = attr.Factory(list) knocked: list[KnockedSyncResult] = attr.Factory(list) diff --git a/synapse/replication/http/profile.py b/synapse/replication/http/profile.py index 21a79226b6..a692836489 100644 --- a/synapse/replication/http/profile.py +++ b/synapse/replication/http/profile.py @@ -21,7 +21,7 @@ from twisted.web.server import Request from synapse.api.constants import Membership from synapse.http.server import HttpServer from synapse.replication.http._base import ReplicationEndpoint -from synapse.types import JsonDict, UserID, create_requester +from synapse.types import JsonDict, JsonValue, UserID, create_requester if TYPE_CHECKING: from synapse.server import HomeServer @@ -65,7 +65,7 @@ class ReplicationProfileSetFieldValue(ReplicationEndpoint): user_id: str, requester_id: str, field_name: str, - new_value: str, + new_value: JsonValue | dict[str, JsonValue], by_admin: bool = False, propagate: bool = False, authenticated_entity: str | None = None, diff --git a/synapse/rest/client/profile.py b/synapse/rest/client/profile.py index 7cd4678bc0..1767b13eb6 100644 --- a/synapse/rest/client/profile.py +++ b/synapse/rest/client/profile.py @@ -150,7 +150,9 @@ class ProfileFieldRestServlet(RestServlet): await self.profile_handler.check_profile_query_allowed(user, requester_user) if field_name == ProfileFields.DISPLAYNAME: - field_value: JsonValue = await self.profile_handler.get_displayname(user) + field_value: ( + JsonValue | dict[str, JsonValue] + ) = await self.profile_handler.get_displayname(user) elif field_name == ProfileFields.AVATAR_URL: field_value = await self.profile_handler.get_avatar_url(user) else: diff --git a/synapse/storage/databases/main/profile.py b/synapse/storage/databases/main/profile.py index 7c41c4562c..f1141f90fb 100644 --- a/synapse/storage/databases/main/profile.py +++ b/synapse/storage/databases/main/profile.py @@ -265,7 +265,9 @@ class ProfileWorkerStore(SQLBaseStore): desc="get_profile_avatar_url", ) - async def get_profile_field(self, user_id: UserID, field_name: str) -> JsonValue: + async def get_profile_field( + self, user_id: UserID, field_name: str + ) -> JsonValue | dict[str, JsonValue]: """ Get a custom profile field for a user. @@ -277,7 +279,9 @@ class ProfileWorkerStore(SQLBaseStore): The string value if the field exists, otherwise raises 404. """ - def get_profile_field(txn: LoggingTransaction) -> JsonValue: + def get_profile_field( + txn: LoggingTransaction, + ) -> JsonValue | dict[str, JsonValue]: # This will error if field_name has double quotes in it, but that's not # possible due to the grammar. field_path = f'$."{field_name}"' @@ -295,7 +299,9 @@ class ProfileWorkerStore(SQLBaseStore): # Test exists first since value being None is used for both # missing and a null JSON value. - exists, value = cast(tuple[bool, JsonValue], txn.fetchone()) + exists, value = cast( + tuple[bool, JsonValue | dict[str, JsonValue]], txn.fetchone() + ) if not exists: raise StoreError(404, "No row found") return value @@ -312,7 +318,9 @@ class ProfileWorkerStore(SQLBaseStore): ) # If value_type is None, then the value did not exist. - value_type, value = cast(tuple[str | None, JsonValue], txn.fetchone()) + value_type, value = cast( + tuple[str | None, JsonValue | dict[str, JsonValue]], txn.fetchone() + ) if not value_type: raise StoreError(404, "No row found") # If value_type is object or array, then need to deserialize the JSON. @@ -548,7 +556,7 @@ class ProfileWorkerStore(SQLBaseStore): async def get_profile_data_for_users( self, user_ids: Collection[str] - ) -> dict[str, dict[str, str | JsonDict | None]]: + ) -> dict[str, dict[str, JsonValue | dict[str, JsonValue]]]: """Fetch displayname/avatar_url/custom fields for a list of users. Currently, this returns only local users as the `profiles` table only @@ -571,7 +579,7 @@ class ProfileWorkerStore(SQLBaseStore): desc="get_profile_data_for_users", ) - results: dict[str, dict[str, str | JsonDict | None]] = {} + results: dict[str, dict[str, JsonValue | dict[str, JsonValue]]] = {} for full_user_id, displayname, avatar_url, fields in rows: user_fields = fields or {} # The SQLite driver doesn't automatically convert JSON to @@ -708,7 +716,7 @@ class ProfileWorkerStore(SQLBaseStore): txn: LoggingTransaction, user_id: UserID, new_field_name: str, - new_value: JsonValue, + new_value: JsonValue | dict[str, JsonValue], ) -> None: # For each entry there are 4 quotes (2 each for key and value), 1 colon, # and 1 comma. @@ -835,7 +843,10 @@ class ProfileWorkerStore(SQLBaseStore): ) async def set_profile_field( - self, user_id: UserID, field_name: str, new_value: JsonValue + self, + user_id: UserID, + field_name: str, + new_value: JsonValue | dict[str, JsonValue], ) -> None: """ Set a custom profile field for a user.