From 3ff83ed9fa72b64603fc849f73446729946caa66 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 03:14:13 +0200 Subject: [PATCH 1/8] Implement pagination for MSC2666 --- synapse/rest/client/mutual_rooms.py | 69 +++++++++++++++++++++----- tests/rest/client/test_mutual_rooms.py | 32 +++++++++++- 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/synapse/rest/client/mutual_rooms.py b/synapse/rest/client/mutual_rooms.py index 3e5316c4b7f..733e0749c22 100644 --- a/synapse/rest/client/mutual_rooms.py +++ b/synapse/rest/client/mutual_rooms.py @@ -19,9 +19,12 @@ # # import logging +from bisect import bisect from http import HTTPStatus from typing import TYPE_CHECKING +from unpaddedbase64 import decode_base64, encode_base64 + from synapse.api.errors import Codes, SynapseError from synapse.http.server import HttpServer from synapse.http.servlet import RestServlet, parse_strings_from_args @@ -35,6 +38,30 @@ logger = logging.getLogger(__name__) +MUTUAL_ROOMS_BATCH_LIMIT = 100 + + +def _parse_mutual_rooms_batch_token_args(args: dict[bytes, list[bytes]]) -> str | None: + from_batches = parse_strings_from_args(args, "from") + if not from_batches: + return None + if len(from_batches) > 1: + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "Duplicate from query parameter", + errcode=Codes.INVALID_PARAM, + ) + if from_batches[0]: + try: + return decode_base64(from_batches[0]).decode("utf-8") + except Exception: + raise SynapseError( + HTTPStatus.BAD_REQUEST, + "Malformed from token", + errcode=Codes.INVALID_PARAM, + ) + return None + class UserMutualRoomsServlet(RestServlet): """ @@ -56,6 +83,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: args: dict[bytes, list[bytes]] = request.args # type: ignore user_ids = parse_strings_from_args(args, "user_id", required=True) + from_batch = _parse_mutual_rooms_batch_token_args(args) if len(user_ids) > 1: raise SynapseError( @@ -64,29 +92,44 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: errcode=Codes.INVALID_PARAM, ) - # We don't do batching, so a batch token is illegal by default - if b"batch_token" in args: - raise SynapseError( - HTTPStatus.BAD_REQUEST, - "Unknown batch_token", - errcode=Codes.INVALID_PARAM, - ) - user_id = user_ids[0] requester = await self.auth.get_user_by_req(request) if user_id == requester.user.to_string(): raise SynapseError( - HTTPStatus.UNPROCESSABLE_ENTITY, + HTTPStatus.BAD_REQUEST, "You cannot request a list of shared rooms with yourself", - errcode=Codes.INVALID_PARAM, + errcode=Codes.UNKNOWN, ) - rooms = await self.store.get_mutual_rooms_between_users( - frozenset((requester.user.to_string(), user_id)) + rooms = sorted( + await self.store.get_mutual_rooms_between_users( + frozenset((requester.user.to_string(), user_id)) + ) ) - return 200, {"joined": list(rooms)} + if from_batch: + # A from_batch token was provided, so cut off any rooms where the ID is + # lower than or equal to the token + rooms = rooms[bisect(rooms, from_batch) :] + + if len(rooms) < MUTUAL_ROOMS_BATCH_LIMIT: + # We've reached the end of the list, don't return a batch token + return 200, {"joined": rooms} + + rooms = rooms[:MUTUAL_ROOMS_BATCH_LIMIT] + # We use urlsafe unpadded base64 encoding for the batch token in order to + # handle funny room IDs in old pre-v12 rooms properly. We also truncate it + # to stay within the 255-character limit of opaque tokens. + next_batch = encode_base64(rooms[-1].encode("utf-8"), urlsafe=True)[:255] + # Due to the truncation, it is technically possible to have conflicting next + # batches by creating hundreds of rooms with the same 191 character prefix + # in the room ID. In the event that some silly user does that, don't let + # them paginate further. + if next_batch == from_batch: + return 200, {"joined": rooms} + + return 200, {"joined": list(rooms), "next_batch": next_batch} def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None: diff --git a/tests/rest/client/test_mutual_rooms.py b/tests/rest/client/test_mutual_rooms.py index ea063707aa3..bba8af724a6 100644 --- a/tests/rest/client/test_mutual_rooms.py +++ b/tests/rest/client/test_mutual_rooms.py @@ -55,12 +55,16 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.store = hs.get_datastores().main + mutual_rooms.MUTUAL_ROOMS_BATCH_LIMIT = 10 - def _get_mutual_rooms(self, token: str, other_user: str) -> FakeChannel: + def _get_mutual_rooms( + self, token: str, other_user: str, since_token: str | None = None + ) -> FakeChannel: return self.make_request( "GET", "/_matrix/client/unstable/uk.half-shot.msc2666/user/mutual_rooms" - f"?user_id={quote(other_user)}", + f"?user_id={quote(other_user)}" + + (f"&from={quote(since_token)}" if since_token else ""), access_token=token, ) @@ -141,6 +145,30 @@ def _check_mutual_rooms_with( for room_id_id in channel.json_body["joined"]: self.assertIn(room_id_id, [room_id_one, room_id_two]) + def test_shared_room_list_pagination(self) -> None: + u1 = self.register_user("user1", "pass") + u1_token = self.login(u1, "pass") + u2 = self.register_user("user2", "pass") + u2_token = self.login(u2, "pass") + self.get_success(self.hs.get_datastores().main.set_ratelimit_for_user(u1, 0, 0)) + room_ids = [] + for i in range(15): + room_id = self.helper.create_room_as(u1, is_public=i % 2 == 0, tok=u1_token) + self.helper.invite(room_id, src=u1, targ=u2, tok=u1_token) + self.helper.join(room_id, user=u2, tok=u2_token) + room_ids.append(room_id) + room_ids.sort() + + channel = self._get_mutual_rooms(u1_token, u2) + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(channel.json_body["joined"], room_ids[0:10]) + self.assertIn("next_batch", channel.json_body) + + channel = self._get_mutual_rooms(u1_token, u2, channel.json_body["next_batch"]) + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(channel.json_body["joined"], room_ids[10:20]) + self.assertNotIn("next_batch", channel.json_body) + def test_shared_room_list_after_leave(self) -> None: """ A room should no longer be considered shared if the other From 2357d3801a639140e29d825192b097c8a60ce3ec Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 03:16:05 +0200 Subject: [PATCH 2/8] Add changelog --- changelog.d/19279.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/19279.feature diff --git a/changelog.d/19279.feature b/changelog.d/19279.feature new file mode 100644 index 00000000000..031e48dcebf --- /dev/null +++ b/changelog.d/19279.feature @@ -0,0 +1 @@ +Implemented pagination for the [MSC2666](https://github.com/matrix-org/matrix-spec-proposals/pull/2666) mutual rooms endpoint. Contributed by @tulir @ Beeper. From 3b977e1a3402741546e47e34d2a5ba95343ab538 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 13:47:40 +0200 Subject: [PATCH 3/8] Apply suggestions from code review Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- synapse/rest/client/mutual_rooms.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/rest/client/mutual_rooms.py b/synapse/rest/client/mutual_rooms.py index 733e0749c22..6834ee6be05 100644 --- a/synapse/rest/client/mutual_rooms.py +++ b/synapse/rest/client/mutual_rooms.py @@ -102,6 +102,8 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: errcode=Codes.UNKNOWN, ) + # Sort here instead of the database function, so that we don't expose + # clients to any unrelated changes to the sorting algorithm. rooms = sorted( await self.store.get_mutual_rooms_between_users( frozenset((requester.user.to_string(), user_id)) @@ -113,7 +115,7 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: # lower than or equal to the token rooms = rooms[bisect(rooms, from_batch) :] - if len(rooms) < MUTUAL_ROOMS_BATCH_LIMIT: + if len(rooms) <= MUTUAL_ROOMS_BATCH_LIMIT: # We've reached the end of the list, don't return a batch token return 200, {"joined": rooms} From af072d9043c59f6356b81c4fd6415883ff1d887d Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 13:48:35 +0200 Subject: [PATCH 4/8] Update docstring --- synapse/rest/client/mutual_rooms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/mutual_rooms.py b/synapse/rest/client/mutual_rooms.py index 6834ee6be05..98d407918bc 100644 --- a/synapse/rest/client/mutual_rooms.py +++ b/synapse/rest/client/mutual_rooms.py @@ -65,7 +65,7 @@ def _parse_mutual_rooms_batch_token_args(args: dict[bytes, list[bytes]]) -> str class UserMutualRoomsServlet(RestServlet): """ - GET /uk.half-shot.msc2666/user/mutual_rooms?user_id={user_id} HTTP/1.1 + GET /uk.half-shot.msc2666/user/mutual_rooms?user_id={user_id}&from={token} HTTP/1.1 """ PATTERNS = client_patterns( From 1a779e87507c028d97c895ec7cf1e85f3423d735 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 13:51:30 +0200 Subject: [PATCH 5/8] Clarify features and limitations of token format --- synapse/rest/client/mutual_rooms.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/synapse/rest/client/mutual_rooms.py b/synapse/rest/client/mutual_rooms.py index 98d407918bc..a6a913db34d 100644 --- a/synapse/rest/client/mutual_rooms.py +++ b/synapse/rest/client/mutual_rooms.py @@ -112,7 +112,13 @@ async def on_GET(self, request: SynapseRequest) -> tuple[int, JsonDict]: if from_batch: # A from_batch token was provided, so cut off any rooms where the ID is - # lower than or equal to the token + # lower than or equal to the token. This method doesn't care whether the + # provided token room still exists, nor whether it's even a real room ID. + # + # However, if rooms with a lower ID are added after the token was issued, + # they will not be included until the client makes a new request without a + # from token. This is considered acceptable, as clients generally won't + # persist these results for long periods. rooms = rooms[bisect(rooms, from_batch) :] if len(rooms) <= MUTUAL_ROOMS_BATCH_LIMIT: From c8eb6fcea60391fa0f075dd13dd19a47b34ec044 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 13:56:00 +0200 Subject: [PATCH 6/8] Add more tests --- tests/rest/client/test_mutual_rooms.py | 34 ++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_mutual_rooms.py b/tests/rest/client/test_mutual_rooms.py index bba8af724a6..1de09cb46b5 100644 --- a/tests/rest/client/test_mutual_rooms.py +++ b/tests/rest/client/test_mutual_rooms.py @@ -145,19 +145,23 @@ def _check_mutual_rooms_with( for room_id_id in channel.json_body["joined"]: self.assertIn(room_id_id, [room_id_one, room_id_two]) - def test_shared_room_list_pagination(self) -> None: + def _create_rooms_for_pagination_test(self, count: int) -> tuple[str, str, list[str]]: u1 = self.register_user("user1", "pass") u1_token = self.login(u1, "pass") u2 = self.register_user("user2", "pass") u2_token = self.login(u2, "pass") self.get_success(self.hs.get_datastores().main.set_ratelimit_for_user(u1, 0, 0)) room_ids = [] - for i in range(15): + for i in range(count): room_id = self.helper.create_room_as(u1, is_public=i % 2 == 0, tok=u1_token) self.helper.invite(room_id, src=u1, targ=u2, tok=u1_token) self.helper.join(room_id, user=u2, tok=u2_token) room_ids.append(room_id) room_ids.sort() + return u1_token, u2, room_ids + + def test_shared_room_list_pagination_two_pages(self) -> None: + u1_token, u2, room_ids = self._create_rooms_for_pagination_test(15) channel = self._get_mutual_rooms(u1_token, u2) self.assertEqual(200, channel.code, channel.result) @@ -169,6 +173,21 @@ def test_shared_room_list_pagination(self) -> None: self.assertEqual(channel.json_body["joined"], room_ids[10:20]) self.assertNotIn("next_batch", channel.json_body) + def test_shared_room_list_pagination_one_page(self) -> None: + u1_token, u2, room_ids = self._create_rooms_for_pagination_test(10) + + channel = self._get_mutual_rooms(u1_token, u2) + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(channel.json_body["joined"], room_ids) + self.assertNotIn("next_batch", channel.json_body) + + def test_shared_room_list_pagination_invalid_token(self) -> None: + u1_token, u2, room_ids = self._create_rooms_for_pagination_test(10) + + channel = self._get_mutual_rooms(u1_token, u2, "!<>##faketoken") + self.assertEqual(400, channel.code, channel.result) + self.assertEqual("M_INVALID_PARAM", channel.json_body["errcode"], channel.result) + def test_shared_room_list_after_leave(self) -> None: """ A room should no longer be considered shared if the other @@ -200,3 +219,14 @@ def test_shared_room_list_after_leave(self) -> None: channel = self._get_mutual_rooms(u2_token, u1) self.assertEqual(200, channel.code, channel.result) self.assertEqual(len(channel.json_body["joined"]), 0) + + def test_shared_room_list_nonexistent_user(self) -> None: + u1 = self.register_user("user1", "pass") + u1_token = self.login(u1, "pass") + + # Check shared rooms from user1's perspective. + # We should see the one room in common + channel = self._get_mutual_rooms(u1_token, "@meow:example.com") + self.assertEqual(200, channel.code, channel.result) + self.assertEqual(len(channel.json_body["joined"]), 0) + self.assertNotIn("next_batch", channel.json_body) From bed637f26f6916cf3f8f5b06b9c58d61f114d35e Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 13:58:37 +0200 Subject: [PATCH 7/8] Remove per-issuer invite ratelimits and room creation ratelimits in test config --- tests/rest/client/test_mutual_rooms.py | 1 - tests/utils.py | 2 ++ 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/rest/client/test_mutual_rooms.py b/tests/rest/client/test_mutual_rooms.py index 1de09cb46b5..2117fe95346 100644 --- a/tests/rest/client/test_mutual_rooms.py +++ b/tests/rest/client/test_mutual_rooms.py @@ -150,7 +150,6 @@ def _create_rooms_for_pagination_test(self, count: int) -> tuple[str, str, list[ u1_token = self.login(u1, "pass") u2 = self.register_user("user2", "pass") u2_token = self.login(u2, "pass") - self.get_success(self.hs.get_datastores().main.set_ratelimit_for_user(u1, 0, 0)) room_ids = [] for i in range(count): room_id = self.helper.create_room_as(u1, is_public=i % 2 == 0, tok=u1_token) diff --git a/tests/utils.py b/tests/utils.py index 4052c9a4fbc..0cf97a7e8d6 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -198,7 +198,9 @@ def default_config( "rc_invites": { "per_room": {"per_second": 10000, "burst_count": 10000}, "per_user": {"per_second": 10000, "burst_count": 10000}, + "per_issuer": {"per_second": 10000, "burst_count": 10000}, }, + "rc_room_creation": {"per_second": 10000, "burst_count": 10000}, "rc_3pid_validation": {"per_second": 10000, "burst_count": 10000}, "rc_presence": {"per_user": {"per_second": 10000, "burst_count": 10000}}, "saml2_enabled": False, From 54a4e07107a7f9b869703421086ce2caaee28422 Mon Sep 17 00:00:00 2001 From: Tulir Asokan Date: Fri, 5 Dec 2025 14:03:19 +0200 Subject: [PATCH 8/8] Fix lint --- tests/rest/client/test_mutual_rooms.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/rest/client/test_mutual_rooms.py b/tests/rest/client/test_mutual_rooms.py index 2117fe95346..f78c67fcd97 100644 --- a/tests/rest/client/test_mutual_rooms.py +++ b/tests/rest/client/test_mutual_rooms.py @@ -145,7 +145,9 @@ def _check_mutual_rooms_with( for room_id_id in channel.json_body["joined"]: self.assertIn(room_id_id, [room_id_one, room_id_two]) - def _create_rooms_for_pagination_test(self, count: int) -> tuple[str, str, list[str]]: + def _create_rooms_for_pagination_test( + self, count: int + ) -> tuple[str, str, list[str]]: u1 = self.register_user("user1", "pass") u1_token = self.login(u1, "pass") u2 = self.register_user("user2", "pass") @@ -185,7 +187,9 @@ def test_shared_room_list_pagination_invalid_token(self) -> None: channel = self._get_mutual_rooms(u1_token, u2, "!<>##faketoken") self.assertEqual(400, channel.code, channel.result) - self.assertEqual("M_INVALID_PARAM", channel.json_body["errcode"], channel.result) + self.assertEqual( + "M_INVALID_PARAM", channel.json_body["errcode"], channel.result + ) def test_shared_room_list_after_leave(self) -> None: """