Skip to content

Commit ddbcd85

Browse files
Improve order of validation and ratelimiting in room creation (#18723)
Spawning from looking at this stuff while reviewing #18721
1 parent 7ed5566 commit ddbcd85

2 files changed

Lines changed: 33 additions & 31 deletions

File tree

changelog.d/18723.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Improve order of validation and ratelimiting in room creation.

synapse/handlers/room.py

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -779,6 +779,25 @@ async def create_room(
779779

780780
await self.auth_blocking.check_auth_blocking(requester=requester)
781781

782+
if ratelimit:
783+
# Limit the rate of room creations,
784+
# using both the limiter specific to room creations as well
785+
# as the general request ratelimiter.
786+
#
787+
# Note that we don't rate limit the individual
788+
# events in the room — room creation isn't atomic and
789+
# historically it was very janky if half the events in the
790+
# initial state don't make it because of rate limiting.
791+
792+
# First check the room creation ratelimiter without updating it
793+
# (this is so we don't consume a token if the other ratelimiter doesn't
794+
# allow us to proceed)
795+
await self.creation_ratelimiter.ratelimit(requester, update=False)
796+
797+
# then apply the ratelimits
798+
await self.common_request_ratelimiter.ratelimit(requester)
799+
await self.creation_ratelimiter.ratelimit(requester)
800+
782801
if (
783802
self._server_notices_mxid is not None
784803
and user_id == self._server_notices_mxid
@@ -810,37 +829,6 @@ async def create_room(
810829
Codes.MISSING_PARAM,
811830
)
812831

813-
if not is_requester_admin:
814-
spam_check = await self._spam_checker_module_callbacks.user_may_create_room(
815-
user_id, config
816-
)
817-
if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
818-
raise SynapseError(
819-
403,
820-
"You are not permitted to create rooms",
821-
errcode=spam_check[0],
822-
additional_fields=spam_check[1],
823-
)
824-
825-
if ratelimit:
826-
# Limit the rate of room creations,
827-
# using both the limiter specific to room creations as well
828-
# as the general request ratelimiter.
829-
#
830-
# Note that we don't rate limit the individual
831-
# events in the room — room creation isn't atomic and
832-
# historically it was very janky if half the events in the
833-
# initial state don't make it because of rate limiting.
834-
835-
# First check the room creation ratelimiter without updating it
836-
# (this is so we don't consume a token if the other ratelimiter doesn't
837-
# allow us to proceed)
838-
await self.creation_ratelimiter.ratelimit(requester, update=False)
839-
840-
# then apply the ratelimits
841-
await self.common_request_ratelimiter.ratelimit(requester)
842-
await self.creation_ratelimiter.ratelimit(requester)
843-
844832
room_version_id = config.get(
845833
"room_version", self.config.server.default_room_version.identifier
846834
)
@@ -932,6 +920,19 @@ async def create_room(
932920

933921
self._validate_room_config(config, visibility)
934922

923+
# Run the spam checker after other validation
924+
if not is_requester_admin:
925+
spam_check = await self._spam_checker_module_callbacks.user_may_create_room(
926+
user_id, config
927+
)
928+
if spam_check != self._spam_checker_module_callbacks.NOT_SPAM:
929+
raise SynapseError(
930+
403,
931+
"You are not permitted to create rooms",
932+
errcode=spam_check[0],
933+
additional_fields=spam_check[1],
934+
)
935+
935936
room_id = await self._generate_and_create_room_id(
936937
creator_id=user_id,
937938
is_public=is_public,

0 commit comments

Comments
 (0)