From ddbcd859aa6f046c7fa54344007f0437d71cb6ca Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 4 Aug 2025 11:08:02 -0500 Subject: [PATCH] Improve order of validation and ratelimiting in room creation (#18723) Spawning from looking at this stuff while reviewing https://github.com/element-hq/synapse/pull/18721 --- changelog.d/18723.misc | 1 + synapse/handlers/room.py | 63 ++++++++++++++++++++-------------------- 2 files changed, 33 insertions(+), 31 deletions(-) create mode 100644 changelog.d/18723.misc diff --git a/changelog.d/18723.misc b/changelog.d/18723.misc new file mode 100644 index 000000000..90d9ad071 --- /dev/null +++ b/changelog.d/18723.misc @@ -0,0 +1 @@ +Improve order of validation and ratelimiting in room creation. diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index a1731752c..50ef25b09 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -779,6 +779,25 @@ class RoomCreationHandler: await self.auth_blocking.check_auth_blocking(requester=requester) + if ratelimit: + # Limit the rate of room creations, + # using both the limiter specific to room creations as well + # as the general request ratelimiter. + # + # Note that we don't rate limit the individual + # events in the room — room creation isn't atomic and + # historically it was very janky if half the events in the + # initial state don't make it because of rate limiting. + + # First check the room creation ratelimiter without updating it + # (this is so we don't consume a token if the other ratelimiter doesn't + # allow us to proceed) + await self.creation_ratelimiter.ratelimit(requester, update=False) + + # then apply the ratelimits + await self.common_request_ratelimiter.ratelimit(requester) + await self.creation_ratelimiter.ratelimit(requester) + if ( self._server_notices_mxid is not None and user_id == self._server_notices_mxid @@ -810,37 +829,6 @@ class RoomCreationHandler: Codes.MISSING_PARAM, ) - if not is_requester_admin: - spam_check = await self._spam_checker_module_callbacks.user_may_create_room( - user_id, config - ) - if spam_check != self._spam_checker_module_callbacks.NOT_SPAM: - raise SynapseError( - 403, - "You are not permitted to create rooms", - errcode=spam_check[0], - additional_fields=spam_check[1], - ) - - if ratelimit: - # Limit the rate of room creations, - # using both the limiter specific to room creations as well - # as the general request ratelimiter. - # - # Note that we don't rate limit the individual - # events in the room — room creation isn't atomic and - # historically it was very janky if half the events in the - # initial state don't make it because of rate limiting. - - # First check the room creation ratelimiter without updating it - # (this is so we don't consume a token if the other ratelimiter doesn't - # allow us to proceed) - await self.creation_ratelimiter.ratelimit(requester, update=False) - - # then apply the ratelimits - await self.common_request_ratelimiter.ratelimit(requester) - await self.creation_ratelimiter.ratelimit(requester) - room_version_id = config.get( "room_version", self.config.server.default_room_version.identifier ) @@ -932,6 +920,19 @@ class RoomCreationHandler: self._validate_room_config(config, visibility) + # Run the spam checker after other validation + if not is_requester_admin: + spam_check = await self._spam_checker_module_callbacks.user_may_create_room( + user_id, config + ) + if spam_check != self._spam_checker_module_callbacks.NOT_SPAM: + raise SynapseError( + 403, + "You are not permitted to create rooms", + errcode=spam_check[0], + additional_fields=spam_check[1], + ) + room_id = await self._generate_and_create_room_id( creator_id=user_id, is_public=is_public,