Fix `/messages` throwing a 500 when querying for non-existent room (#12683)
Fix https://github.com/matrix-org/synapse/issues/12678
Complement test added: https://github.com/matrix-org/complement/pull/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 https://github.com/matrix-org/synapse/pull/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"
}
```
2 files changed, 12 insertions, 16 deletions
diff --git a/synapse/handlers/pagination.py b/synapse/handlers/pagination.py
index 7ee3340373..2e30180094 100644
--- a/synapse/handlers/pagination.py
+++ b/synapse/handlers/pagination.py
@@ -448,7 +448,7 @@ class PaginationHandler:
)
# 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
diff --git a/synapse/storage/databases/main/stream.py b/synapse/storage/databases/main/stream.py
index 793e906630..4e1d9647b7 100644
--- a/synapse/storage/databases/main/stream.py
+++ b/synapse/storage/databases/main/stream.py
@@ -785,22 +785,14 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
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,
@@ -870,7 +862,11 @@ class StreamWorkerStore(EventsWorkerStore, SQLBaseStore):
)
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(
|