From 71e1da976cfbfbdf4be2598a84d6a22671c1bf46 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 15 May 2026 11:36:47 +0100 Subject: [PATCH 01/12] Revert "Send a SSS response immediately if the config has changed and there are new results to sync (#19714)" (#19784) Reverts: #19714 Opens: #19783 Closes: https://github.com/element-hq/backend-internal/issues/242 Related: #18880 (the performance problem that is aggravated by #19714) This reverts commit 2691d0b8b143e01987504210e320198560371435. --------- Signed-off-by: Olivier 'reivilibre --- changelog.d/19784.bugfix | 1 + synapse/handlers/sliding_sync/__init__.py | 52 ++++---- synapse/handlers/sliding_sync/room_lists.py | 12 +- .../sliding_sync/test_room_subscriptions.py | 119 ------------------ 4 files changed, 29 insertions(+), 155 deletions(-) create mode 100644 changelog.d/19784.bugfix diff --git a/changelog.d/19784.bugfix b/changelog.d/19784.bugfix new file mode 100644 index 0000000000..c68524d57a --- /dev/null +++ b/changelog.d/19784.bugfix @@ -0,0 +1 @@ +Revert 'Have [MSC4186: Simplified Sliding Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4186) return a new response immediately if a room subscription has changed and produced a new response. ([\#19714](https://github.com/element-hq/synapse/issues/19714))' due to performance problems. diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index a3443b300c..1cc587d4a7 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -167,38 +167,34 @@ class SlidingSyncHandler: timeout_ms -= after_wait_ts - before_wait_ts timeout_ms = max(timeout_ms, 0) - # Compute a response immediately. We always need to do this before - # waiting for new data (unlike in /v3/sync), as the request config might - # have changed (e.g. new room subscriptions, etc). - now_token = self.event_sources.get_current_token() - result = await self.current_sync_for_user( - sync_config, - from_token=from_token, - to_token=now_token, - ) - - # Return immediately if we have a result, the timeout is 0, or this is - # an initial sync. - if result or timeout_ms == 0 or from_token is None: - return result, did_wait - - # Otherwise, we wait for something to happen and report it to the user. - async def current_sync_callback( - before_token: StreamToken, after_token: StreamToken - ) -> SlidingSyncResult: - return await self.current_sync_for_user( + # We're going to respond immediately if the timeout is 0 or if this is an + # initial sync (without a `from_token`) so we can avoid calling + # `notifier.wait_for_events()`. + if timeout_ms == 0 or from_token is None: + now_token = self.event_sources.get_current_token() + result = await self.current_sync_for_user( sync_config, from_token=from_token, - to_token=after_token, + to_token=now_token, ) + else: + # Otherwise, we wait for something to happen and report it to the user. + async def current_sync_callback( + before_token: StreamToken, after_token: StreamToken + ) -> SlidingSyncResult: + return await self.current_sync_for_user( + sync_config, + from_token=from_token, + to_token=after_token, + ) - result = await self.notifier.wait_for_events( - sync_config.user.to_string(), - timeout_ms, - current_sync_callback, - from_token=now_token, - ) - did_wait = True + result = await self.notifier.wait_for_events( + sync_config.user.to_string(), + timeout_ms, + current_sync_callback, + from_token=from_token.stream_token, + ) + did_wait = True return result, did_wait diff --git a/synapse/handlers/sliding_sync/room_lists.py b/synapse/handlers/sliding_sync/room_lists.py index 216ef3b071..8969d91583 100644 --- a/synapse/handlers/sliding_sync/room_lists.py +++ b/synapse/handlers/sliding_sync/room_lists.py @@ -852,15 +852,11 @@ class SlidingSyncRoomLists: previous_connection_state.room_configs.get(room_id) ) if prev_room_sync_config is not None: - # Always include rooms whose effective config has - # expanded. This covers timeline-limit increases and - # required-state additions introduced by room - # subscriptions overriding list-derived params. + # Always include rooms whose timeline limit has increased. + # (see the "XXX: Odd behavior" described below) if ( - prev_room_sync_config.combine_room_sync_config( - room_config - ) - != prev_room_sync_config + prev_room_sync_config.timeline_limit + < room_config.timeline_limit ): rooms_should_send.add(room_id) continue diff --git a/tests/rest/client/sliding_sync/test_room_subscriptions.py b/tests/rest/client/sliding_sync/test_room_subscriptions.py index d970af367d..811478f1ba 100644 --- a/tests/rest/client/sliding_sync/test_room_subscriptions.py +++ b/tests/rest/client/sliding_sync/test_room_subscriptions.py @@ -22,7 +22,6 @@ import synapse.rest.admin from synapse.api.constants import EventTypes, HistoryVisibility from synapse.rest.client import login, room, sync from synapse.server import HomeServer -from synapse.types import JsonDict from synapse.util.clock import Clock from tests.rest.client.sliding_sync.test_sliding_sync import SlidingSyncBase @@ -127,124 +126,6 @@ class SlidingSyncRoomSubscriptionsTestCase(SlidingSyncBase): response_body["rooms"][room_id1], ) - def test_room_subscription_required_state_expansion_returns_immediately( - self, - ) -> None: - """ - Test that adding a room subscription with stronger params than the list causes an - incremental long-poll to return immediately, even without new stream activity. - """ - user1_id = self.register_user("user1", "pass") - user1_tok = self.login(user1_id, "pass") - - room_id1 = self.helper.create_room_as(user1_id, tok=user1_tok) - - sync_body: JsonDict = { - "lists": { - "foo-list": { - "ranges": [[0, 0]], - "required_state": [], - "timeline_limit": 0, - } - }, - "conn_id": "conn_id", - } - _, from_token = self.do_sync(sync_body, tok=user1_tok) - - sync_body["room_subscriptions"] = { - room_id1: { - "required_state": [ - [EventTypes.Create, ""], - ], - "timeline_limit": 0, - } - } - - channel = self.make_request( - "POST", - self.sync_endpoint + f"?timeout=10000&pos={from_token}", - content=sync_body, - access_token=user1_tok, - await_result=False, - ) - channel.await_result(timeout_ms=3000) - self.assertEqual(channel.code, 200, channel.json_body) - - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id1) - ) - - room_response = channel.json_body["rooms"][room_id1] - self.assertNotIn("initial", room_response) - self._assertRequiredStateIncludes( - room_response["required_state"], - { - state_map[(EventTypes.Create, "")], - }, - exact=True, - ) - - def test_room_subscription_required_state_change_returns_immediately(self) -> None: - """ - Test that expanding an existing room subscription's required state causes an - incremental long-poll to return immediately, even without new stream activity. - """ - user1_id = self.register_user("user1", "pass") - user1_tok = self.login(user1_id, "pass") - - room_id1 = self.helper.create_room_as( - user1_id, tok=user1_tok, extra_content={"name": "Foo"} - ) - - sync_body: JsonDict = { - "room_subscriptions": { - room_id1: { - "required_state": [ - [EventTypes.Create, ""], - ], - "timeline_limit": 0, - } - }, - "conn_id": "conn_id", - } - response_body, from_token = self.do_sync(sync_body, tok=user1_tok) - - state_map = self.get_success( - self.storage_controllers.state.get_current_state(room_id1) - ) - self._assertRequiredStateIncludes( - response_body["rooms"][room_id1]["required_state"], - { - state_map[(EventTypes.Create, "")], - }, - exact=True, - ) - - sync_body["room_subscriptions"][room_id1]["required_state"] = [ - [EventTypes.Create, ""], - [EventTypes.Name, ""], - ] - - channel = self.make_request( - "POST", - self.sync_endpoint + f"?timeout=10000&pos={from_token}", - content=sync_body, - access_token=user1_tok, - await_result=False, - ) - channel.await_result(timeout_ms=3000) - self.assertEqual(channel.code, 200, channel.json_body) - - room_response = channel.json_body["rooms"][room_id1] - self.assertNotIn("initial", room_response) - self._assertRequiredStateIncludes( - room_response["required_state"], - { - state_map[(EventTypes.Name, "")], - }, - exact=True, - ) - def test_room_subscriptions_with_leave_membership(self) -> None: """ Test `room_subscriptions` with a leave room should give us timeline and state From 0ff50720d899e6f867b5694c0f05be1561d650f6 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Fri, 15 May 2026 11:43:12 +0100 Subject: [PATCH 02/12] 1.153.0rc3 --- CHANGES.md | 7 +++++++ changelog.d/19784.bugfix | 1 - debian/changelog | 6 ++++++ pyproject.toml | 2 +- 4 files changed, 14 insertions(+), 2 deletions(-) delete mode 100644 changelog.d/19784.bugfix diff --git a/CHANGES.md b/CHANGES.md index f722a1e703..96a9fadbfb 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,10 @@ +# Synapse 1.153.0rc3 (2026-05-15) + +## Bugfixes + +- Revert 'Have [MSC4186: Simplified Sliding Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4186) return a new response immediately if a room subscription has changed and produced a new response. ([\#19714](https://github.com/element-hq/synapse/issues/19714))' (introduced in 1.153.0rc1) due to performance problems. ([\#19784](https://github.com/element-hq/synapse/issues/19784)) + + # Synapse 1.153.0rc2 (2026-05-13) ## Bugfixes diff --git a/changelog.d/19784.bugfix b/changelog.d/19784.bugfix deleted file mode 100644 index c68524d57a..0000000000 --- a/changelog.d/19784.bugfix +++ /dev/null @@ -1 +0,0 @@ -Revert 'Have [MSC4186: Simplified Sliding Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4186) return a new response immediately if a room subscription has changed and produced a new response. ([\#19714](https://github.com/element-hq/synapse/issues/19714))' due to performance problems. diff --git a/debian/changelog b/debian/changelog index bbd164b2a8..81f080bead 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (1.153.0~rc3) stable; urgency=medium + + * New Synapse release 1.153.0rc3. + + -- Synapse Packaging team Fri, 15 May 2026 11:42:29 +0100 + matrix-synapse-py3 (1.153.0~rc2) stable; urgency=medium * New Synapse release 1.153.0rc2. diff --git a/pyproject.toml b/pyproject.toml index a7422e1987..fed1a23f4a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "matrix-synapse" -version = "1.153.0rc2" +version = "1.153.0rc3" description = "Homeserver for the Matrix decentralised comms protocol" readme = "README.rst" authors = [ From 0c6e0f79e5fef944ceca5f74404ec4a44be310bd Mon Sep 17 00:00:00 2001 From: mhlas7 Date: Fri, 15 May 2026 08:29:53 -0700 Subject: [PATCH 03/12] doc: Enhance update_profile_information documentation with picture claim (#19508) Added details how synapse syncs the picture claim when update_profile_information setting is true. Addresses #17836 --------- Co-authored-by: Michael Hlas <3398654+mhlas7@users.noreply.github.com> --- changelog.d/19508.doc | 1 + docs/usage/configuration/config_documentation.md | 5 ++++- schema/synapse-config.schema.yaml | 14 +++++++++----- 3 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 changelog.d/19508.doc diff --git a/changelog.d/19508.doc b/changelog.d/19508.doc new file mode 100644 index 0000000000..2550116341 --- /dev/null +++ b/changelog.d/19508.doc @@ -0,0 +1 @@ +Added details about how Synapse syncs the picture claim when `update_profile_information` setting is true. diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index e58a2dcf10..d028d65fe3 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3788,7 +3788,10 @@ This setting has the following sub-options: Defaults to `null`. -* `update_profile_information` (boolean): Use this setting to keep a user's profile fields in sync with information from the identity provider. Currently only syncing the displayname is supported. Fields are checked on every SSO login, and are updated if necessary. Note that enabling this option will override user profile information, regardless of whether users have opted-out of syncing that information when first signing in. Defaults to `false`. +* `update_profile_information` (boolean): Use this setting to keep a user's profile fields in sync with information from the identity provider. Fields are checked on every SSO login, and are updated if necessary. Note that enabling this option will override user profile information, regardless of whether users have opted-out of syncing that information when first signing in. Fields that will be synced: + * displayname + * picture - only if Synapse media repository is running in the main + process (i.e. not workerized) and media is stored locally Defaults to `false`. Example configuration: ```yaml diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 8888d2c673..8b8d57b9bf 100644 --- a/schema/synapse-config.schema.yaml +++ b/schema/synapse-config.schema.yaml @@ -4620,11 +4620,15 @@ properties: type: boolean description: >- Use this setting to keep a user's profile fields in sync with - information from the identity provider. Currently only syncing the - displayname is supported. Fields are checked on every SSO login, and - are updated if necessary. Note that enabling this option will override - user profile information, regardless of whether users have opted-out - of syncing that information when first signing in. + information from the identity provider. Fields are checked on every + SSO login, and are updated if necessary. Note that enabling this + option will override user profile information, regardless of whether + users have opted-out of syncing that information when first signing + in. + Fields that will be synced: + * displayname + * picture - only if Synapse media repository is running in the main + process (i.e. not workerized) and media is stored locally default: false examples: - client_whitelist: From 19f636244ce3a4fa6970f5abe970d105d963e0e6 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 15 May 2026 11:49:11 -0500 Subject: [PATCH 04/12] Prefer close backfill points (absolute distance) (#19748) This isn't fixing any particular issue. It's just a follow-up I thought about after merging https://github.com/element-hq/synapse/pull/19611 since we're now also dealing with backfill points in the nearby range ahead of the `current_depth`. And it's possible that the previous sort could bias to all nearby backfill points ahead of the `current_depth` that don't extend into the visible window of events we're paginating through. --- changelog.d/19748.misc | 1 + synapse/handlers/federation.py | 19 +++++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 changelog.d/19748.misc diff --git a/changelog.d/19748.misc b/changelog.d/19748.misc new file mode 100644 index 0000000000..eedd4e92a2 --- /dev/null +++ b/changelog.d/19748.misc @@ -0,0 +1 @@ +Prefer close backfill points (absolute distance). diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 166a02d7c7..ba83d4fd26 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -275,11 +275,22 @@ class FederationHandler: ) ] - # we now have a list of potential places to backpaginate from. We prefer to - # start with the most recent (ie, max depth), so let's sort the list. + # we now have a list of potential places to backpaginate from. Figure out which + # ones we should prefer, so let's sort the list. sorted_backfill_points: list[_BackfillPoint] = sorted( backwards_extremities, - key=lambda e: -int(e.depth), + key=lambda e: ( + # Prefer backfill points that are closer to the `current_depth` + # (absolute distance) + abs(current_depth - e.depth), + # For the tie-break, we care about events that are actually in the past + # as they're more likely to reveal history that we can return (something + # absolutely in the past is better than something can potentially extend + # into the past). + # + # This sorts ascending so 0 sorts before 1 + 0 if current_depth >= e.depth else 1, + ), ) logger.debug( @@ -300,7 +311,7 @@ class FederationHandler: str(len(sorted_backfill_points)), ) - # If we have no backfill points lower than the `current_depth` then either we + # If we have no backfill points lower than the `nearby_depth` then either we # can a) bail or b) still attempt to backfill. We opt to try backfilling anyway # just in case we do get relevant events. This is good for eventual consistency # sake but we don't need to block the client for something that is just as From 8eb220a5e25c57da979d5b0bc8dbc7b5b657e501 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 15 May 2026 13:51:03 -0500 Subject: [PATCH 05/12] Replace `wait_for_quarantined_media_stream_id(...)` with standard `wait_for_stream_token(...)` (#19764) In order to be able to use `wait_for_stream_token(...)`, we have to add the `quarantined_media` stream to the `StreamToken`. Even though we don't care about `/sync`'ing `quarantined_media`, this aligns with the future where all endpoints should probably use `StreamToken`, see https://github.com/element-hq/synapse/issues/19647 Follow-up to https://github.com/element-hq/synapse/pull/19558 and https://github.com/element-hq/synapse/pull/19644 --- changelog.d/19764.misc | 1 + synapse/rest/admin/media.py | 23 ++++++-- synapse/storage/databases/main/room.py | 80 ++++---------------------- synapse/streams/events.py | 3 + synapse/types/__init__.py | 27 +++++++-- tests/rest/admin/test_room.py | 4 +- tests/rest/client/test_rooms.py | 4 +- 7 files changed, 59 insertions(+), 83 deletions(-) create mode 100644 changelog.d/19764.misc diff --git a/changelog.d/19764.misc b/changelog.d/19764.misc new file mode 100644 index 0000000000..8704e3eed6 --- /dev/null +++ b/changelog.d/19764.misc @@ -0,0 +1 @@ +Replace unique `quarantined_media` waiting patterns with standard `wait_for_stream_token(...)`. diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 1633cca884..35454c1522 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -43,7 +43,13 @@ from synapse.rest.admin._base import ( from synapse.storage.databases.main.media_repository import ( MediaSortOrder, ) -from synapse.types import JsonDict, UserID +from synapse.types import ( + JsonDict, + MultiWriterStreamToken, + StreamKeyType, + StreamToken, + UserID, +) if TYPE_CHECKING: from synapse.server import HomeServer @@ -243,6 +249,7 @@ class ListQuarantineChanges(RestServlet): self.auth = hs.get_auth() self.server_name = hs.hostname self.replication = hs.get_replication_data_handler() + self.notifier = hs.get_notifier() async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) @@ -256,8 +263,7 @@ class ListQuarantineChanges(RestServlet): # The caller is trying to get future data, which we don't allow because # we know it's an invalid state that should never happen. We could # wait until we reach the token but we might as well not waste our - # resources on that which is why `wait_for_quarantined_media_stream_id(...)` - # has assertions around this. + # resources on that. raise SynapseError( HTTPStatus.BAD_REQUEST, "The `from` token is considered invalid because it includes stream positions " @@ -268,9 +274,16 @@ class ListQuarantineChanges(RestServlet): errcode=Codes.INVALID_PARAM, ) + # Create a `StreamToken` that's compatible with `wait_for_stream_token`. + # + # FIXME: Ideally, this endpoint would use a `StreamToken` to begin with + from_token = StreamToken.START.copy_and_replace( + StreamKeyType.QUARANTINED_MEDIA, MultiWriterStreamToken(stream=from_id) + ) + # We need to wait to ensure that our current worker is actually caught up with # the stream position, otherwise we might not return what we think we're returning. - if not await self.store.wait_for_quarantined_media_stream_id(from_id): + if not await self.notifier.wait_for_stream_token(from_token): raise SynapseError( HTTPStatus.INTERNAL_SERVER_ERROR, "Timed out while waiting for the worker serving this request to catch up to the given " @@ -280,7 +293,7 @@ class ListQuarantineChanges(RestServlet): errcode=Codes.UNKNOWN, ) - to_id = await self.store.get_current_quarantined_media_stream_id() + to_id = self.store.get_current_quarantined_media_stream_id() changes = await self.store.get_quarantined_media_changes( from_id=from_id, to_id=to_id, diff --git a/synapse/storage/databases/main/room.py b/synapse/storage/databases/main/room.py index a0c42082f0..95aa2cb7dc 100644 --- a/synapse/storage/databases/main/room.py +++ b/synapse/storage/databases/main/room.py @@ -62,12 +62,12 @@ from synapse.storage.types import Cursor from synapse.storage.util.id_generators import IdGenerator, MultiWriterIdGenerator from synapse.types import ( JsonDict, + MultiWriterStreamToken, RetentionPolicy, StrCollection, ThirdPartyInstanceID, ) from synapse.util.caches.descriptors import cached, cachedList -from synapse.util.duration import Duration from synapse.util.json import json_encoder from synapse.util.stringutils import MXC_REGEX @@ -1302,7 +1302,15 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): return local_media_ids - async def get_current_quarantined_media_stream_id(self) -> int: + def get_quarantined_media_stream_token(self) -> MultiWriterStreamToken: + return MultiWriterStreamToken.from_generator( + self._quarantined_media_changes_id_gen + ) + + def get_quarantined_media_stream_id_generator(self) -> MultiWriterIdGenerator: + return self._quarantined_media_changes_id_gen + + def get_current_quarantined_media_stream_id(self) -> int: """Gets the position of the quarantined media changes stream. Returns: @@ -1318,74 +1326,6 @@ class RoomWorkerStore(CacheInvalidationWorkerStore): """ return await self._quarantined_media_changes_id_gen.get_max_allocated_token() - async def wait_for_quarantined_media_stream_id(self, target_id: int) -> bool: - """Waits until the quarantined media changes stream reaches the given stream ID. - - See https://github.com/element-hq/synapse/pull/19644 for more details. - - TODO: Replace function and call sites with https://github.com/element-hq/synapse/pull/19644 - - Args: - target_id: The stream ID to wait for. - - Returns: - True when caught up to the target stream ID. - False when timing out while waiting. - """ - # We ideally would use something like `wait_for_stream_position` in the meantime, - # but that short circuits if the instance name matches the current instance name. - # Doing so means that if *another* writer is actually leading the to_id, then we'll - # assume that we're caught up when we aren't. - # - # NOTE: Because this is implemented to wait for stream positions by integer ID, - # we're technically waiting for *all* workers to catch up rather than just waiting - # for *our* worker to catch up. This is okay for now because the quarantined media - # stream should be pretty fast to update, and if it's not then the only thing we're - # affecting is an admin API that probably has a tool automatically retrying requests - # anyway. https://github.com/element-hq/synapse/pull/19644 does the waiting properly - # so this should be replaced by that (or similar). - - # Get the minimum shared position/ID across all workers - current_id = self._quarantined_media_changes_id_gen.get_current_token() - if current_id >= target_id: - return True # nothing to wait for: we're already caught up. - - # "This should never happen". Tokens we hand out via the API should exist. If they - # don't, then we're in a bad state and need to explode. - max_persisted_position = ( - await self._quarantined_media_changes_id_gen.get_max_allocated_token() - ) - assert max_persisted_position >= target_id, ( - f"Unable to wait for invalid future token (token={target_id} has positions " - f"ahead of our max persisted position={max_persisted_position})" - ) - - # Start waiting until we've caught up to the `stream_token` - start = self.clock.time_msec() - logged = False - while True: - # Like above, get the minimum shared ID across all workers - current_id = self._quarantined_media_changes_id_gen.get_current_token() - if current_id >= target_id: - return True - - now = self.clock.time_msec() - - # Timed out - if now - start > 10_000: - return False - - if not logged: - logger.info( - "Waiting for current token to reach %s; currently at %s", - target_id, - current_id, - ) - logged = True - - # TODO: be better - await self.clock.sleep(Duration(milliseconds=500)) - async def get_quarantined_media_changes( self, *, from_id: int, to_id: int, limit: int ) -> list[QuarantinedMediaUpdate]: diff --git a/synapse/streams/events.py b/synapse/streams/events.py index f5677a2082..36490fcb35 100644 --- a/synapse/streams/events.py +++ b/synapse/streams/events.py @@ -85,6 +85,7 @@ class EventSources: ) thread_subscriptions_key = self.store.get_max_thread_subscriptions_stream_id() sticky_events_key = self.store.get_max_sticky_events_stream_id() + quarantined_media_key = self.store.get_quarantined_media_stream_token() token = StreamToken( room_key=self.sources.room.get_current_key(), @@ -100,6 +101,7 @@ class EventSources: un_partial_stated_rooms_key=un_partial_stated_rooms_key, thread_subscriptions_key=thread_subscriptions_key, sticky_events_key=sticky_events_key, + quarantined_media_key=quarantined_media_key, ) return token @@ -128,6 +130,7 @@ class EventSources: StreamKeyType.UN_PARTIAL_STATED_ROOMS: self.store.get_un_partial_stated_rooms_id_generator(), StreamKeyType.THREAD_SUBSCRIPTIONS: self.store.get_thread_subscriptions_stream_id_generator(), StreamKeyType.STICKY_EVENTS: self.store.get_sticky_events_stream_id_generator(), + StreamKeyType.QUARANTINED_MEDIA: self.store.get_quarantined_media_stream_id_generator(), } for _, key in StreamKeyType.__members__.items(): diff --git a/synapse/types/__init__.py b/synapse/types/__init__.py index 8b005ef84d..8537a63bde 100644 --- a/synapse/types/__init__.py +++ b/synapse/types/__init__.py @@ -1060,6 +1060,7 @@ class StreamKeyType(Enum): UN_PARTIAL_STATED_ROOMS = "un_partial_stated_rooms_key" THREAD_SUBSCRIPTIONS = "thread_subscriptions_key" STICKY_EVENTS = "sticky_events_key" + QUARANTINED_MEDIA = "quarantined_media_key" @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -1067,7 +1068,7 @@ class StreamToken: """A collection of keys joined together by underscores in the following order and which represent the position in their respective streams. - ex. `s2633508_17_338_6732159_1082514_541479_274711_265584_1_379_4242` + ex. `s2633508_17_338_6732159_1082514_541479_274711_265584_1_379_4242_4141_4343` 1. `room_key`: `s2633508` which is a `RoomStreamToken` - `RoomStreamToken`'s can also look like `t426-2633508` or `m56~2.58~3.59` - See the docstring for `RoomStreamToken` for more details. @@ -1082,12 +1083,13 @@ class StreamToken: 10. `un_partial_stated_rooms_key`: `379` 11. `thread_subscriptions_key`: 4242 12. `sticky_events_key`: 4141 + 13. `quarantined_media_key`: 4343 You can see how many of these keys correspond to the various fields in a "/sync" response: ```json { - "next_batch": "s12_4_0_1_1_1_1_4_1_1", + "next_batch": "s12_4_0_1_1_1_1_4_1_1_1_1_1", "presence": { "events": [] }, @@ -1099,7 +1101,7 @@ class StreamToken: "!QrZlfIDQLNLdZHqTnt:hs1": { "timeline": { "events": [], - "prev_batch": "s10_4_0_1_1_1_1_4_1_1", + "prev_batch": "s10_4_0_1_1_1_1_4_1_1_1_1_1", "limited": false }, "state": { @@ -1142,6 +1144,9 @@ class StreamToken: un_partial_stated_rooms_key: int thread_subscriptions_key: int sticky_events_key: int + quarantined_media_key: MultiWriterStreamToken = attr.ib( + validator=attr.validators.instance_of(MultiWriterStreamToken) + ) _SEPARATOR = "_" START: ClassVar["StreamToken"] @@ -1171,6 +1176,7 @@ class StreamToken: un_partial_stated_rooms_key, thread_subscriptions_key, sticky_events_key, + quarantined_media_key, ) = keys return cls( @@ -1188,6 +1194,9 @@ class StreamToken: un_partial_stated_rooms_key=int(un_partial_stated_rooms_key), thread_subscriptions_key=int(thread_subscriptions_key), sticky_events_key=int(sticky_events_key), + quarantined_media_key=await MultiWriterStreamToken.parse( + store, quarantined_media_key + ), ) except CancelledError: raise @@ -1212,6 +1221,7 @@ class StreamToken: str(self.un_partial_stated_rooms_key), str(self.thread_subscriptions_key), str(self.sticky_events_key), + await self.quarantined_media_key.to_string(store), ] ) @@ -1241,6 +1251,12 @@ class StreamToken: self.device_list_key.copy_and_advance(new_value), ) return new_token + elif key == StreamKeyType.QUARANTINED_MEDIA: + new_token = self.copy_and_replace( + StreamKeyType.QUARANTINED_MEDIA, + self.quarantined_media_key.copy_and_advance(new_value), + ) + return new_token new_token = self.copy_and_replace(key, new_value) new_id = new_token.get_field(key) @@ -1263,6 +1279,7 @@ class StreamToken: key: Literal[ StreamKeyType.RECEIPT, StreamKeyType.DEVICE_LIST, + StreamKeyType.QUARANTINED_MEDIA, ], ) -> MultiWriterStreamToken: ... @@ -1334,7 +1351,8 @@ class StreamToken: f"account_data: {self.account_data_key}, push_rules: {self.push_rules_key}, " f"to_device: {self.to_device_key}, device_list: {self.device_list_key}, " f"groups: {self.groups_key}, un_partial_stated_rooms: {self.un_partial_stated_rooms_key}," - f"thread_subscriptions: {self.thread_subscriptions_key}, sticky_events: {self.sticky_events_key})" + f"thread_subscriptions: {self.thread_subscriptions_key}, sticky_events: {self.sticky_events_key}" + f"quarantined_media: {self.quarantined_media_key})" ) @@ -1351,6 +1369,7 @@ StreamToken.START = StreamToken( un_partial_stated_rooms_key=0, thread_subscriptions_key=0, sticky_events_key=0, + quarantined_media_key=MultiWriterStreamToken(stream=0), ) diff --git a/tests/rest/admin/test_room.py b/tests/rest/admin/test_room.py index 507cf10c5d..c4e4170c6f 100644 --- a/tests/rest/admin/test_room.py +++ b/tests/rest/admin/test_room.py @@ -2549,7 +2549,7 @@ class RoomMessagesTestCase(unittest.HomeserverTestCase): def test_topo_token_is_accepted(self) -> None: """Test Topo Token is accepted.""" - token = "t1-0_0_0_0_0_0_0_0_0_0_0_0" + token = "t1-0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/_synapse/admin/v1/rooms/%s/messages?from=%s" % (self.room_id, token), @@ -2563,7 +2563,7 @@ class RoomMessagesTestCase(unittest.HomeserverTestCase): def test_stream_token_is_accepted_for_fwd_pagianation(self) -> None: """Test that stream token is accepted for forward pagination.""" - token = "s0_0_0_0_0_0_0_0_0_0_0_0" + token = "s0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/_synapse/admin/v1/rooms/%s/messages?from=%s" % (self.room_id, token), diff --git a/tests/rest/client/test_rooms.py b/tests/rest/client/test_rooms.py index 28872fa06c..10325c536a 100644 --- a/tests/rest/client/test_rooms.py +++ b/tests/rest/client/test_rooms.py @@ -2248,7 +2248,7 @@ class RoomMessageListTestCase(RoomBase): self.room_id = self.helper.create_room_as(self.user_id) def test_topo_token_is_accepted(self) -> None: - token = "t1-0_0_0_0_0_0_0_0_0_0_0_0" + token = "t1-0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/rooms/%s/messages?access_token=x&from=%s" % (self.room_id, token) ) @@ -2259,7 +2259,7 @@ class RoomMessageListTestCase(RoomBase): self.assertTrue("end" in channel.json_body) def test_stream_token_is_accepted_for_fwd_pagianation(self) -> None: - token = "s0_0_0_0_0_0_0_0_0_0_0_0" + token = "s0_0_0_0_0_0_0_0_0_0_0_0_0" channel = self.make_request( "GET", "/rooms/%s/messages?access_token=x&from=%s" % (self.room_id, token) ) From d8b4ffdf2d987a04ab1f98d8e67388a36f79a017 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABtan?= <166950915+gaetan-sbt@users.noreply.github.com> Date: Mon, 18 May 2026 10:27:10 +0100 Subject: [PATCH 06/12] Fix validation of frozen message event with mentions. (#19634) Fixes: #19689 # What This PR fixes a bug I found when I run synapse (from dockerhub) and register a `check_event_allowed` callback and my client makes use of the mentions field in messages (`cinny:latest`). The bug doesn't appear when the `check_event_allowed` callback is not loaded. After some digging I noticed that the current validation of the mentions doesn't work when an event has been frozen with `event.freeze()`. For the messages this seems to happen when a the `check_event_allowed` is registered (but not otherwise), see [where the event is frozen for check_event_allowed callback](https://github.com/element-hq/synapse/blob/b0fc0b7a612a42e6f15b87dee2a1db4c383645fb/synapse/module_api/callbacks/third_party_event_rules_callbacks.py#L289) and [where the validation function is called](https://github.com/element-hq/synapse/blob/b0fc0b7a612a42e6f15b87dee2a1db4c383645fb/synapse/handlers/message.py#L1404). To have a minimal reproduction example, the following scripts fails on `develop` but succeeds in this branch: ``` python from synapse.api.room_versions import RoomVersions from synapse.events import EventBase, make_event_from_dict from synapse.events.validator import EventValidator from tests.utils import default_config def make_message_event(content: dict) -> EventBase: return make_event_from_dict( { "room_id": "!room:test", "type": "m.room.message", "sender": "@alice:test", "content": content, "auth_events": [], "prev_events": [], "hashes": {"sha256": "aGVsbG8="}, "signatures": {}, "depth": 1, "origin_server_ts": 1000, }, room_version=RoomVersions.V9, ) event = make_message_event( { "msgtype": "m.text", "body": "@moderator:example.com hello", "m.mentions": {"user_ids": ["@moderator:jailbreak-challenge.aqtiveguard.com"]}, } ) EventValidator().validate_new(event, default_config) # Ok event.freeze() EventValidator().validate_new(event, default_config) # throws # pydantic_core._pydantic_core.ValidationError: 1 validation error for Mentions # Input should be a valid dictionary or instance of Mentions [type=model_type, input_value=immutabledict({'user_ids'...nge.aqtiveguard.com',)}), input_type=immutabledict] # For further information visit https://errors.pydantic.dev/2.12/v/model_type ``` # How I made the validation logic also validate the transformation performed by the freezing process, namely: - `immutabledict` validates as `dict`. (was already implemented for POWER_LEVELS) - `tuple` validates as array (added this to the validator in this PR). --------- Co-authored-by: Eric Eastwood Co-authored-by: Olivier 'reivilibre --- changelog.d/19634.bugfix | 1 + synapse/events/validator.py | 81 ++++++++++++++++++++++------------ tests/events/test_validator.py | 50 +++++++++++++++++++++ 3 files changed, 103 insertions(+), 29 deletions(-) create mode 100644 changelog.d/19634.bugfix create mode 100644 tests/events/test_validator.py diff --git a/changelog.d/19634.bugfix b/changelog.d/19634.bugfix new file mode 100644 index 0000000000..e8fcb43570 --- /dev/null +++ b/changelog.d/19634.bugfix @@ -0,0 +1 @@ +Fix bug where Synapse would return 400 (`M_BAD_JSON`) when sending a message with `mentions` field and Synapse module `check_event_allowed` callback registered (frozen event). Contributed by @gaetan-sbt. \ No newline at end of file diff --git a/synapse/events/validator.py b/synapse/events/validator.py index d1b5152d77..e8d6cc9710 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -22,7 +22,6 @@ import collections.abc from typing import cast import jsonschema -from pydantic import Field, StrictBool, StrictStr from synapse.api.constants import ( MAX_ALIAS_LENGTH, @@ -40,10 +39,8 @@ from synapse.events.utils import ( CANONICALJSON_MIN_INT, validate_canonicaljson, ) -from synapse.http.servlet import validate_json_object from synapse.storage.controllers.state import server_acl_evaluator_from_event from synapse.types import EventID, JsonDict, JsonMapping, RoomID, StrCollection, UserID -from synapse.types.rest import RequestBodyModel class EventValidator: @@ -116,29 +113,18 @@ class EventValidator: cls=POWER_LEVELS_VALIDATOR, ) except jsonschema.ValidationError as e: - if e.path: - # example: "users_default": '0' is not of type 'integer' - # cast safety: path entries can be integers, if we fail to validate - # items in an array. However, the POWER_LEVELS_SCHEMA doesn't expect - # to see any arrays. - message = ( - '"' + cast(str, e.path[-1]) + '": ' + e.message # noqa: B306 - ) - # jsonschema.ValidationError.message is a valid attribute - else: - # example: '0' is not of type 'integer' - message = e.message # noqa: B306 - # jsonschema.ValidationError.message is a valid attribute - - raise SynapseError( - code=400, - msg=message, - errcode=Codes.BAD_JSON, - ) + raise _validation_error_to_api_error(e) # If the event contains a mentions key, validate it. if EventContentFields.MENTIONS in event.content: - validate_json_object(event.content[EventContentFields.MENTIONS], Mentions) + try: + jsonschema.validate( + instance=event.content[EventContentFields.MENTIONS], + schema=MENTIONS_SCHEMA, + cls=MENTIONS_VALIDATOR, + ) + except jsonschema.ValidationError as e: + raise _validation_error_to_api_error(e) def _validate_retention(self, event: EventBase) -> None: """Checks that an event that defines the retention policy for a room respects the @@ -284,10 +270,16 @@ POWER_LEVELS_SCHEMA = { }, } - -class Mentions(RequestBodyModel): - user_ids: list[StrictStr] = Field(default_factory=list) - room: StrictBool = False +MENTIONS_SCHEMA = { + "type": "object", + "properties": { + "user_ids": { + "type": "array", + "items": {"type": "string"}, + }, + "room": {"type": "boolean"}, + }, +} # This could return something newer than Draft 7, but that's the current "latest" @@ -295,14 +287,45 @@ class Mentions(RequestBodyModel): def _create_validator(schema: JsonDict) -> type[jsonschema.Draft7Validator]: validator = jsonschema.validators.validator_for(schema) - # by default jsonschema does not consider a immutabledict to be an object so - # we need to use a custom type checker + # by default jsonschema does not consider a immutabledict to be an object, or + # a tuple to be an array (frozenutils freezes lists to tuples), so we need a + # custom type checker for both. # https://python-jsonschema.readthedocs.io/en/stable/validate/?highlight=object#validating-with-additional-types type_checker = validator.TYPE_CHECKER.redefine( "object", lambda checker, thing: isinstance(thing, collections.abc.Mapping) + ).redefine( + "array", + lambda checker, thing: isinstance(thing, collections.abc.Sequence), ) return jsonschema.validators.extend(validator, type_checker=type_checker) +def _validation_error_to_api_error(err: jsonschema.ValidationError) -> SynapseError: + """ + Converts a JSONSchema `ValidationError` to a `SynapseError` that can be thrown + to give a Matrix API-compatible 400 Bad Request response with `M_BAD_JSON` code + and a descriptive error message. + """ + if err.path: + # example: "users_default": '0' is not of type 'integer' + # cast safety: path entries can be integers, if we fail to validate + # items in an array. However, the POWER_LEVELS_SCHEMA doesn't expect + # to see any arrays. + message = '"' + cast(str, err.path[-1]) + '": ' + err.message + # jsonschema.ValidationError.message is a valid attribute + else: + # example: '0' is not of type 'integer' + message = err.message + # jsonschema.ValidationError.message is a valid attribute + + return SynapseError( + code=400, + msg=message, + errcode=Codes.BAD_JSON, + ) + + POWER_LEVELS_VALIDATOR = _create_validator(POWER_LEVELS_SCHEMA) + +MENTIONS_VALIDATOR = _create_validator(MENTIONS_SCHEMA) diff --git a/tests/events/test_validator.py b/tests/events/test_validator.py new file mode 100644 index 0000000000..3810fdb3da --- /dev/null +++ b/tests/events/test_validator.py @@ -0,0 +1,50 @@ +# +# This file is licensed under the Affero General Public License (AGPL) version 3. +# +# Copyright (C) 2026 Element Creations Ltd +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# See the GNU Affero General Public License for more details: +# . +# +from synapse.api.room_versions import RoomVersions +from synapse.events import make_event_from_dict +from synapse.events.validator import EventValidator + +from tests.unittest import HomeserverTestCase + + +class EventValidatorTestCase(HomeserverTestCase): + def test_validate_new_with_mentions_succeeds_even_when_frozen(self) -> None: + """ + Test that `EventValidator.validate_new` accepts an event with valid `m.mentions` + content even when the event is frozen. + """ + event = make_event_from_dict( + { + "room_id": "!room:test", + "type": "m.room.message", + "sender": "@alice:example.com", + "content": { + "msgtype": "m.text", + "body": "@alice:example.com hello", + "m.mentions": {"user_ids": ["@alice:example.com"]}, + }, + "auth_events": [], + "prev_events": [], + "hashes": {"sha256": "aGVsbG8="}, + "signatures": {}, + "depth": 1, + "origin_server_ts": 1000, + }, + room_version=RoomVersions.V9, + ) + # Sanity check that the event is valid before freezing + EventValidator().validate_new(event, self.hs.config) + event.freeze() + # Event should still be valid after freezing + EventValidator().validate_new(event, self.hs.config) From 4d0e4ff9359ecf40997a5f03890cf2aece410319 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Mon, 18 May 2026 12:15:57 +0100 Subject: [PATCH 07/12] Fix `/sync` failing when MSC4354 Sticky Events are enabled and the sync request filters out Ephemeral Data Units (EDUs). (#19787) Fixes: #19779 Fixes: https://github.com/element-hq/synapse/issues/19618 See also: #19786 (which would have caught this, but currently has too many findings to enable) Fix UnboundLocalError when MSC4354 is enabled in sync and all EDUs are filtered out --------- Signed-off-by: Olivier 'reivilibre --- changelog.d/19787.bugfix | 1 + synapse/handlers/sync.py | 14 +++++++++----- 2 files changed, 10 insertions(+), 5 deletions(-) create mode 100644 changelog.d/19787.bugfix diff --git a/changelog.d/19787.bugfix b/changelog.d/19787.bugfix new file mode 100644 index 0000000000..26bd3e2252 --- /dev/null +++ b/changelog.d/19787.bugfix @@ -0,0 +1 @@ +Fix `/sync` failing when [MSC4354 Sticky Events](https://github.com/matrix-org/matrix-spec-proposals/pull/4354) are enabled and the sync request filters out Ephemeral Data Units (EDUs). \ No newline at end of file diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index bca51b64b7..9ecfe0da0f 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -2236,19 +2236,23 @@ class SyncHandler: if block_all_room_ephemeral: ephemeral_by_room: dict[str, list[JsonDict]] = {} else: - now_token, ephemeral_by_room = await self.ephemeral_by_room( + ( + sync_result_builder.now_token, + ephemeral_by_room, + ) = await self.ephemeral_by_room( sync_result_builder, now_token=sync_result_builder.now_token, since_token=sync_result_builder.since_token, ) - sync_result_builder.now_token = now_token sticky_by_room: dict[str, list[str]] = {} if self.hs_config.experimental.msc4354_enabled: - now_token, sticky_by_room = await self.sticky_events_by_room( - sync_result_builder, now_token, since_token + ( + sync_result_builder.now_token, + sticky_by_room, + ) = await self.sticky_events_by_room( + sync_result_builder, sync_result_builder.now_token, since_token ) - sync_result_builder.now_token = now_token # 2. We check up front if anything has changed, if it hasn't then there is # no point in going further. From ec4950b438f17efd7ceaf3fd1e8fa2b9c7d41ac4 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Mon, 18 May 2026 16:10:40 +0000 Subject: [PATCH 08/12] Bump types-jsonschema from 4.26.0.20260202 to 4.26.0.20260508 (#19683) Signed-off-by: dependabot[bot] --- poetry.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/poetry.lock b/poetry.lock index 78f18caadf..ab790015bb 100644 --- a/poetry.lock +++ b/poetry.lock @@ -3417,14 +3417,14 @@ types-webencodings = "*" [[package]] name = "types-jsonschema" -version = "4.26.0.20260202" +version = "4.26.0.20260508" description = "Typing stubs for jsonschema" optional = false -python-versions = ">=3.9" +python-versions = ">=3.10" groups = ["dev"] files = [ - {file = "types_jsonschema-4.26.0.20260202-py3-none-any.whl", hash = "sha256:41c95343abc4de9264e333a55e95dfb4d401e463856d0164eec9cb182e8746da"}, - {file = "types_jsonschema-4.26.0.20260202.tar.gz", hash = "sha256:29831baa4308865a9aec547a61797a06fc152b0dac8dddd531e002f32265cb07"}, + {file = "types_jsonschema-4.26.0.20260508-py3-none-any.whl", hash = "sha256:4ec1dea0a757c8c2e2aa7bc085612fb54e1ae9562428d5da6f26dd7a0f24dbc2"}, + {file = "types_jsonschema-4.26.0.20260508.tar.gz", hash = "sha256:ae0be85ac6ec0cb94a98f75f876b0620cf2afa3e37fdf8460203f4d05f745acb"}, ] [package.dependencies] From 7b1c4da5dfbe48fa5e0dabc3933aed54f40e1644 Mon Sep 17 00:00:00 2001 From: Olivier 'reivilibre Date: Tue, 19 May 2026 14:13:03 +0100 Subject: [PATCH 09/12] 1.153.0 --- CHANGES.md | 5 +++++ debian/changelog | 6 ++++++ pyproject.toml | 2 +- 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/CHANGES.md b/CHANGES.md index 96a9fadbfb..95ab39d0e3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,8 @@ +# Synapse 1.153.0 (2026-05-19) + +No significant changes since 1.153.0rc3. + + # Synapse 1.153.0rc3 (2026-05-15) ## Bugfixes diff --git a/debian/changelog b/debian/changelog index 81f080bead..b1a4e04bc3 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (1.153.0) stable; urgency=medium + + * New Synapse release 1.153.0. + + -- Synapse Packaging team Tue, 19 May 2026 14:12:51 +0100 + matrix-synapse-py3 (1.153.0~rc3) stable; urgency=medium * New Synapse release 1.153.0rc3. diff --git a/pyproject.toml b/pyproject.toml index fed1a23f4a..66cd3e83b5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "matrix-synapse" -version = "1.153.0rc3" +version = "1.153.0" description = "Homeserver for the Matrix decentralised comms protocol" readme = "README.rst" authors = [ From 87095ae500ef1371346f57ef6dfb29231e822cb9 Mon Sep 17 00:00:00 2001 From: FrenchGithubUser Date: Wed, 20 May 2026 12:06:16 +0200 Subject: [PATCH 10/12] fix: invalidate access token cache on device deletion (#19483) when an access token had a refresh token associated to it in the database, deleting this refresh token (for example when deleting the device using it) would cascade delete the access token, which wouldn't be returned by the sql query that was supposed to delete it on its own, and an empty array was passed to the cache invalidation function. --- changelog.d/19483.bugfix | 1 + .../storage/databases/main/registration.py | 19 ++-- tests/rest/client/test_auth.py | 90 ++++++++++++++++++- 3 files changed, 100 insertions(+), 10 deletions(-) create mode 100644 changelog.d/19483.bugfix diff --git a/changelog.d/19483.bugfix b/changelog.d/19483.bugfix new file mode 100644 index 0000000000..9e4fb20996 --- /dev/null +++ b/changelog.d/19483.bugfix @@ -0,0 +1 @@ +Fix access token cache not being invalidated for sessions using refresh tokens. Contributed by @FrenchGithubUser @ Famedly. diff --git a/synapse/storage/databases/main/registration.py b/synapse/storage/databases/main/registration.py index 7b0de92435..2455057b02 100644 --- a/synapse/storage/databases/main/registration.py +++ b/synapse/storage/databases/main/registration.py @@ -2474,14 +2474,6 @@ class RegistrationWorkerStore(StatsStore, CacheInvalidationWorkerStore): def user_delete_access_tokens_for_devices_txn( txn: LoggingTransaction, batch_device_ids: StrCollection ) -> list[tuple[str, int, str | None]]: - self.db_pool.simple_delete_many_txn( - txn, - table="refresh_tokens", - keyvalues={"user_id": user_id}, - column="device_id", - values=batch_device_ids, - ) - clause, args = make_in_list_sql_clause( txn.database_engine, "device_id", batch_device_ids ) @@ -2500,6 +2492,17 @@ class RegistrationWorkerStore(StatsStore, CacheInvalidationWorkerStore): self.get_user_by_access_token, [(t[0],) for t in tokens_and_devices], ) + # Delete access tokens first, before refresh tokens. + # This ensures we can capture the deleted access tokens for cache invalidation + # before any CASCADE deletes occur. + self.db_pool.simple_delete_many_txn( + txn, + table="refresh_tokens", + keyvalues={"user_id": user_id}, + column="device_id", + values=batch_device_ids, + ) + return tokens_and_devices results = [] diff --git a/tests/rest/client/test_auth.py b/tests/rest/client/test_auth.py index c266937a65..4eb918d5c5 100644 --- a/tests/rest/client/test_auth.py +++ b/tests/rest/client/test_auth.py @@ -30,7 +30,14 @@ import synapse.rest.admin from synapse.api.constants import ApprovalNoticeMedium, LoginType from synapse.api.errors import Codes, SynapseError from synapse.handlers.ui_auth.checkers import UserInteractiveAuthChecker -from synapse.rest.client import account, auth, devices, login, logout, register +from synapse.rest.client import ( + account, + auth, + devices, + login, + logout, + register, +) from synapse.rest.synapse.client import build_synapse_client_resource_tree from synapse.server import HomeServer from synapse.storage.database import LoggingTransaction @@ -765,10 +772,11 @@ class RefreshAuthTests(unittest.HomeserverTestCase): servlets = [ auth.register_servlets, account.register_servlets, + devices.register_servlets, login.register_servlets, logout.register_servlets, - synapse.rest.admin.register_servlets_for_client_rest_resource, register.register_servlets, + synapse.rest.admin.register_servlets_for_client_rest_resource, ] hijack_auth = False @@ -1309,6 +1317,84 @@ class RefreshAuthTests(unittest.HomeserverTestCase): self.assertEqual(_table_length("access_tokens"), 0) self.assertEqual(_table_length("refresh_tokens"), 0) + def test_token_invalid_after_refresh_token_issued_and_device_removed(self) -> None: + """ + Test that an access token is invalidated after the device (which had a + refresh token) is removed by another device. + The removal of a refresh token cascade deletes the associated access + token in the db, which can make cache invalidation fail, if not handled + properly. This test will catch such behavior if it ever happens again. + 1. User logs in with device1 + 2. User logs in with device2 and requests a refresh token + 3. Device2 calls /whoami (should work) + 4. Device1 removes device2 + 5. Device2 calls /whoami (should fail) + """ + # Login with device1 + device1_tok = self.login("test", self.user_pass, device_id="device1") + + # Login with device2 and request a refresh token + login_response = self.make_request( + "POST", + "/_matrix/client/r0/login", + { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "device_id": "device2", + "refresh_token": True, + }, + ) + self.assertEqual(login_response.code, HTTPStatus.OK, login_response.result) + device2_tok = login_response.json_body["access_token"] + device2_id = login_response.json_body["device_id"] + self.assertEqual(device2_id, "device2") + + # Device2 calls /whoami (should work) + channel = self.make_request( + "GET", + "/_matrix/client/v3/account/whoami", + access_token=device2_tok, + ) + self.assertEqual(channel.code, HTTPStatus.OK, channel.result) + + # Device1 removes device2 + # First, attempt to delete device2 + delete_channel = self.make_request( + "DELETE", + f"devices/{device2_id}", + access_token=device1_tok, + ) + self.assertEqual( + delete_channel.code, HTTPStatus.UNAUTHORIZED, delete_channel.result + ) + session = delete_channel.json_body["session"] + + # Complete the UI auth flow + delete_channel = self.make_request( + "DELETE", + f"devices/{device2_id}", + content={ + "auth": { + "type": "m.login.password", + "user": "test", + "password": self.user_pass, + "session": session, + }, + }, + access_token=device1_tok, + ) + self.assertEqual(delete_channel.code, HTTPStatus.OK, delete_channel.result) + + # Device2 calls /whoami (should fail) + channel = self.make_request( + "GET", + "/_matrix/client/v3/account/whoami", + access_token=device2_tok, + ) + self.assertEqual(channel.code, HTTPStatus.UNAUTHORIZED, channel.result) + self.assertEqual(channel.json_body["errcode"], "M_UNKNOWN_TOKEN") + def oidc_config( id: str, with_localpart_template: bool, **kwargs: Any From be03be7b50ca914d73757a00d2819807187aa3a8 Mon Sep 17 00:00:00 2001 From: Oleg Girko Date: Wed, 20 May 2026 11:47:43 +0100 Subject: [PATCH 11/12] Partially revert "Bump attrs from 25.4.0 to 26.1.0 (#19684)" (#19789) Accidental bump broke build for Fedora and RHEL. This reverts commit 2e9d6f7f35df53f510b689c09d3a9ace82a0d840. As discussed in the [Synapse Package Maintainers](https://matrix.to/#/!rh9Uxk45AsPongyP3ypgpsCmuIufiggD6mDXFWh4_FM/$0mdulZEyJFdI6bwS8GFwYnFt-zmpyCyx2DwcA8JyuY8?via=jki.re&via=matrix.org&via=element.io) room (private) --- changelog.d/19789.bugfix | 1 + poetry.lock | 53 ++++++++++++++++++++-------------------- pyproject.toml | 2 +- 3 files changed, 29 insertions(+), 27 deletions(-) create mode 100644 changelog.d/19789.bugfix diff --git a/changelog.d/19789.bugfix b/changelog.d/19789.bugfix new file mode 100644 index 0000000000..f6c325ec82 --- /dev/null +++ b/changelog.d/19789.bugfix @@ -0,0 +1 @@ +Fix packaging for Fedora and EPEL caused by unnecessary bumping `attrs` minimum version requirement in `pyproject.toml` file. Contributed by Oleg Girko. diff --git a/poetry.lock b/poetry.lock index ab790015bb..92c25d9308 100644 --- a/poetry.lock +++ b/poetry.lock @@ -31,7 +31,7 @@ description = "The ultimate Python library in building OAuth and OpenID Connect optional = true python-versions = ">=3.9" groups = ["main"] -markers = "extra == \"all\" or extra == \"jwt\" or extra == \"oidc\"" +markers = "extra == \"oidc\" or extra == \"jwt\" or extra == \"all\"" files = [ {file = "authlib-1.6.12-py2.py3-none-any.whl", hash = "sha256:e9229ad7fde610b139dd12f5edbe97eab9ee78bfb85691247e767727850b99ab"}, {file = "authlib-1.6.12.tar.gz", hash = "sha256:0656d8482f28fc8221929d5f35b2bde5d13e10555ebc06b4561b0d622e83b1bd"}, @@ -62,7 +62,7 @@ description = "Backport of CPython tarfile module" optional = false python-versions = ">=3.8" groups = ["dev"] -markers = "platform_machine != \"ppc64le\" and platform_machine != \"s390x\" and python_version < \"3.12\"" +markers = "python_version < \"3.12\" and platform_machine != \"ppc64le\" and platform_machine != \"s390x\"" files = [ {file = "backports.tarfile-1.2.0-py3-none-any.whl", hash = "sha256:77e284d754527b01fb1e6fa8a1afe577858ebe4e9dad8919e34c862cb399bc34"}, {file = "backports_tarfile-1.2.0.tar.gz", hash = "sha256:d75e02c268746e1b8144c278978b6e98e85de6ad16f8e4b0844a154557eca991"}, @@ -531,7 +531,7 @@ description = "XML bomb protection for Python stdlib modules" optional = true python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "defusedxml-0.7.1-py2.py3-none-any.whl", hash = "sha256:a352e7e428770286cc899e2542b6cdaedb2b4953ff269a210103ec58f6198a61"}, {file = "defusedxml-0.7.1.tar.gz", hash = "sha256:1bb3032db185915b62d7c6209c5a8792be6a32ab2fedacc84e01b52c51aa3e69"}, @@ -556,7 +556,7 @@ description = "XPath 1.0/2.0/3.0/3.1 parsers and selectors for ElementTree and l optional = true python-versions = ">=3.8" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "elementpath-4.8.0-py3-none-any.whl", hash = "sha256:5393191f84969bcf8033b05ec4593ef940e58622ea13cefe60ecefbbf09d58d9"}, {file = "elementpath-4.8.0.tar.gz", hash = "sha256:5822a2560d99e2633d95f78694c7ff9646adaa187db520da200a8e9479dc46ae"}, @@ -606,7 +606,7 @@ description = "Python wrapper for hiredis" optional = true python-versions = ">=3.8" groups = ["main"] -markers = "extra == \"all\" or extra == \"redis\"" +markers = "extra == \"redis\" or extra == \"all\"" files = [ {file = "hiredis-3.3.1-cp310-cp310-macosx_10_15_universal2.whl", hash = "sha256:f525734382a47f9828c9d6a1501522c78d5935466d8e2be1a41ba40ca5bb922b"}, {file = "hiredis-3.3.1-cp310-cp310-macosx_10_15_x86_64.whl", hash = "sha256:6e2e1024f0a021777740cb7c633a0efb2c4a4bc570f508223a8dcbcf79f99ef9"}, @@ -889,7 +889,7 @@ description = "Read metadata from Python packages" optional = false python-versions = ">=3.9" groups = ["dev"] -markers = "platform_machine != \"ppc64le\" and platform_machine != \"s390x\" and python_version < \"3.12\"" +markers = "python_version < \"3.12\" and platform_machine != \"ppc64le\" and platform_machine != \"s390x\"" files = [ {file = "importlib_metadata-8.7.1-py3-none-any.whl", hash = "sha256:5a1f80bf1daa489495071efbb095d75a634cf28a8bc299581244063b53176151"}, {file = "importlib_metadata-8.7.1.tar.gz", hash = "sha256:49fef1ae6440c182052f407c8d34a68f72efc36db9ca90dc0113398f2fdde8bb"}, @@ -930,7 +930,7 @@ description = "Jaeger Python OpenTracing Tracer implementation" optional = true python-versions = ">=3.7" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "jaeger-client-4.8.0.tar.gz", hash = "sha256:3157836edab8e2c209bd2d6ae61113db36f7ee399e66b1dcbb715d87ab49bfe0"}, ] @@ -1122,7 +1122,7 @@ description = "A strictly RFC 4510 conforming LDAP V3 pure Python client library optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"matrix-synapse-ldap3\"" +markers = "extra == \"matrix-synapse-ldap3\" or extra == \"all\"" files = [ {file = "ldap3-2.9.1-py2.py3-none-any.whl", hash = "sha256:5869596fc4948797020d3f03b7939da938778a0f9e2009f7a072ccf92b8e8d70"}, {file = "ldap3-2.9.1.tar.gz", hash = "sha256:f3e7fc4718e3f09dda568b57100095e0ce58633bcabbed8667ce3f8fbaa4229f"}, @@ -1239,7 +1239,7 @@ description = "Powerful and Pythonic XML processing library combining libxml2/li optional = true python-versions = ">=3.8" groups = ["main"] -markers = "extra == \"all\" or extra == \"url-preview\"" +markers = "extra == \"url-preview\" or extra == \"all\"" files = [ {file = "lxml-6.0.2-cp310-cp310-macosx_10_9_universal2.whl", hash = "sha256:e77dd455b9a16bbd2a5036a63ddbd479c19572af81b624e79ef422f929eef388"}, {file = "lxml-6.0.2-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:5d444858b9f07cefff6455b983aea9a67f7462ba1f6cbe4a21e8bf6791bf2153"}, @@ -1553,7 +1553,7 @@ description = "An LDAP3 auth provider for Synapse" optional = true python-versions = ">=3.10" groups = ["main"] -markers = "extra == \"all\" or extra == \"matrix-synapse-ldap3\"" +markers = "extra == \"matrix-synapse-ldap3\" or extra == \"all\"" files = [ {file = "matrix_synapse_ldap3-0.4.0-py3-none-any.whl", hash = "sha256:bf080037230d2af5fd3639cb87266de65c1cad7a68ea206278c5b4bf9c1a17f3"}, {file = "matrix_synapse_ldap3-0.4.0.tar.gz", hash = "sha256:cff52ba780170de5e6e8af42863d2648ee23f3bf0a9fea6db52372f9fc00be2b"}, @@ -1834,7 +1834,7 @@ description = "OpenTracing API for Python. See documentation at http://opentraci optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "opentracing-2.4.0.tar.gz", hash = "sha256:a173117e6ef580d55874734d1fa7ecb6f3655160b8b8974a2a1e98e5ec9c840d"}, ] @@ -2032,7 +2032,7 @@ description = "psycopg2 - Python-PostgreSQL Database Adapter" optional = true python-versions = ">=3.9" groups = ["main"] -markers = "extra == \"all\" or extra == \"postgres\"" +markers = "extra == \"postgres\" or extra == \"all\"" files = [ {file = "psycopg2-2.9.11-cp310-cp310-win_amd64.whl", hash = "sha256:103e857f46bb76908768ead4e2d0ba1d1a130e7b8ed77d3ae91e8b33481813e8"}, {file = "psycopg2-2.9.11-cp311-cp311-win_amd64.whl", hash = "sha256:210daed32e18f35e3140a1ebe059ac29209dd96468f2f7559aa59f75ee82a5cb"}, @@ -2050,7 +2050,7 @@ description = ".. image:: https://travis-ci.org/chtd/psycopg2cffi.svg?branch=mas optional = true python-versions = "*" groups = ["main"] -markers = "platform_python_implementation == \"PyPy\" and (extra == \"all\" or extra == \"postgres\")" +markers = "platform_python_implementation == \"PyPy\" and (extra == \"postgres\" or extra == \"all\")" files = [ {file = "psycopg2cffi-2.9.0.tar.gz", hash = "sha256:7e272edcd837de3a1d12b62185eb85c45a19feda9e62fa1b120c54f9e8d35c52"}, ] @@ -2066,7 +2066,7 @@ description = "A Simple library to enable psycopg2 compatability" optional = true python-versions = "*" groups = ["main"] -markers = "platform_python_implementation == \"PyPy\" and (extra == \"all\" or extra == \"postgres\")" +markers = "platform_python_implementation == \"PyPy\" and (extra == \"postgres\" or extra == \"all\")" files = [ {file = "psycopg2cffi-compat-1.1.tar.gz", hash = "sha256:d25e921748475522b33d13420aad5c2831c743227dc1f1f2585e0fdb5c914e05"}, ] @@ -2348,7 +2348,7 @@ description = "A development tool to measure, monitor and analyze the memory beh optional = true python-versions = ">=3.6" groups = ["main"] -markers = "extra == \"all\" or extra == \"cache-memory\"" +markers = "extra == \"cache-memory\" or extra == \"all\"" files = [ {file = "Pympler-1.0.1-py3-none-any.whl", hash = "sha256:d260dda9ae781e1eab6ea15bacb84015849833ba5555f141d2d9b7b7473b307d"}, {file = "Pympler-1.0.1.tar.gz", hash = "sha256:993f1a3599ca3f4fcd7160c7545ad06310c9e12f70174ae7ae8d4e25f6c5d3fa"}, @@ -2480,7 +2480,7 @@ description = "Python implementation of SAML Version 2 Standard" optional = true python-versions = ">=3.9,<4.0" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "pysaml2-7.5.0-py3-none-any.whl", hash = "sha256:bc6627cc344476a83c757f440a73fda1369f13b6fda1b4e16bca63ffbabb5318"}, {file = "pysaml2-7.5.0.tar.gz", hash = "sha256:f36871d4e5ee857c6b85532e942550d2cf90ea4ee943d75eb681044bbc4f54f7"}, @@ -2505,7 +2505,7 @@ description = "Extensions to the standard Python datetime module" optional = true python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,>=2.7" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "python-dateutil-2.9.0.post0.tar.gz", hash = "sha256:37dd54208da7e1cd875388217d5e00ebd4179249f90fb72437e91a35459a0ad3"}, {file = "python_dateutil-2.9.0.post0-py2.py3-none-any.whl", hash = "sha256:a8b2bc7bffae282281c8140a97d3aa9c14da0b136dfe83f850eea9a5f7470427"}, @@ -2533,7 +2533,7 @@ description = "World timezone definitions, modern and historical" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "pytz-2026.1.post1-py2.py3-none-any.whl", hash = "sha256:f2fd16142fda348286a75e1a524be810bb05d444e5a081f37f7affc635035f7a"}, {file = "pytz-2026.1.post1.tar.gz", hash = "sha256:3378dde6a0c3d26719182142c56e60c7f9af7e968076f31aae569d72a0358ee1"}, @@ -2938,7 +2938,7 @@ description = "Python client for Sentry (https://sentry.io)" optional = true python-versions = ">=3.6" groups = ["main"] -markers = "extra == \"all\" or extra == \"sentry\"" +markers = "extra == \"sentry\" or extra == \"all\"" files = [ {file = "sentry_sdk-2.57.0-py2.py3-none-any.whl", hash = "sha256:812c8bf5ff3d2f0e89c82f5ce80ab3a6423e102729c4706af7413fd1eb480585"}, {file = "sentry_sdk-2.57.0.tar.gz", hash = "sha256:4be8d1e71c32fb27f79c577a337ac8912137bba4bcbc64a4ec1da4d6d8dc5199"}, @@ -3138,7 +3138,7 @@ description = "Tornado IOLoop Backed Concurrent Futures" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "threadloop-1.0.2-py2-none-any.whl", hash = "sha256:5c90dbefab6ffbdba26afb4829d2a9df8275d13ac7dc58dccb0e279992679599"}, {file = "threadloop-1.0.2.tar.gz", hash = "sha256:8b180aac31013de13c2ad5c834819771992d350267bddb854613ae77ef571944"}, @@ -3154,7 +3154,7 @@ description = "Python bindings for the Apache Thrift RPC system" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "thrift-0.22.0.tar.gz", hash = "sha256:42e8276afbd5f54fe1d364858b6877bc5e5a4a5ed69f6a005b94ca4918fe1466"}, ] @@ -3220,6 +3220,7 @@ files = [ {file = "tomli-2.4.0-py3-none-any.whl", hash = "sha256:1f776e7d669ebceb01dee46484485f43a4048746235e683bcdffacdf1fb4785a"}, {file = "tomli-2.4.0.tar.gz", hash = "sha256:aa89c3f6c277dd275d8e243ad24f3b5e701491a860d5121f2cdd399fbb31fc9c"}, ] +markers = {main = "python_version < \"3.14\""} [[package]] name = "tornado" @@ -3228,7 +3229,7 @@ description = "Tornado is a Python web framework and asynchronous networking lib optional = true python-versions = ">=3.9" groups = ["main"] -markers = "extra == \"all\" or extra == \"opentracing\"" +markers = "extra == \"opentracing\" or extra == \"all\"" files = [ {file = "tornado-6.5.5-cp39-abi3-macosx_10_9_universal2.whl", hash = "sha256:487dc9cc380e29f58c7ab88f9e27cdeef04b2140862e5076a66fb6bb68bb1bfa"}, {file = "tornado-6.5.5-cp39-abi3-macosx_10_9_x86_64.whl", hash = "sha256:65a7f1d46d4bb41df1ac99f5fcb685fb25c7e61613742d5108b010975a9a6521"}, @@ -3360,7 +3361,7 @@ description = "non-blocking redis client for python" optional = true python-versions = "*" groups = ["main"] -markers = "extra == \"all\" or extra == \"redis\"" +markers = "extra == \"redis\" or extra == \"all\"" files = [ {file = "txredisapi-1.4.11-py3-none-any.whl", hash = "sha256:ac64d7a9342b58edca13ef267d4fa7637c1aa63f8595e066801c1e8b56b22d0b"}, {file = "txredisapi-1.4.11.tar.gz", hash = "sha256:3eb1af99aefdefb59eb877b1dd08861efad60915e30ad5bf3d5bf6c5cedcdbc6"}, @@ -3621,7 +3622,7 @@ description = "An XML Schema validator and decoder" optional = true python-versions = ">=3.7" groups = ["main"] -markers = "extra == \"all\" or extra == \"saml2\"" +markers = "extra == \"saml2\" or extra == \"all\"" files = [ {file = "xmlschema-2.5.1-py3-none-any.whl", hash = "sha256:ec2b2a15c8896c1fcd14dcee34ca30032b99456c3c43ce793fdb9dca2fb4b869"}, {file = "xmlschema-2.5.1.tar.gz", hash = "sha256:4f7497de6c8b6dc2c28ad7b9ed6e21d186f4afe248a5bea4f54eedab4da44083"}, @@ -3642,7 +3643,7 @@ description = "Backport of pathlib-compatible object wrapper for zip files" optional = false python-versions = ">=3.9" groups = ["dev"] -markers = "platform_machine != \"ppc64le\" and platform_machine != \"s390x\" and python_version < \"3.12\"" +markers = "python_version < \"3.12\" and platform_machine != \"ppc64le\" and platform_machine != \"s390x\"" files = [ {file = "zipp-3.23.0-py3-none-any.whl", hash = "sha256:071652d6115ed432f5ce1d34c336c0adfd6a884660d1e9712a256d3d3bd4b14e"}, {file = "zipp-3.23.0.tar.gz", hash = "sha256:a07157588a12518c9d4034df3fbbee09c814741a33ff63c05fa29d26a2404166"}, @@ -3755,4 +3756,4 @@ url-preview = ["lxml"] [metadata] lock-version = "2.1" python-versions = ">=3.10.0,<4.0.0" -content-hash = "d97bee07fec0f4048d964aa7127a50813920bce77b00e5191aa1815f83922c85" +content-hash = "ef0540b89c417a69668f551688bd0974256ea7a580044f3954a76bdf0d8fe7c9" diff --git a/pyproject.toml b/pyproject.toml index 66cd3e83b5..e92e7b5c21 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,7 +66,7 @@ dependencies = [ "prometheus-client>=0.6.0", # we use `order`, which arrived in attrs 19.2.0. # Note: 21.1.0 broke `/sync`, see https://github.com/matrix-org/synapse/issues/9936 - "attrs>=26.1.0,!=21.1.0", + "attrs>=19.2.0,!=21.1.0", "netaddr>=0.7.18", # Jinja 2.x is incompatible with MarkupSafe>=2.1. To ensure that admins do not # end up with a broken installation, with recent MarkupSafe but old Jinja, we From 966e193e4e1900bc28891ed697aee30ab5a72dbd Mon Sep 17 00:00:00 2001 From: Joe Groocock Date: Wed, 20 May 2026 12:10:54 +0100 Subject: [PATCH 12/12] Fix sending heroes in SSS when `m.room.name=""` (#19468) As per the spec, a room with m.room.name value that is absent, null or empty should be treated as if there is no m.room.name event at all: https://spec.matrix.org/v1.17/client-server-api/#mroomname This fetches the full m.room.name event and checks the content.name instead of only checking the existence of the m.room.name event. This results in correctly sending heroes for those rooms. Fixes: https://github.com/element-hq/synapse/issues/19447 Signed-off-by: Joe Groocock --- changelog.d/19468.bugfix | 1 + synapse/handlers/sliding_sync/__init__.py | 51 +++++++------- .../client/sliding_sync/test_rooms_meta.py | 70 +++++++++++++++++++ 3 files changed, 98 insertions(+), 24 deletions(-) create mode 100644 changelog.d/19468.bugfix diff --git a/changelog.d/19468.bugfix b/changelog.d/19468.bugfix new file mode 100644 index 0000000000..003716d296 --- /dev/null +++ b/changelog.d/19468.bugfix @@ -0,0 +1 @@ +Fix a bug in [MSC4186: Simplified Sliding Sync](https://github.com/matrix-org/matrix-spec-proposals/pull/4186) that could prevent user avatars from showing if the room had an empty name. diff --git a/synapse/handlers/sliding_sync/__init__.py b/synapse/handlers/sliding_sync/__init__.py index ddce04d5b3..df1270f7f9 100644 --- a/synapse/handlers/sliding_sync/__init__.py +++ b/synapse/handlers/sliding_sync/__init__.py @@ -872,20 +872,10 @@ class SlidingSyncHandler: # For incremental syncs, we can do this first to determine if something relevant # has changed and strategically avoid fetching other costly things. room_state_delta_id_map: MutableStateMap[str] = {} - name_event_id: str | None = None membership_changed = False name_changed = False avatar_changed = False - if initial: - # Check whether the room has a name set - name_state_ids = await self.get_current_state_ids_at( - room_id=room_id, - room_membership_for_user_at_to_token=room_membership_for_user_at_to_token, - state_filter=StateFilter.from_types([(EventTypes.Name, "")]), - to_token=to_token, - ) - name_event_id = name_state_ids.get((EventTypes.Name, "")) - else: + if not initial: assert from_bound is not None # TODO: Limit the number of state events we're about to send down @@ -933,6 +923,27 @@ class SlidingSyncHandler: ): avatar_changed = True + # If a room has an m.room.name event with an absent, null, or empty + # name field, it should be treated the same as a room with no + # m.room.name event. + # https://spec.matrix.org/v1.17/client-server-api/#mroomname + # + # TODO: Should we also check for `EventTypes.CanonicalAlias` + # (`m.room.canonical_alias`) as a fallback for the room name? see + # https://github.com/matrix-org/matrix-spec-proposals/pull/4186/changes#r2860107511 + room_name: str | None = None + if initial or name_changed: + # Check whether the room has a name set + name_states = await self.get_current_state_at( + room_id=room_id, + room_membership_for_user_at_to_token=room_membership_for_user_at_to_token, + state_filter=StateFilter.from_types([(EventTypes.Name, "")]), + to_token=to_token, + ) + name_event = name_states.get((EventTypes.Name, "")) + if name_event is not None: + room_name = name_event.content.get("name") + # We only need the room summary for calculating heroes, however if we do # fetch it then we can use it to calculate `joined_count` and # `invited_count`. @@ -949,12 +960,13 @@ class SlidingSyncHandler: hero_user_ids: list[str] = [] # TODO: Should we also check for `EventTypes.CanonicalAlias` # (`m.room.canonical_alias`) as a fallback for the room name? see - # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1671260153 + # https://github.com/matrix-org/matrix-spec-proposals/pull/4186/changes#r2860107511 # - # We need to fetch the `heroes` if the room name is not set. But we only need to - # get them on initial syncs (or the first time we send down the room) or if the + # We need to fetch the `heroes` if the room name is not set (taking + # care to treat an empty string as unset). But we only need to get them + # on initial syncs (or the first time we send down the room) or if the # membership has changed which may change the heroes. - if name_event_id is None and (initial or (not initial and membership_changed)): + if not room_name and (initial or (not initial and membership_changed)): # We need the room summary to extract the heroes from if room_membership_for_user_at_to_token.membership != Membership.JOIN: # TODO: Figure out how to get the membership summary for left/banned rooms @@ -1332,15 +1344,6 @@ class SlidingSyncHandler: if required_state_filter != StateFilter.none(): required_room_state = required_state_filter.filter_state(room_state) - # Find the room name and avatar from the state - room_name: str | None = None - # TODO: Should we also check for `EventTypes.CanonicalAlias` - # (`m.room.canonical_alias`) as a fallback for the room name? see - # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1671260153 - name_event = room_state.get((EventTypes.Name, "")) - if name_event is not None: - room_name = name_event.content.get("name") - room_avatar: str | None = None avatar_event = room_state.get((EventTypes.RoomAvatar, "")) if avatar_event is not None: diff --git a/tests/rest/client/sliding_sync/test_rooms_meta.py b/tests/rest/client/sliding_sync/test_rooms_meta.py index 4ff07b4d26..b1b771ef84 100644 --- a/tests/rest/client/sliding_sync/test_rooms_meta.py +++ b/tests/rest/client/sliding_sync/test_rooms_meta.py @@ -631,6 +631,76 @@ class SlidingSyncRoomsMetaTestCase(SlidingSyncBase): # We didn't request any state so we shouldn't see any `required_state` self.assertIsNone(response_body["rooms"][room_id1].get("required_state")) + def test_rooms_meta_heroes_empty_room_name(self) -> None: + """ + Test that the `rooms` `heroes` are included when the room name is an + empty string (i.e. unset as per the spec) + """ + user1_id = self.register_user("user1", "pass") + user1_tok = self.login(user1_id, "pass") + user2_id = self.register_user("user2", "pass") + user2_tok = self.login(user2_id, "pass") + user3_id = self.register_user("user3", "pass") + _user3_tok = self.login(user3_id, "pass") + + room_id = self.helper.create_room_as( + user2_id, + tok=user2_tok, + extra_content={ + # https://spec.matrix.org/v1.17/client-server-api/#mroomname + # > If a room has an m.room.name event with an absent, null, or + # > empty name field, it should be treated the same as a room + # > with no m.room.name event. + "name": "", + }, + ) + self.helper.join(room_id, user1_id, tok=user1_tok) + # User3 is invited + self.helper.invite(room_id, src=user2_id, targ=user3_id, tok=user2_tok) + + # Make the Sliding Sync request + sync_body = { + "lists": { + "foo-list": { + "ranges": [[0, 1]], + "required_state": [], + "timeline_limit": 1, + } + } + } + response_body, from_token = self.do_sync(sync_body, tok=user1_tok) + + # Room has an empty name so we should see `heroes` populated + self.assertEqual(response_body["rooms"][room_id]["initial"], True) + self.assertIsNone(response_body["rooms"][room_id].get("name")) + self.assertCountEqual( + [ + hero["user_id"] + for hero in response_body["rooms"][room_id].get("heroes", []) + ], + # Heroes shouldn't include the user themselves (we shouldn't see user1) + [user2_id, user3_id], + ) + self.assertEqual( + response_body["rooms"][room_id]["joined_count"], + 2, + ) + self.assertEqual( + response_body["rooms"][room_id]["invited_count"], + 1, + ) + + # We didn't request any state so we shouldn't see any `required_state` + self.assertIsNone(response_body["rooms"][room_id].get("required_state")) + + # Send a message to make the room come down sync + self.helper.send(room_id, "message in room", tok=user2_tok) + + # Incremental sync + incremental_body, _ = self.do_sync(sync_body, since=from_token, tok=user1_tok) + self.assertNotIn("name", incremental_body["rooms"][room_id]) + self.assertNotIn("heroes", incremental_body["rooms"][room_id]) + def test_rooms_meta_heroes_when_banned(self) -> None: """ Test that the `rooms` `heroes` are included in the response when the room