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

Attempting to call /messages on a room that doesn't exist results in a 500 #12678

Closed
bradtgmurray opened this issue May 9, 2022 · 3 comments · Fixed by #12683
Closed

Attempting to call /messages on a room that doesn't exist results in a 500 #12678

bradtgmurray opened this issue May 9, 2022 · 3 comments · Fixed by #12683
Assignees
Labels
A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release

Comments

@bradtgmurray
Copy link
Contributor

bradtgmurray commented May 9, 2022

Description

Requesting messages for a nonsense room id should probably return a 404 or a 403, but instead it just blows up in Sentry with a 500 here: https://github.com/matrix-org/synapse/blob/develop/synapse/handlers/pagination.py#L451

Interestingly the spec only says 200 or 403 (You're not a member of this room) are valid responses.
https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3roomsroomidmessages

Steps to reproduce

Throw this in your javascript console for your element web login

await fetch("https://matrix-client.matrix.org/_matrix/client/r0/rooms/!doesnotexist%3Amatrix.org/messages?limit=20&dir=b", {
  "headers": {
    "authorization": "Bearer <your access token here>",
  },
  "method": "GET",
}).then(r => r.json());

Version information

Beeper's fork of 1.57.1 (haven't gotten to 1.58 yet) but reproduced as well on matrix.org (above)

@bradtgmurray
Copy link
Contributor Author

bradtgmurray commented May 9, 2022

Ah, new in 1.57 I think

793d03e (from #12370)

@MadLittleMods

@clokep
Copy link
Contributor

clokep commented May 9, 2022

@clokep clokep added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release labels May 9, 2022
@MadLittleMods
Copy link
Contributor

Here is the traceback:

2022-05-09 15:31:37,607 - synapse.http.server - 120 - ERROR - GET-95853 - Failed handle request via 'RoomMessageListRestServlet': <XForwardedForRequest at 0x112643fd0 method='GET' uri='/_matrix/client/v3/rooms/!dne:my.synapse.server/messages?dir=b' clientproto='HTTP/1.1' site='8008'>
Traceback (most recent call last):
  File "/Users/eric/Documents/github/element/synapse/env/lib/python3.9/site-packages/twisted/internet/defer.py", line 1661, in _inlineCallbacks
    result = current_context.run(gen.send, result)
StopIteration

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/eric/Documents/github/element/synapse/synapse/http/server.py", line 298, in _async_render_wrapper
    callback_return = await self._async_render(request)
  File "/Users/eric/Documents/github/element/synapse/synapse/http/server.py", line 500, in _async_render
    callback_return = await raw_callback_return
  File "/Users/eric/Documents/github/element/synapse/synapse/rest/client/room.py", line 585, in on_GET
    msgs = await self.pagination_handler.get_messages(
  File "/Users/eric/Documents/github/element/synapse/synapse/handlers/pagination.py", line 451, in get_messages
    assert from_token.room_key.topological
AssertionError

Before #12370, /messages would return a 403 forbidden for a non-existent room:

{
    "errcode": "M_FORBIDDEN",
    "error": "User @test:my.synapse.server not in room !dne:my.synapse.server, and room previews are disabled"
}

@MadLittleMods MadLittleMods self-assigned this May 9, 2022
MadLittleMods added a commit that referenced this issue May 10, 2022
Fix #12678

Before: 500 internal server error

After: When calling `/messages` against a non-existent room_id it should throw a 403 forbidden,
see https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3roomsroomidmessages
```json
{
    "errcode": "M_FORBIDDEN",
    "error": "User @test:my.synapse.server not in room !dne:my.synapse.server, and room previews are disabled"
}
```
MadLittleMods added a commit to matrix-org/complement that referenced this issue May 10, 2022
MadLittleMods added a commit that referenced this issue May 11, 2022
…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"
}
```
kegsay added a commit to matrix-org/complement that referenced this issue May 11, 2022
…t `room_id`'s (#369)

* Add test to make sure /messages behaves as expected for non-existent room_ids

Tests for Synapse regression fix: matrix-org/synapse#12683
Issue: matrix-org/synapse#12678

* Explain why 403

* Remove dendrite skip

Co-authored-by: Kegan Dougal <kegan@matrix.org>
SpiritCroc pushed a commit to SpiritCroc/synapse-old that referenced this issue May 12, 2022
Fix matrix-org#12678

Before: 500 internal server error

After: When calling `/messages` against a non-existent room_id it should throw a 403 forbidden,
see https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3roomsroomidmessages
```json
{
    "errcode": "M_FORBIDDEN",
    "error": "User @test:my.synapse.server not in room !dne:my.synapse.server, and room previews are disabled"
}
```
@MadLittleMods MadLittleMods added the A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) label Jul 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Messages-Endpoint /messages client API endpoint (`RoomMessageListRestServlet`) (which also triggers /backfill) S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. X-Regression Something broke which worked on a previous release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants