Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix /messages throwing a 500 when querying for non-existent room (#…
Browse files Browse the repository at this point in the history
…12683)

Fix #12678

Complement test added:  matrix-org/complement#369

**Before:** 500 internal server error

**After:** According to the [spec](https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3roomsroomidmessages), calling `/messages` against a non-existent `room_id` should throw a 403 forbidden (since you're not part of the room). This also matches the behavior before #12370 which regressed Synapse to the 500 behavior.
```json
{
    "errcode": "M_FORBIDDEN",
    "error": "User @test:my.synapse.server not in room !dne:my.synapse.server, and room previews are disabled"
}
```
  • Loading branch information
MadLittleMods authored May 11, 2022
1 parent c72d26c commit 84facf7
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 16 deletions.
1 change: 1 addition & 0 deletions changelog.d/12683.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug introduced in Synapse 1.57.0 where `/messages` would throw a 500 error when querying for a non-existent room.
2 changes: 1 addition & 1 deletion synapse/handlers/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,7 @@ async def get_messages(
)
# We expect `/messages` to use historic pagination tokens by default but
# `/messages` should still works with live tokens when manually provided.
assert from_token.room_key.topological
assert from_token.room_key.topological is not None

if pagin_config.limit is None:
# This shouldn't happen as we've set a default limit before this
Expand Down
26 changes: 11 additions & 15 deletions synapse/storage/databases/main/stream.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,22 +785,14 @@ async def get_last_event_in_room_before_stream_ordering(
return None

async def get_current_room_stream_token_for_room_id(
self, room_id: Optional[str] = None
self, room_id: str
) -> RoomStreamToken:
"""Returns the current position of the rooms stream.
By default, it returns a live token with the current global stream
token. Specifying a `room_id` causes it to return a historic token with
the room specific topological token.
"""
"""Returns the current position of the rooms stream (historic token)."""
stream_ordering = self.get_room_max_stream_ordering()
if room_id is None:
return RoomStreamToken(None, stream_ordering)
else:
topo = await self.db_pool.runInteraction(
"_get_max_topological_txn", self._get_max_topological_txn, room_id
)
return RoomStreamToken(topo, stream_ordering)
topo = await self.db_pool.runInteraction(
"_get_max_topological_txn", self._get_max_topological_txn, room_id
)
return RoomStreamToken(topo, stream_ordering)

def get_stream_id_for_event_txn(
self,
Expand Down Expand Up @@ -870,7 +862,11 @@ def _get_max_topological_txn(self, txn: LoggingTransaction, room_id: str) -> int
)

rows = txn.fetchall()
return rows[0][0] if rows else 0
# An aggregate function like MAX() will always return one row per group
# so we can safely rely on the lookup here. For example, when a we
# lookup a `room_id` which does not exist, `rows` will look like
# `[(None,)]`
return rows[0][0] if rows[0][0] is not None else 0

@staticmethod
def _set_before_and_after(
Expand Down

0 comments on commit 84facf7

Please sign in to comment.