Skip to content

Conversation

MadLittleMods
Copy link
Collaborator

Add test to make sure /messages behaves as expected for non-existent room_id's

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

@@ -59,6 +59,25 @@ func TestSendAndFetchMessage(t *testing.T) {
})
}

// With a non-existent room_id, GET /rooms/:room_id/messages returns 403
// forbidden ("You aren't a member of the room").
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As described in the spec,

403 | You aren’t a member of the room.

-- https://spec.matrix.org/latest/client-server-api/#get_matrixclientv3roomsroomidmessages

MadLittleMods added a commit to matrix-org/synapse that referenced this pull request 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/dendrite that referenced this pull request May 11, 2022
kegsay added a commit to matrix-org/dendrite that referenced this pull request May 11, 2022
@kegsay kegsay reopened this May 11, 2022
@kegsay
Copy link
Member

kegsay commented May 11, 2022

Silly github auto-closed the PR when I merged the Dendrite fix :S

@kegsay kegsay merged commit 15deea3 into main May 11, 2022
@kegsay kegsay deleted the madlittlemods/12678-403-when-/messages-with-does-not-exist-room-id branch May 11, 2022 11:17
@MadLittleMods
Copy link
Collaborator Author

Thanks for the review and merge @kegsay! Cool to see the Dendrite fix for this to all align as well! 🦏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
synapse-regression Issue or PR tests a failure in Synapse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants