From e1e700b7f4e155a4e270ad38d78866dc2f443dc6 Mon Sep 17 00:00:00 2001 From: FrenchGithubUser Date: Tue, 30 Jun 2026 17:10:37 +0200 Subject: [PATCH 1/3] fix: third-party (3pid) invites over federation failing intermittently in version 12 rooms, whose room IDs no longer encode a server name --- changelog.d/19898.bugfix | 1 + synapse/handlers/federation.py | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changelog.d/19898.bugfix diff --git a/changelog.d/19898.bugfix b/changelog.d/19898.bugfix new file mode 100644 index 00000000000..134847bede2 --- /dev/null +++ b/changelog.d/19898.bugfix @@ -0,0 +1 @@ +Fix third-party (3pid) invites over federation failing intermittently in version 12 rooms, whose room IDs no longer encode a server name. diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index ba83d4fd268..aacbd00afe9 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1539,7 +1539,14 @@ async def exchange_third_party_invite( if i == max_retries - 1: raise e else: - destinations = {x.split(":", 1)[-1] for x in (sender_user_id, room_id)} + # The sender always tells us a server to try. Pre-v12 room IDs also + # encode the resident server's domain, but v12+ room IDs are a hash + # with no domain component, so we must not treat them as a server + # name -- doing so raises an invalid-destination error which can + # abort the whole exchange before the valid destination is tried. + destinations = {get_domain_from_id(sender_user_id)} + if ":" in room_id: + destinations.add(get_domain_from_id(room_id)) try: await self.federation_client.forward_third_party_invite( From ec8f6c4323242bbeab596c927769aa17a3f99b49 Mon Sep 17 00:00:00 2001 From: FrenchGithubUser Date: Wed, 1 Jul 2026 14:41:19 +0200 Subject: [PATCH 2/3] fix: `/createRoom` intermittently failing with a 500 error in version 12 rooms when the same user creates several rooms at once, due to colliding room IDs --- changelog.d/19898.bugfix | 2 ++ synapse/handlers/room.py | 66 ++++++++++++++++++++++++++++------------ 2 files changed, 48 insertions(+), 20 deletions(-) diff --git a/changelog.d/19898.bugfix b/changelog.d/19898.bugfix index 134847bede2..2081f4c9e8b 100644 --- a/changelog.d/19898.bugfix +++ b/changelog.d/19898.bugfix @@ -1 +1,3 @@ Fix third-party (3pid) invites over federation failing intermittently in version 12 rooms, whose room IDs no longer encode a server name. + +Fix `/createRoom` intermittently failing with a 500 error in version 12 rooms when the same user creates several rooms at once, due to colliding room IDs. \ No newline at end of file diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index c75fff91706..4c9b86ebe35 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -1409,30 +1409,56 @@ async def _generate_create_event_for_room_id( is_public: bool, room_version: RoomVersion, ) -> tuple[EventBase, synapse.events.snapshot.EventContext]: - ( - creation_event, - new_unpersisted_context, - ) = await self.event_creation_handler.create_event( - creator, - { + # In MSC4291 rooms the room ID is the reference hash of the create event, + # so two rooms whose create events have identical content collide on the + # same room ID. This happens in practice when the same user creates + # several rooms at once (e.g. concurrent /createRoom requests with the + # same config), as the only entropy in the create event is + # `origin_server_ts`, which has millisecond resolution. Retry a few times + # on collision, perturbing `origin_server_ts` so the create event hashes + # to a fresh room ID. This mirrors the collision handling in + # `_generate_and_create_room_id` used for older room versions. + attempts = 0 + while attempts < 5: + event_dict: JsonDict = { "content": creation_content, "sender": creator.user.to_string(), "type": EventTypes.Create, "state_key": "", - }, - prev_event_ids=[], - depth=1, - state_map={}, - for_batch=False, - ) - await self.store.store_room( - room_id=creation_event.room_id, - room_creator_user_id=creator.user.to_string(), - is_public=is_public, - room_version=room_version, - ) - creation_context = await new_unpersisted_context.persist(creation_event) - return (creation_event, creation_context) + } + if attempts > 0: + # Bump the timestamp to give the create event (and hence the + # room ID it hashes to) different content from the colliding one. + event_dict["origin_server_ts"] = ( + self.clock.time_msec() + random.randint(1, 10) + ) + + ( + creation_event, + new_unpersisted_context, + ) = await self.event_creation_handler.create_event( + creator, + event_dict, + prev_event_ids=[], + depth=1, + state_map={}, + for_batch=False, + ) + try: + await self.store.store_room( + room_id=creation_event.room_id, + room_creator_user_id=creator.user.to_string(), + is_public=is_public, + room_version=room_version, + ) + except StoreError: + attempts += 1 + continue + + creation_context = await new_unpersisted_context.persist(creation_event) + return (creation_event, creation_context) + + raise StoreError(500, "Couldn't generate a unique room ID.") async def _send_events_for_new_room( self, From 16d8f8f7241f8302d4659f3ca91605ae149efa56 Mon Sep 17 00:00:00 2001 From: FrenchGithubUser Date: Thu, 16 Jul 2026 17:22:34 +0200 Subject: [PATCH 3/3] tests: add regression tests as suggested by copilot --- tests/handlers/test_federation.py | 40 ++++++++++++++++++++++++++++++ tests/handlers/test_room.py | 41 +++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index 794c0a3185f..e3ef6268747 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -108,6 +108,46 @@ def test_exchange_revoked_invite(self) -> None: self.assertEqual(failure.errcode, Codes.FORBIDDEN, failure) self.assertEqual(failure.msg, "You are not invited to this room.") + def test_exchange_third_party_invite_forwards_to_sender_for_v12_room( + self, + ) -> None: + """When we are not resident in the room, a 3pid invite is forwarded to + a remote server for exchange. Pre-v12 room IDs encode the resident + server's domain, but v12+ room IDs are a content hash with no domain, + so the room ID must not be treated as a destination as doing so raises + an invalid-destination error and aborts the exchange. + + Regression test for 3pid invites over federation failing intermittently + in v12 rooms. + """ + sender_user_id = "@sender:remote.example.com" + # A v12-style room ID: a reference hash with no ":domain" suffix. + room_id = "!somereferencehashwithnodomain" + + # Pretend we're not in the room so we take the "forward to a remote + # server" branch. + self.handler._event_auth_handler.is_host_in_room = AsyncMock( # type: ignore[method-assign] + return_value=False + ) + forward = AsyncMock(return_value=None) + self.handler.federation_client.forward_third_party_invite = forward # type: ignore[method-assign] + + self.get_success( + self.handler.exchange_third_party_invite( + sender_user_id=sender_user_id, + target_user_id="@target:localhost", + room_id=room_id, + signed={"mxid": "@target:localhost", "token": "sometoken"}, + ) + ) + + forward.assert_called_once() + destinations = forward.call_args.args[0] + # Only the sender's server is a valid destination; the domainless room + # ID must not be misinterpreted as one. + self.assertEqual(destinations, {"remote.example.com"}) + self.assertNotIn(room_id, destinations) + def test_rejected_message_event_state(self) -> None: """ Check that we store the state group correctly for rejected non-state events. diff --git a/tests/handlers/test_room.py b/tests/handlers/test_room.py index df95490d3b4..500f4c89157 100644 --- a/tests/handlers/test_room.py +++ b/tests/handlers/test_room.py @@ -1,6 +1,9 @@ +from unittest.mock import patch + import synapse from synapse.api.constants import EventTypes, RoomEncryptionAlgorithms from synapse.rest.client import login, room +from synapse.types import create_requester from tests import unittest from tests.unittest import override_config @@ -106,3 +109,41 @@ def test_encrypted_by_default_config_option_off(self) -> None: tok=user_token, expect_code=404, ) + + +class RoomIDCollisionTestCase(unittest.HomeserverTestCase): + servlets = [ + login.register_servlets, + synapse.rest.admin.register_servlets_for_client_rest_resource, + room.register_servlets, + ] + + def test_colliding_v12_room_ids_are_retried(self) -> None: + """In v12 room the room ID is the reference hash of the + create event, so two rooms whose create events have identical content + collide on the same room ID. This happens when the same user creates + several rooms at once (e.g. concurrent /createRoom requests with the + same config within the same millisecond). + + Regression test: the collision must be retried transparently and both + rooms created with distinct IDs. + """ + handler = self.hs.get_room_creation_handler() + user_id = self.register_user("alice", "pass") + requester = create_requester(user_id) + + # Freeze the clock so both create events carry the same + # `origin_server_ts`; with identical config this forces the two room + # IDs to hash to the same value, reproducing the collision. + with patch.object(self.hs.get_clock(), "time_msec", return_value=1234567890000): + room_id1, _, _ = self.get_success( + handler.create_room(requester, {"room_version": "12"}, ratelimit=False) + ) + room_id2, _, _ = self.get_success( + handler.create_room(requester, {"room_version": "12"}, ratelimit=False) + ) + + self.assertNotEqual(room_id1, room_id2) + # v12 room IDs are content hashes with no domain component. + self.assertNotIn(":", room_id1) + self.assertNotIn(":", room_id2)