From 093a6cb1116403f89fb5facb5b3fc45734dceb45 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Wed, 15 Jul 2026 15:28:15 -0500 Subject: [PATCH 1/3] Remove messy `WithWaitForLeave` test clean-up in `TestPartialStateJoin` (#894) Follow-up to https://github.com/matrix-org/complement/pull/880 which introduces a better way to avoid the test pollution and obsoletes the need for all of this cleanup logic. See the PR description there for a more complete context on why. `WithWaitForLeave` was originally introduced in this context: > Many of the faster joins test flakes are due to the homeserver under test failing to contact Complement homeservers after they have been torn down. When this happens, subsequent tests can fail if they use a Complement homeserver that happens to have the same `hostname:port` as one which the homeserver under test has previously marked as offline. > > *-- https://github.com/matrix-org/complement/pull/626* But thanks to https://github.com/matrix-org/complement/pull/880 we no longer use the same `hostname:port` for engineered homeservers anymore. And we can go even further with cleaning this up by removing the need for `Destroy(...)` altogether, see https://github.com/matrix-org/complement/pull/895 --- ...federation_room_join_partial_state_test.go | 119 ++---------------- 1 file changed, 9 insertions(+), 110 deletions(-) diff --git a/tests/msc3902/federation_room_join_partial_state_test.go b/tests/msc3902/federation_room_join_partial_state_test.go index 622ff01bc..3313aa4eb 100644 --- a/tests/msc3902/federation_room_join_partial_state_test.go +++ b/tests/msc3902/federation_room_join_partial_state_test.go @@ -123,74 +123,6 @@ func (s *server) AddEDUHandler(eduHandler func(gomatrixserverlib.EDU) bool) func } } -// WithWaitForLeave runs the given action and, when the resulting leave is -// expected to reach this server, waits for it. `leaveAction` is always run; the -// wait is skipped when `user` had already left the room (per their own -// homeserver, so the action produces no new leave) or when this server isn't in -// the room (so the leave won't be federated to us). -func (s *server) WithWaitForLeave( - t *testing.T, room *federation.ServerRoom, user *client.CSAPI, leaveAction func(), -) { - userID := user.UserID - leaveChannel := make(chan gomatrixserverlib.PDU, 10) - removePDUHandler := s.AddPDUHandler( - func(e gomatrixserverlib.PDU) bool { - if membership, _ := e.Membership(); e.Type() == "m.room.member" && - *e.StateKey() == userID && - membership == "leave" { - leaveChannel <- e - return true - } - return false - }, - ) - defer removePDUHandler() - - // We need to check if the user (on their homeserver) thinks they're in the - // room, before performing the `leaveAction` (to avoid races). - // - // If they are not in the room, then the `leaveAction` will not produce a - // new leave event and we should not wait for one. - // - // If they are in the room then the `leaveAction` will produce a new leave - // event. We then need to check if we expect this server receive the leave - // event by checking if this server is in the room. If they are, we wait, if - // not we can return immediately after the `leaveAction`. - userInRoom := userIsJoinedTo(t, user, room.RoomID) - - leaveAction() - - if !userInRoom { - // The user had already left, so the action produced no new leave and - // none is coming: don't wait. - t.Logf("%s is not joined to test room %s; not waiting for them to leave.", userID, room.RoomID) - return - } - - if !s.isInRoom(room) { - // The homeserver only federates the leave to servers that are in the - // room. If we aren't, no leave PDU is coming to us, so don't block until - // the timeout. - t.Logf("%s is not in test room %s; not waiting for %s to leave.", s.ServerName(), room.RoomID, userID) - return - } - - // Otherwise the action triggered the leave, which arrives as a PDU our - // handler matches. Wait on its channel rather than polling - // `room.CurrentState`: the room's current state is updated (by - // `room.AddEvent`) *before* the PDU callback runs, so returning on a - // `CurrentState` check could deregister our handler in the window before the - // callback fires, making the (expected) leave look unexpected to - // `HandleTransactionRequests`. This returns as soon as the leave arrives; the - // timeout is only a ceiling for declaring failure. - select { - case <-leaveChannel: - t.Logf("%s saw %s leave test room %s.", s.ServerName(), userID, room.RoomID) - case <-time.After(1 * time.Second): - t.Errorf("%s timed out waiting for %s to leave test room %s.", s.ServerName(), userID, room.RoomID) - } -} - // isInRoom reports whether this Complement server has a joined user in the room, // according to its own `ServerRoom` view. The server reliably tracks its own // users' membership (it created their join/leave events), so this answers "will @@ -2295,7 +2227,7 @@ func TestPartialStateJoin(t *testing.T) { // The room starts with @charlie:server1 and @derek:server1 in it. // @elsie:server2 joins the room before @t23alice:hs1. - server2Room := server2.MustJoinRoom( + server2.MustJoinRoom( t, deployment, server1.ServerName(), @@ -2306,7 +2238,6 @@ func TestPartialStateJoin(t *testing.T) { // @t23alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server1, room, alice) - defer server2.WithWaitForLeave(t, server2Room, alice, func() { psjResult.Destroy(t) }) // Both homeservers should receive device list updates. renameDevice(t, alice, "A new device name 1") @@ -2343,7 +2274,7 @@ func TestPartialStateJoin(t *testing.T) { t.Log("@charlie and @derek received device list update.") // @elsie:server2 joins the room. - server2Room := server2.MustJoinRoom( + server2.MustJoinRoom( t, deployment, server1.ServerName(), @@ -2351,9 +2282,6 @@ func TestPartialStateJoin(t *testing.T) { server2.UserID("elsie"), federation.WithPartialState(), ) - // NB: We register the `psjResult.Destroy()` cleanup twice. This is alright because it - // is idempotent. Here we wait for server 2 to observe the leave too. - defer server2.WithWaitForLeave(t, server2Room, alice, func() { psjResult.Destroy(t) }) joinEvent := room.CurrentState("m.room.member", server2.UserID("elsie")) server1.MustSendTransaction(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), []json.RawMessage{joinEvent.JSON()}, nil) awaitEventViaSync(t, alice, room.RoomID, joinEvent.EventID(), "") @@ -2505,14 +2433,14 @@ func TestPartialStateJoin(t *testing.T) { t *testing.T, deployment complement.Deployment, alice *client.CSAPI, server1 *server, server2 *server, partialStateRoom *federation.ServerRoom, syncToken string, - ) (nextSyncToken string, leaveSharedRoom func()) { + ) (nextSyncToken string) { elsie := server2.UserID("elsie") // @alice:hs1 creates a public room. roomID := alice.MustCreateRoom(t, map[string]interface{}{"preset": "public_chat"}) // @elsie:server2 joins the room. - server2Room := server2.MustJoinRoom(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), roomID, elsie) + server2.MustJoinRoom(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), roomID, elsie) alice.MustSyncUntil(t, client.SyncReq{ Since: syncToken, @@ -2529,13 +2457,7 @@ func TestPartialStateJoin(t *testing.T) { server1.MustSendTransaction(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), []json.RawMessage{leaveEvent.JSON()}, nil) syncToken = awaitEventViaSync(t, alice, partialStateRoom.RoomID, leaveEvent.EventID(), syncToken) - leaveSharedRoom = func() { - server2.WithWaitForLeave(t, server2Room, alice, func() { - alice.MustLeaveRoom(t, roomID) - }) - } - - return syncToken, leaveSharedRoom + return syncToken } // testMissedDeviceListUpdateSentOncePartialJoinCompletes takes a room where hs1 incorrectly @@ -2561,8 +2483,7 @@ func TestPartialStateJoin(t *testing.T) { // The homeserver under test cannot simply use the current state of the room to // determine which device list updates it must send out once the partial state join // completes. - _, leaveSharedRoom := setupAnotherSharedRoomThenLeave(t, deployment, alice, server1, server2, room, syncToken) - defer leaveSharedRoom() + setupAnotherSharedRoomThenLeave(t, deployment, alice, server1, server2, room, syncToken) } // Finish the partial state join. @@ -2589,8 +2510,7 @@ func TestPartialStateJoin(t *testing.T) { // The room starts with @charlie:server1 and @derek:server1 in it. // @t26alice:hs1 joins the room, followed by @elsie:server2. // @elsie:server2 is kicked with an invalid event. - syncToken, server2Room, psjResult := setupIncorrectlyAcceptedKick(t, deployment, alice, server1, server2, deviceListUpdateChannel1, deviceListUpdateChannel2, room) - defer server2.WithWaitForLeave(t, server2Room, alice, func() { psjResult.Destroy(t) }) + syncToken, _, psjResult := setupIncorrectlyAcceptedKick(t, deployment, alice, server1, server2, deviceListUpdateChannel1, deviceListUpdateChannel2, room) // @t26alice:hs1 sends out a device list update which is missed by @elsie:server2. // @elsie:server2 must receive missed device list updates once the partial state join finishes. @@ -2641,7 +2561,7 @@ func TestPartialStateJoin(t *testing.T) { // The room starts with @charlie:server1 and @derek:server1 in it. // @elsie:server2 joins the room, followed by @t28alice:hs1. // server1 does not tell hs1 that server2 is in the room. - server2Room := server2.MustJoinRoom( + server2.MustJoinRoom( t, deployment, server1.ServerName(), @@ -2650,7 +2570,6 @@ func TestPartialStateJoin(t *testing.T) { federation.WithPartialState(), ) psjResult := beginPartialStateJoin(t, server1, room, alice) - defer server2.WithWaitForLeave(t, server2Room, alice, func() { psjResult.Destroy(t) }) // @t28alice:hs1 sends out a device list update which is missed by @elsie:server2. // @elsie:server2 must receive missed device list updates once the partial state join finishes. @@ -3045,7 +2964,7 @@ func TestPartialStateJoin(t *testing.T) { // @charlie joins the room. // Now @charlie's device list is definitely being tracked. - otherRoom := server.MustJoinRoom(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), otherRoomID, server.UserID("charlie")) + server.MustJoinRoom(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), otherRoomID, server.UserID("charlie")) alice.MustSyncUntil(t, client.SyncReq{ Since: syncToken, @@ -3053,7 +2972,6 @@ func TestPartialStateJoin(t *testing.T) { }, client.SyncJoinedTo(server.UserID("charlie"), otherRoomID), ) - defer server.WithWaitForLeave(t, otherRoom, alice, func() { alice.MustLeaveRoom(t, otherRoomID) }) // Depending on the homeserver implementation, @t31alice:hs1 must have been told that either: // * charlie updated their device list, or @@ -4455,8 +4373,6 @@ func beginPartialStateJoin(t *testing.T, server *server, serverRoom *federation. // Destroy cleans up the resources associated with the join attempt. // It is idempotent and must be called once the test is finished. -// Specifically, it ensures that the partial state join completes and makes the joining user leave -// the room. func (psj *partialStateJoinResult) Destroy(t *testing.T) { if psj.fedStateIdsSendResponseWaiter != nil { psj.fedStateIdsSendResponseWaiter.Finish() @@ -4465,23 +4381,6 @@ func (psj *partialStateJoinResult) Destroy(t *testing.T) { if psj.fedStateIdsRequestReceivedWaiter != nil { psj.fedStateIdsRequestReceivedWaiter.Finish() } - - // Since the same deployment is being used across multiple tests, ensure that it - // has finished all federation activity before tearing down the Complement server. - // Otherwise the homeserver at the Complement's hostname:port combination may be - // considered offline and interfere with subsequent tests. - t.Log("Cleaning up after test...") - - awaitPartialStateJoinCompletion(t, psj.ServerRoom, psj.User) - - // The caller is about to tear down the Complement homeserver. Leave the room, so - // that the homeserver under test stops sending it presence updates. - psj.Server.WithWaitForLeave( - t, - psj.ServerRoom, - psj.User, - func() { psj.User.MustLeaveRoom(t, psj.ServerRoom.RoomID) }, - ) } // send a message into the room without letting the homeserver under test know about it. From e7e93b4cbdea876d30d8624f67915db95fdf46d4 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 21 Jul 2026 09:43:27 -0500 Subject: [PATCH 2/3] Remove messy `Destroy` test clean-up in `TestPartialStateJoin` (#895) Follow-up to https://github.com/matrix-org/complement/pull/894 which removes another part of the clean-up code in this file and https://github.com/matrix-org/complement/pull/880 which introduces a better way to avoid the test pollution and obsoletes the need for all of this cleanup logic. See the PR description there for a more complete context on why. `Destroy(...)` was originally introduced in this context: > Since the partial state join tests reuse the same homeserver deployment, it is important that each test leaves the homeserver in an acceptable state for subsequent tests. In particular, we must allow in-progress partial-state joins to complete before tearing down Complement servers, otherwise the homeserver may mark Complement `hostname:port` combinations as offline and refuse to contact them in subsequent tests. > > *-- https://github.com/matrix-org/complement/pull/570* But thanks to https://github.com/matrix-org/complement/pull/880 we no longer use the same `hostname:port` for engineered homeservers anymore. --- Given the nature of these changes, this also fixes the goroutine panic that can happen when the waiter finishes after the test and `t` is used after the test finishes, see https://github.com/matrix-org/complement/pull/894#discussion_r3574583828 --- helpers/waiter.go | 7 + ...federation_room_join_partial_state_test.go | 189 ++++++++---------- 2 files changed, 88 insertions(+), 108 deletions(-) diff --git a/helpers/waiter.go b/helpers/waiter.go index 8ee6fa732..59d066f4f 100644 --- a/helpers/waiter.go +++ b/helpers/waiter.go @@ -47,6 +47,13 @@ func (w *Waiter) Waitf(t ct.TestLike, timeout time.Duration, errFormat string, a } } +// Done returns a channel which is closed when Finish is called. It can be used +// in `select` statements where failing the test on a timeout (as Wait does) is +// not appropriate. Prefer `Wait`/`Waitf` otherwise. +func (w *Waiter) Done() <-chan bool { + return w.ch +} + // Finish will cause all goroutines waiting via Wait to stop waiting and return. // Once this function has been called, subsequent calls to Wait will return immediately. // To begin waiting again, make a new Waiter. diff --git a/tests/msc3902/federation_room_join_partial_state_test.go b/tests/msc3902/federation_room_join_partial_state_test.go index 3313aa4eb..6cd7e6fd2 100644 --- a/tests/msc3902/federation_room_join_partial_state_test.go +++ b/tests/msc3902/federation_room_join_partial_state_test.go @@ -302,7 +302,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) t.Log("2. Have Alice lazy-sync until she sees (1).") lazySyncToken = alice.MustSyncUntil( @@ -407,7 +406,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) t.Log("Alice eager-syncs. The response should not contain the remote room.") response, nextBatch := alice.MustSync(t, client.SyncReq{}) @@ -470,8 +468,7 @@ func TestPartialStateJoin(t *testing.T) { cancel := server.Listen() defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) alice.MustSyncUntil(t, client.SyncReq{ @@ -492,8 +489,7 @@ func TestPartialStateJoin(t *testing.T) { cancel := server.Listen() defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) pdusChannel := make(chan gomatrixserverlib.PDU) removePDUHandler := server.AddPDUHandler( @@ -536,7 +532,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // Derek starts typing in the room. derekUserId := psjResult.Server.UserID("derek") @@ -606,7 +601,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) derekUserId := psjResult.Server.UserID("derek") @@ -653,7 +647,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // Send a to-device message from Derek to Alice. derekUserId := psjResult.Server.UserID("derek") @@ -703,7 +696,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) derekUserId := psjResult.Server.UserID("derek") @@ -756,7 +748,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) derekUserId := psjResult.Server.UserID("derek") @@ -806,7 +797,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) derekUserId := psjResult.Server.UserID("derek") @@ -835,7 +825,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // the HS will make an /event_auth request for the event federation.HandleEventAuthRequests()(server.Server) @@ -859,7 +848,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // we construct the following event graph: // ... <-- M <-- A <-- B @@ -899,7 +887,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // we construct the following event graph: // +---------+ @@ -942,7 +929,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // we construct the following event graph: // +---------+ @@ -988,7 +974,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // the HS will make an /event_auth request for the event federation.HandleEventAuthRequests()(server.Server) @@ -1030,7 +1015,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) syncToken = alice.MustSyncUntil(t, client.SyncReq{ @@ -1102,7 +1086,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) syncToken = alice.MustSyncUntil(t, client.SyncReq{ @@ -1151,7 +1134,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // we need a sync token to pass to the `at` param. syncToken := alice.MustSyncUntil(t, @@ -1213,7 +1195,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // Alice has now joined the room, and the server is syncing the state in the background. @@ -1284,7 +1265,29 @@ func TestPartialStateJoin(t *testing.T) { queryParams := req.URL.Query() t.Logf("Incoming state_ids request for event %s in room %s", queryParams["event_id"], roomID) fedStateIdsRequestReceivedWaiter.Finish() - fedStateIdsSendResponseWaiter.Wait(t, 60*time.Second) + + // Wait for `fedStateIdsSendResponseWaiter` + select { + case <-fedStateIdsSendResponseWaiter.Done(): + // Happy-path now that we're done waiting, continue serving the request now + case <-req.Context().Done(): + // The request was cancelled (the Complement server is probably shutting down) + // which means nobody wants this response any more (just bail out without + // doing any more work). + // + // Also as a note: although the cancellation itself happens while the test is + // still running, `srv.Close()` cancels any if-flight requests but does not + // wait for this goroutine, so by the time we wake up here the test may have + // already completed and touching `t` after that panics. + return + case <-time.After(60 * time.Second): + // Sanity check so a wedged test fails loudly instead of blocking forever. + t.Fatalf( + "Timed out waiting for the test to finish the `sendResponseWaiter` while trying"+ + "to serve /state_ids response for event %s", queryParams["event_id"], + ) + } + t.Logf("Replying to /state_ids request with invalid response") w.WriteHeader(200) @@ -1340,7 +1343,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // get a sync token before state syncing finishes. syncToken := alice.MustSyncUntil(t, @@ -1437,7 +1439,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // Alice has now joined the room, and the server is syncing the state in the background. @@ -1551,7 +1552,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // the HS will make an /event_auth request for the event federation.HandleEventAuthRequests()(server.Server) @@ -1646,7 +1646,6 @@ func TestPartialStateJoin(t *testing.T) { serverRoom.AddEvent(derekLeaveEvent) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // derek now sends a state event with auth_events that say he was in the room. It will be // accepted during the faster join, but should then ultimately be rejected. @@ -1731,7 +1730,6 @@ func TestPartialStateJoin(t *testing.T) { serverRoom.AddEvent(elsieJoinEvent) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // Derek now kicks Elsie, with auth_events that say he was in the room. It will be // accepted during the faster join, but should then ultimately be rejected. @@ -1828,8 +1826,7 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, testServer1, alice.GetDefaultRoomVersion(t)) roomID := serverRoom.RoomID - psjResult := beginPartialStateJoin(t, testServer1, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, testServer1, serverRoom, alice) // The partial join is now in progress. // Let's have a new test server rock up and ask to join the room by making a @@ -1878,8 +1875,7 @@ func TestPartialStateJoin(t *testing.T) { cancel := testServer1.Listen() defer cancel() serverRoom := createTestRoom(t, testServer1, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, testServer1, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, testServer1, serverRoom, alice) // hs1's partial join is now in progress. // Let's have a test server rock up and ask to /send_join in the room via hs1. @@ -1939,7 +1935,6 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) // Alice has now joined the room, and the server is syncing the state in the background. @@ -2006,8 +2001,7 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, testServer1, alice.GetDefaultRoomVersion(t)) roomID := serverRoom.RoomID - psjResult := beginPartialStateJoin(t, testServer1, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, testServer1, serverRoom, alice) // The partial join is now in progress. // Let's have a new test server rock up and ask to join the room by making a @@ -2056,8 +2050,7 @@ func TestPartialStateJoin(t *testing.T) { cancel := testServer1.Listen() defer cancel() serverRoom := createTestRoom(t, testServer1, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, testServer1, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, testServer1, serverRoom, alice) // hs1's partial join is now in progress. // Let's have a test server rock up and ask to /send_knock in the room via hs1. @@ -2265,7 +2258,6 @@ func TestPartialStateJoin(t *testing.T) { // The room starts with @charlie:server1 and @derek:server1 in it. // @t24alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server1, room, alice) - defer psjResult.Destroy(t) // Only server1 should receive device list updates. renameDevice(t, alice, "A new device name 1") @@ -2324,7 +2316,6 @@ func TestPartialStateJoin(t *testing.T) { // @t25alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server1, room, alice) - defer psjResult.Destroy(t) // @elsie:server2 leaves the room. // Create and send the event to the homeserver using server1, since the test setup did @@ -2531,7 +2522,6 @@ func TestPartialStateJoin(t *testing.T) { // @t27alice:hs1 joins the room, followed by @elsie:server2. // @elsie:server2 is kicked with an invalid event. syncToken, _, psjResult := setupIncorrectlyAcceptedKick(t, deployment, alice, server1, server2, deviceListUpdateChannel1, deviceListUpdateChannel2, room) - defer psjResult.Destroy(t) // @t27alice:hs1 sends out a device list update which is missed by @elsie:server2. // @elsie:server2 joins another room shared with @t27alice:hs1 and leaves the partial state room. @@ -2600,7 +2590,6 @@ func TestPartialStateJoin(t *testing.T) { federation.WithPartialState(), ) psjResult := beginPartialStateJoin(t, server1, room, alice) - defer psjResult.Destroy(t) // @t29alice:hs1 sends out a device list update which is missed by @elsie:server2. // @elsie:server2 joins another room shared with @t29alice:hs1 and leaves the partial state room. @@ -2896,7 +2885,6 @@ func TestPartialStateJoin(t *testing.T) { // @t30alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server, room, alice) - defer psjResult.Destroy(t) // @charlie and @derek's device list ought to not be cached. mustQueryKeysWithFederationRequest(t, alice, userDevicesChannel, server.UserID("charlie")) @@ -2946,7 +2934,6 @@ func TestPartialStateJoin(t *testing.T) { // @t31alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server, room, alice) - defer psjResult.Destroy(t) // @charlie sends a message. // Depending on the homeserver implementation, @t31alice:hs1 may be told that @charlie's devices are being tracked. @@ -2989,7 +2976,6 @@ func TestPartialStateJoin(t *testing.T) { // @t32alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server, room, alice) - defer psjResult.Destroy(t) syncToken := getSyncToken(t, alice) @@ -3037,8 +3023,7 @@ func TestPartialStateJoin(t *testing.T) { // The room starts with @charlie and @derek in it. // @t33alice:hs1 joins the room. - psjResult := beginPartialStateJoin(t, server, room, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, room, alice) syncToken := getSyncToken(t, alice) @@ -3082,8 +3067,7 @@ func TestPartialStateJoin(t *testing.T) { // The room starts with @charlie and @derek in it. // @t34alice:hs1 joins the room. - psjResult := beginPartialStateJoin(t, server, room, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, room, alice) syncToken := getSyncToken(t, alice) @@ -3125,7 +3109,6 @@ func TestPartialStateJoin(t *testing.T) { // @t35alice:hs1 joins the room. psjResult := beginPartialStateJoin(t, server, room, alice) - defer psjResult.Destroy(t) syncToken := getSyncToken(t, alice) @@ -3263,7 +3246,6 @@ func TestPartialStateJoin(t *testing.T) { // @charlie "kicks" @derek, which the homeserver under test incorrectly accepts. // @derek kicks @elsie, which the homeserver under test incorrectly rejects. _, psjResult := setupUserIncorrectlyInRoom(t, deployment, alice, server, room) - defer psjResult.Destroy(t) // @elsie is now incorrectly believed to be in the room. // The homeserver under test incorrectly thinks it is subscribed to @elsie's device list updates. @@ -3297,7 +3279,6 @@ func TestPartialStateJoin(t *testing.T) { // @charlie "kicks" @derek, which the homeserver under test incorrectly accepts. // @derek kicks @elsie, which the homeserver under test incorrectly rejects. syncToken, psjResult := setupUserIncorrectlyInRoom(t, deployment, alice, server, room) - defer psjResult.Destroy(t) // @elsie is now incorrectly believed to be in the room. // The homeserver under test incorrectly thinks it is subscribed to @elsie's device list updates. @@ -3339,7 +3320,6 @@ func TestPartialStateJoin(t *testing.T) { // @charlie "kicks" @derek, which the homeserver under test incorrectly accepts. // @derek kicks @elsie, which the homeserver under test incorrectly rejects. syncToken, psjResult := setupUserIncorrectlyInRoom(t, deployment, alice, server, room) - defer psjResult.Destroy(t) // @elsie is now incorrectly believed to be in the room. // The homeserver under test incorrectly thinks it is subscribed to @elsie's device list updates. @@ -3379,7 +3359,6 @@ func TestPartialStateJoin(t *testing.T) { // @charlie "kicks" @derek, which the homeserver under test incorrectly accepts. // @derek kicks @elsie, which the homeserver under test incorrectly rejects. syncToken, psjResult := setupUserIncorrectlyInRoom(t, deployment, alice, server, room) - defer psjResult.Destroy(t) // @elsie is now incorrectly believed to be in the room. // The homeserver under test incorrectly thinks it is subscribed to @elsie's device list updates. @@ -3429,8 +3408,7 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) // Alice creates an alias for the room aliasName := "#t40alice-room:hs1" @@ -3482,8 +3460,7 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) // Alice creates an alias for the room aliasName := "#t41alice-room:hs1" @@ -3525,8 +3502,7 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) server.AddPDUHandler(func(e gomatrixserverlib.PDU) bool { return true }) bob.MustJoinRoom(t, serverRoom.RoomID, []spec.ServerName{server.ServerName()}) @@ -3559,8 +3535,7 @@ func TestPartialStateJoin(t *testing.T) { defer cancel() serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) pdusChannel := make(chan gomatrixserverlib.PDU) removePDUHandler := server.AddPDUHandler( @@ -3604,7 +3579,6 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice partial-joins her room") psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) t.Log("Alice waits to see her join") aliceNextBatch := alice.MustSyncUntil( @@ -3646,8 +3620,7 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice begins a partial join to a room") - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) t.Log("Alice waits to see her join") aliceNextBatch := alice.MustSyncUntil( @@ -3709,11 +3682,7 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice partial-joins her room") - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - // At the end of the test, keep Bob in the room. Have him make a /members - // call to ensure the resync has completed. - psjResult.User = bob - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) t.Log("Alice sees her join") aliceNextBatch := alice.MustSyncUntil( @@ -3761,8 +3730,7 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice partial-joins her room") - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) t.Log("Alice waits to see her join") aliceNextBatch := alice.MustSyncUntil( @@ -3806,11 +3774,7 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice partial-joins her room") - psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - // At the end of the test, keep Bob in the room. Have him make a /members - // call to ensure the resync has completed. - psjResult.User = bob - defer psjResult.Destroy(t) + beginPartialStateJoin(t, server, serverRoom, alice) t.Log("Alice waits to see her join") aliceNextBatch := alice.MustSyncUntil( @@ -3852,8 +3816,6 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice partial-joins her room") psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - // Alice is not joined to the room at the end of the test, so we do not - // `defer psjResult.Destroy(t)`. t.Log("Alice waits to see her join") aliceNextBatch := alice.MustSyncUntil( @@ -3906,8 +3868,6 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) t.Log("Alice partial-joins her room") psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - // Alice is not joined to the room at the end of the test, so we do not - // `defer psjResult.Destroy(t)`. t.Log("Alice waits to see her join") aliceNextBatch := alice.MustSyncUntil( @@ -3971,7 +3931,6 @@ func TestPartialStateJoin(t *testing.T) { // start a partial state join psjResult := beginPartialStateJoin(t, server, serverRoom, terry) - defer psjResult.Destroy(t) // make the remote room visible in the local room list reqBody := client.WithJSONBody(t, map[string]interface{}{ @@ -4024,7 +3983,6 @@ func TestPartialStateJoin(t *testing.T) { // start a partial state join psjResult := beginPartialStateJoin(t, server, serverRoom, rocky) - defer psjResult.Destroy(t) assertUserInDirectory := func(t *testing.T, localpart string, userID string) { reqBody := client.WithJSONBody(t, map[string]interface{}{ @@ -4087,9 +4045,6 @@ func TestPartialStateJoin(t *testing.T) { serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) psjResult := beginPartialStateJoin(t, server, serverRoom, alice) - // NB: because we do not end up joined to this room at the end of the test, - // we do not `defer psjResult.Destroy(t)` as usual; see the comments below - // about races. t.Log("Alice waits to see her join") alice.MustSyncUntil( @@ -4315,14 +4270,13 @@ func buildLazyLoadingSyncFilter(timelineOptions map[string]interface{}) string { type partialStateJoinResult struct { Server *server ServerRoom *federation.ServerRoom - User *client.CSAPI fedStateIdsRequestReceivedWaiter *helpers.Waiter fedStateIdsSendResponseWaiter *helpers.Waiter } // beginPartialStateJoin has a test user attempt to join the given room. // -// It returns a partialStateJoinResult, which must be Destroy'd on completion. +// It returns a partialStateJoinResult. // // When this method completes, the /join request will have completed, but the // state has not yet been re-synced. To allow the re-sync to proceed, call @@ -4332,14 +4286,7 @@ func beginPartialStateJoin(t *testing.T, server *server, serverRoom *federation. result := partialStateJoinResult{ Server: server, ServerRoom: serverRoom, - User: joiningUser, } - success := false - defer func() { - if !success { - result.Destroy(t) - } - }() // some things for orchestration result.fedStateIdsRequestReceivedWaiter = helpers.NewWaiter() @@ -4367,22 +4314,9 @@ func beginPartialStateJoin(t *testing.T, server *server, serverRoom *federation. joiningUser.MustJoinRoom(t, serverRoom.RoomID, []spec.ServerName{server.ServerName()}) t.Logf("/join request completed") - success = true return result } -// Destroy cleans up the resources associated with the join attempt. -// It is idempotent and must be called once the test is finished. -func (psj *partialStateJoinResult) Destroy(t *testing.T) { - if psj.fedStateIdsSendResponseWaiter != nil { - psj.fedStateIdsSendResponseWaiter.Finish() - } - - if psj.fedStateIdsRequestReceivedWaiter != nil { - psj.fedStateIdsRequestReceivedWaiter.Finish() - } -} - // send a message into the room without letting the homeserver under test know about it. func (psj *partialStateJoinResult) CreateMessageEvent(t *testing.T, senderLocalpart string, prevEventIDs []string) gomatrixserverlib.PDU { var prevEvents interface{} @@ -4436,7 +4370,26 @@ func handleStateIdsRequests( requestReceivedWaiter.Finish() } if sendResponseWaiter != nil { - sendResponseWaiter.Waitf(t, 60*time.Second, "Waiting for /state_ids request") + select { + case <-sendResponseWaiter.Done(): + // Happy-path now that we're done waiting, continue serving the request now + case <-req.Context().Done(): + // The request was cancelled (the Complement server is probably shutting down) + // which means nobody wants this response any more (just bail out without + // doing any more work). + // + // Also as a note: although the cancellation itself happens while the test is + // still running, `srv.Close()` cancels any if-flight requests but does not + // wait for this goroutine, so by the time we wake up here the test may have + // already completed and touching `t` after that panics. + return + case <-time.After(60 * time.Second): + // Sanity check so a wedged test fails loudly instead of blocking forever. + t.Fatalf( + "Timed out waiting for the test to finish the `sendResponseWaiter` while trying"+ + "to serve /state_ids response for event %s", queryParams["event_id"], + ) + } } t.Logf("Replying to /state_ids request for event %s", queryParams["event_id"]) @@ -4476,7 +4429,27 @@ func handleStateRequests( requestReceivedWaiter.Finish() } if sendResponseWaiter != nil { - sendResponseWaiter.Waitf(t, 60*time.Second, "Waiting for /state request") + + select { + case <-sendResponseWaiter.Done(): + // Happy-path now that we're done waiting, continue serving the request now + case <-req.Context().Done(): + // The request was cancelled (the Complement server is probably shutting down) + // which means nobody wants this response any more (just bail out without + // doing any more work). + // + // Also as a note: although the cancellation itself happens while the test is + // still running, `srv.Close()` cancels any if-flight requests but does not + // wait for this goroutine, so by the time we wake up here the test may have + // already completed and touching `t` after that panics. + return + case <-time.After(60 * time.Second): + // Sanity check so a wedged test fails loudly instead of blocking forever. + t.Fatalf( + "Timed out waiting for the test to finish the `sendResponseWaiter` while trying"+ + "to serve /state response for event %s", queryParams["event_id"], + ) + } } t.Logf("Replying to /state request for event %s", queryParams["event_id"]) From 893306c6565c6b854bb3e8ce0092b2a4bb2063e9 Mon Sep 17 00:00:00 2001 From: Jason Little Date: Thu, 30 Jul 2026 02:42:28 -0500 Subject: [PATCH 3/3] Support Room version 12 as default for a homeserver being tested (#885) * Update TestDemotingUsersViaUsersDefault regression test * Update TestPowerLevels tests * Update TestRoomDeleteAlias * Update TestRoomMembers * Update TestInboundFederationRejectsEventsWithRejectedAuthEvents * Update TestPartialStateJoin * Use existing RoomVersion object instead of making a new one * Borrow existing RoomVersion object from the ServerRoom * attempt to run match... function --- tests/csapi/apidoc_room_alias_test.go | 44 +++-- tests/csapi/apidoc_room_members_test.go | 22 ++- tests/csapi/power_levels_test.go | 164 +++++++++++++----- tests/federation_room_event_auth_test.go | 41 +++-- ...federation_room_join_partial_state_test.go | 139 ++++++++++----- 5 files changed, 289 insertions(+), 121 deletions(-) diff --git a/tests/csapi/apidoc_room_alias_test.go b/tests/csapi/apidoc_room_alias_test.go index 65cff3c9a..921c8c6ff 100644 --- a/tests/csapi/apidoc_room_alias_test.go +++ b/tests/csapi/apidoc_room_alias_test.go @@ -14,6 +14,7 @@ import ( "github.com/matrix-org/complement/match" "github.com/matrix-org/complement/must" "github.com/matrix-org/complement/should" + "github.com/matrix-org/gomatrixserverlib" ) func setRoomAliasResp(t *testing.T, c *client.CSAPI, roomID, roomAlias string) *http.Response { @@ -192,6 +193,8 @@ func TestRoomDeleteAlias(t *testing.T) { alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) bob := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + defaultRoomVersion := alice.GetDefaultRoomVersion(t) + t.Run("Parallel", func(t *testing.T) { // sytest: Alias creators can delete alias with no ops t.Run("Alias creators can delete alias with no ops", func(t *testing.T) { @@ -352,14 +355,21 @@ func TestRoomDeleteAlias(t *testing.T) { alice.SendEventSynced(t, roomID, b.Event{ Type: "m.room.power_levels", StateKey: b.Ptr(""), - Content: map[string]interface{}{ - "users": map[string]int64{ - alice.UserID: 100, - }, - "events": map[string]int64{ - "m.room.aliases": 50, - }, - }, + Content: func() map[string]interface{} { + content := map[string]interface{}{ + "events": map[string]int64{ + "m.room.aliases": 50, + }, + "users": map[string]int64{ + alice.UserID: 100, + }, + } + // Room v12+ prohibits the room creator in the `users` object + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(content["users"].(map[string]int64), alice.UserID) + } + return content + }(), }) res := setRoomAliasResp(t, bob, roomID, randomAlias) @@ -428,12 +438,18 @@ func TestRoomDeleteAlias(t *testing.T) { alice.SendEventSynced(t, roomID, b.Event{ Type: "m.room.power_levels", StateKey: b.Ptr(""), - Content: map[string]interface{}{ - "users": map[string]int64{ - alice.UserID: 100, - bob.UserID: 100, - }, - }, + Content: func() map[string]interface{} { + content := map[string]interface{}{ + "users": map[string]int64{ + alice.UserID: 100, + bob.UserID: 100, + }, + } + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(content["users"].(map[string]int64), alice.UserID) + } + return content + }(), }) res := setRoomAliasResp(t, alice, roomID, randomAlias) diff --git a/tests/csapi/apidoc_room_members_test.go b/tests/csapi/apidoc_room_members_test.go index adc2e0949..d4ea40ba4 100644 --- a/tests/csapi/apidoc_room_members_test.go +++ b/tests/csapi/apidoc_room_members_test.go @@ -11,6 +11,7 @@ import ( "github.com/matrix-org/complement/helpers" "github.com/matrix-org/complement/match" "github.com/matrix-org/complement/must" + "github.com/matrix-org/gomatrixserverlib" "github.com/matrix-org/gomatrixserverlib/spec" ) @@ -84,6 +85,7 @@ func TestRoomMembers(t *testing.T) { // sytest: Test that we can be reinvited to a room we created t.Run("Test that we can be reinvited to a room we created", func(t *testing.T) { t.Parallel() + defaultRoomVersion := alice.GetDefaultRoomVersion(t) roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "private_chat", }) @@ -101,13 +103,19 @@ func TestRoomMembers(t *testing.T) { alice.SendEventSynced(t, roomID, b.Event{ Type: "m.room.power_levels", StateKey: &stateKey, - Content: map[string]interface{}{ - "invite": 100, - "users": map[string]interface{}{ - alice.UserID: 100, - bob.UserID: 100, - }, - }, + Content: func() map[string]interface{} { + content := map[string]interface{}{ + "invite": 100, + "users": map[string]int64{ + bob.UserID: 100, + alice.UserID: 100, + }, + } + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(content["users"].(map[string]int64), alice.UserID) + } + return content + }(), }) alice.MustLeaveRoom(t, roomID) diff --git a/tests/csapi/power_levels_test.go b/tests/csapi/power_levels_test.go index 9ed2bd903..350837407 100644 --- a/tests/csapi/power_levels_test.go +++ b/tests/csapi/power_levels_test.go @@ -12,6 +12,8 @@ import ( "github.com/matrix-org/complement/helpers" "github.com/matrix-org/complement/match" "github.com/matrix-org/complement/must" + "github.com/matrix-org/gomatrixserverlib" + "github.com/matrix-org/gomatrixserverlib/spec" ) // This test ensures that an authorised (PL 100) user is able to modify the users_default value @@ -22,30 +24,53 @@ func TestDemotingUsersViaUsersDefault(t *testing.T) { defer deployment.Destroy(t) alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + bob := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + + defaultRoomVersion := alice.GetDefaultRoomVersion(t) roomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", - "power_level_content_override": map[string]interface{}{ - "users_default": 100, // the default is 0 - "users": map[string]interface{}{ - alice.UserID: 100, - }, - "events": map[string]int64{}, - "notifications": map[string]int64{}, - }, + "power_level_content_override": func() map[string]interface{} { + power_level_content := map[string]interface{}{ + "users_default": 100, // the default is 0 + "users": map[string]int64{ + alice.UserID: 100, + bob.UserID: 100, + }, + "events": map[string]int64{}, + "notifications": map[string]int64{}, + } + // Remove the room creator if this is a v12+ room + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(power_level_content["users"].(map[string]int64), alice.UserID) + } + return power_level_content + }(), + }) + + bob.MustJoinRoom(t, roomID, []spec.ServerName{ + deployment.GetFullyQualifiedHomeserverName(t, "hs1"), }) - alice.SendEventSynced(t, roomID, b.Event{ - Type: "m.room.power_levels", + bob.SendEventSynced(t, roomID, b.Event{ + Type: spec.MRoomPowerLevels, StateKey: b.Ptr(""), - Content: map[string]interface{}{ - "users_default": 40, // we change the default to 40. We should be able to do this. - "users": map[string]interface{}{ - alice.UserID: 100, - }, - "events": map[string]int64{}, - "notifications": map[string]int64{}, - }, + Content: func() map[string]interface{} { + content := map[string]interface{}{ + "users_default": 40, // we change the default to 40. We should be able to do this. + "users": map[string]int64{ + alice.UserID: 100, + bob.UserID: 100, + }, + "events": map[string]int64{}, + "notifications": map[string]int64{}, + } + // Remove the room creator if this is a v12+ room + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(content["users"].(map[string]int64), alice.UserID) + } + return content + }(), }) } @@ -55,6 +80,8 @@ func TestPowerLevels(t *testing.T) { alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + defaultRoomVersion := alice.GetDefaultRoomVersion(t) + roomID := alice.MustCreateRoom(t, map[string]interface{}{}) // sytest: GET /rooms/:room_id/state/m.room.power_levels can fetch levels @@ -65,7 +92,11 @@ func TestPowerLevels(t *testing.T) { // However, for this test, we control the test environment, // and we will assume the server is sane and give us powerlevels as numbers, // and if it doesn't, that's an offense worthy of a frown. - content := alice.MustGetStateEventContent(t, roomID, "m.room.power_levels", "") + // note 2: before v12 the `users` object had to explicitly define the room creator, if they + // should have some power level other than the room's default. Starting with v12, this user + // is to be excluded from the `users` object, as they have an infinite power level that is + // not representable in JSON + content := alice.MustGetStateEventContent(t, roomID, spec.MRoomPowerLevels, "") must.MatchGJSON(t, content, match.JSONKeyTypeEqual("ban", gjson.Number), match.JSONKeyTypeEqual("kick", gjson.Number), @@ -91,13 +122,18 @@ func TestPowerLevels(t *testing.T) { }), func(body gjson.Result) error { - userDefault := int(body.Get("users_default").Num) - thisUser := int(body.Get("users." + client.GjsonEscape(alice.UserID)).Num) - - if thisUser > userDefault { + // This key should be missing for room v12+ + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + must.MatchGJSON(t, body, match.JSONKeyMissing("users." + client.GjsonEscape(alice.UserID))) return nil } else { - return fmt.Errorf("expected room creator (%d) to have a higher-than-default powerlevel (which is %d)", thisUser, userDefault) + userDefault := int(body.Get("users_default").Num) + thisUser := int(body.Get("users." + client.GjsonEscape(alice.UserID)).Num) + if thisUser > userDefault { + return nil + } else { + return fmt.Errorf("expected room creator (%d) to have a higher-than-default powerlevel (which is %d)", thisUser, userDefault) + } } }, ) @@ -114,8 +150,13 @@ func TestPowerLevels(t *testing.T) { }, } + // Rooms versioned 12+ do not allow having the room creator in the 'users' object, so just remove it + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(PLContent["users"].(map[string]interface{}), alice.UserID) + } + eventId := alice.SendEventSynced(t, roomID, b.Event{ - Type: "m.room.power_levels", + Type: spec.MRoomPowerLevels, StateKey: b.Ptr(""), Content: PLContent, }) @@ -137,37 +178,70 @@ func TestPowerLevels(t *testing.T) { t.Run("PUT power_levels should not explode if the old power levels were empty", func(t *testing.T) { // Absence of an "events" key alice.SendEventSynced(t, roomID, b.Event{ - Type: "m.room.power_levels", + Type: spec.MRoomPowerLevels, StateKey: b.Ptr(""), - Content: map[string]interface{}{ - "users": map[string]interface{}{ - alice.UserID: 100, - }, - }, + Content: func() map[string]interface{} { + PLContent := map[string]interface{}{ + "users": map[string]int64{ + alice.UserID: 100, + }, + } + // Rooms versioned 12+ do not allow having the room creator in the 'users' object, so just remove the + // single user so the empty `users` object is present + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + delete(PLContent["users"].(map[string]int64), alice.UserID) + } + return PLContent + }(), }) // Absence of a "users" key alice.SendEventSynced(t, roomID, b.Event{ - Type: "m.room.power_levels", + Type: spec.MRoomPowerLevels, StateKey: b.Ptr(""), Content: map[string]interface{}{}, }) - // This should give a 403 (not a 500) - res := alice.Do( - t, - "PUT", - []string{"_matrix", "client", "v3", "rooms", roomID, "state", "m.room.power_levels"}, - client.WithJSONBody(t, map[string]interface{}{ - "users": map[string]string{}, - }), - ) - must.MatchResponse(t, res, match.HTTPResponse{ - StatusCode: 403, - }) + // This part of the test should check that sending a power_levels event fails. As then the previous + // power_levels event should not have changed. Depending on the room version, this can be done in one + // of two different ways: + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).PrivilegedCreators() { + // For rooms with privileged creators(MSC4289), try and send an event with the room creator in the + // `users` object, which should be prohibited with a 400 error code. An empty `users` object appears + // to be allowed, hence the difference from the below condition for other room versions. + res := alice.Do( + t, + "PUT", + []string{"_matrix", "client", "v3", "rooms", roomID, "state", spec.MRoomPowerLevels}, + client.WithJSONBody(t, map[string]interface{}{ + "users": map[string]int64{ + alice.UserID: 100, + }, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{ + StatusCode: 400, + }) + + } else { + // Prior to rooms using privileged creators, an empty `users` object would be prohibited and should + // give a 403 (not a 500) + res := alice.Do( + t, + "PUT", + []string{"_matrix", "client", "v3", "rooms", roomID, "state", spec.MRoomPowerLevels}, + client.WithJSONBody(t, map[string]interface{}{ + "users": map[string]int64{}, + }), + ) + must.MatchResponse(t, res, match.HTTPResponse{ + StatusCode: 403, + }) + + } // Test if the old state still exists - content := alice.MustGetStateEventContent(t, roomID, "m.room.power_levels", "") + content := alice.MustGetStateEventContent(t, roomID, spec.MRoomPowerLevels, "") must.MatchGJSON(t, content, match.JSONKeyMissing("users")) }) } diff --git a/tests/federation_room_event_auth_test.go b/tests/federation_room_event_auth_test.go index bb08d0735..f209b40c1 100644 --- a/tests/federation_room_event_auth_test.go +++ b/tests/federation_room_event_auth_test.go @@ -110,6 +110,7 @@ func TestInboundFederationRejectsEventsWithRejectedAuthEvents(t *testing.T) { // have Alice create a room, and then join it alice := deployment.Register(t, "hs1", helpers.RegistrationOpts{}) + testRoomID := alice.MustCreateRoom(t, map[string]interface{}{ "preset": "public_chat", }) @@ -145,12 +146,20 @@ func TestInboundFederationRejectsEventsWithRejectedAuthEvents(t *testing.T) { StateKey: &charlie, Sender: charlie, Content: map[string]interface{}{"membership": "join", "test": 1}, - AuthEvents: []string{ - room.CurrentState("m.room.create", "").EventID(), - room.CurrentState("m.room.join_rules", "").EventID(), - rejectedEvent.EventID(), - charlieMembershipEvent.EventID(), - }, + AuthEvents: func() []string { + content := []string{ + room.CurrentState("m.room.create", "").EventID(), + room.CurrentState("m.room.join_rules", "").EventID(), + rejectedEvent.EventID(), + charlieMembershipEvent.EventID(), + } + if gomatrixserverlib.MustGetRoomVersion(room.Version).DomainlessRoomIDs() { + // Room v12+ (MSC4291) does not allow the create event in the auth_events key. This outlier + // is supposed to be invalid, but not for that reason + content = content[1:] + } + return content + }(), }) // add it to room.Timeline so that HandleEventRequests() can find it, but // don't use room.AddEvent(), because we don't want it to be a forward extremity. @@ -159,13 +168,19 @@ func TestInboundFederationRejectsEventsWithRejectedAuthEvents(t *testing.T) { // create a regular event which refers to the outlier event in its auth events, // so that the outlier gets pulled in. - sentEventAuthEvents := []gomatrixserverlib.PDU{ - room.CurrentState("m.room.create", ""), - room.CurrentState("m.room.join_rules", ""), - room.CurrentState("m.room.power_levels", ""), - charlieMembershipEvent, - outlierEvent, - } + sentEventAuthEvents := func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + room.CurrentState("m.room.create", ""), + room.CurrentState("m.room.join_rules", ""), + room.CurrentState("m.room.power_levels", ""), + charlieMembershipEvent, + outlierEvent, + } + if gomatrixserverlib.MustGetRoomVersion(room.Version).DomainlessRoomIDs() { + content = content[1:] + } + return content + }() sentEvent1 := srv.MustCreateEvent(t, room, federation.Event{ Type: "m.room.message", Sender: charlie, diff --git a/tests/msc3902/federation_room_join_partial_state_test.go b/tests/msc3902/federation_room_join_partial_state_test.go index 6cd7e6fd2..c3fc919fa 100644 --- a/tests/msc3902/federation_room_join_partial_state_test.go +++ b/tests/msc3902/federation_room_join_partial_state_test.go @@ -1632,7 +1632,11 @@ func TestPartialStateJoin(t *testing.T) { // update the users map in the PL event for _, ev := range initialRoomEvents { if ev.Type == "m.room.power_levels" { - ev.Content["users"] = map[string]int64{charlie: 100, derek: 50} + if gomatrixserverlib.MustGetRoomVersion(roomVer).PrivilegedCreators() { + ev.Content["users"] = map[string]int64{derek: 50} + } else { + ev.Content["users"] = map[string]int64{charlie: 100, derek: 50} + } } } serverRoom := server.MustMakeRoom(t, roomVer, initialRoomEvents) @@ -1656,11 +1660,17 @@ func TestPartialStateJoin(t *testing.T) { Content: map[string]interface{}{ "body": "bad state event", }, - AuthEvents: serverRoom.EventIDsOrReferences([]gomatrixserverlib.PDU{ - serverRoom.CurrentState("m.room.create", ""), - serverRoom.CurrentState("m.room.power_levels", ""), - derekJoinEvent, - }), + AuthEvents: serverRoom.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + serverRoom.CurrentState("m.room.create", ""), + serverRoom.CurrentState("m.room.power_levels", ""), + derekJoinEvent, + } + if gomatrixserverlib.MustGetRoomVersion(roomVer).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) // add to the timeline, but not the state (so that when testReceiveEventDuringPartialStateJoin checks the state, // it doesn't expect to see this) @@ -1712,7 +1722,13 @@ func TestPartialStateJoin(t *testing.T) { // update the users map in the PL event for _, ev := range initialRoomEvents { if ev.Type == "m.room.power_levels" { - ev.Content["users"] = map[string]int64{charlie: 100, derek: 100, elsie: 50} + if gomatrixserverlib.MustGetRoomVersion(roomVer).PrivilegedCreators() { + ev.Content["users"] = map[string]int64{derek: 100, elsie: 50} + + } else { + ev.Content["users"] = map[string]int64{charlie: 100, derek: 100, elsie: 50} + + } } } serverRoom := server.MustMakeRoom(t, roomVer, initialRoomEvents) @@ -1738,12 +1754,18 @@ func TestPartialStateJoin(t *testing.T) { StateKey: &elsie, Sender: derek, Content: map[string]interface{}{"membership": "leave"}, - AuthEvents: serverRoom.EventIDsOrReferences([]gomatrixserverlib.PDU{ - serverRoom.CurrentState("m.room.create", ""), - serverRoom.CurrentState("m.room.power_levels", ""), - derekJoinEvent, - elsieJoinEvent, - }), + AuthEvents: serverRoom.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + serverRoom.CurrentState("m.room.create", ""), + serverRoom.CurrentState("m.room.power_levels", ""), + derekJoinEvent, + elsieJoinEvent, + } + if gomatrixserverlib.MustGetRoomVersion(roomVer).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) // add to the timeline, but not the state (so that when testReceiveEventDuringPartialStateJoin checks the state, // it doesn't expect to see this) @@ -1758,11 +1780,17 @@ func TestPartialStateJoin(t *testing.T) { StateKey: b.Ptr(""), Sender: elsie, Content: map[string]interface{}{"body": "rejected state"}, - AuthEvents: serverRoom.EventIDsOrReferences([]gomatrixserverlib.PDU{ - serverRoom.CurrentState("m.room.create", ""), - serverRoom.CurrentState("m.room.power_levels", ""), - elsieJoinEvent, - }), + AuthEvents: serverRoom.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + serverRoom.CurrentState("m.room.create", ""), + serverRoom.CurrentState("m.room.power_levels", ""), + elsieJoinEvent, + } + if gomatrixserverlib.MustGetRoomVersion(roomVer).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) serverRoom.AddEvent(rejectedStateEvent) t.Logf("elsie created state event %s", rejectedStateEvent.EventID()) @@ -2356,6 +2384,7 @@ func TestPartialStateJoin(t *testing.T) { ) (syncToken string, server2Room *federation.ServerRoom, psjResult partialStateJoinResult) { derek := server1.UserID("derek") elsie := server2.UserID("elsie") + roomVersion := room.Version // The room starts with @charlie:server1 and @derek:server1 in it. // @derek:server1 becomes an admin. @@ -2402,11 +2431,17 @@ func TestPartialStateJoin(t *testing.T) { StateKey: b.Ptr(elsie), Sender: derek, Content: map[string]interface{}{"membership": "leave"}, - AuthEvents: room.EventIDsOrReferences([]gomatrixserverlib.PDU{ - room.CurrentState("m.room.create", ""), - room.CurrentState("m.room.power_levels", ""), - derekJoinEvent, - }), + AuthEvents: room.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + room.CurrentState("m.room.create", ""), + room.CurrentState("m.room.power_levels", ""), + derekJoinEvent, + } + if gomatrixserverlib.MustGetRoomVersion(roomVersion).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) room.Timeline = append(room.Timeline, badKickEvent) room.Depth = badKickEvent.Depth() @@ -3189,11 +3224,17 @@ func TestPartialStateJoin(t *testing.T) { StateKey: b.Ptr(derek), Sender: fred, Content: map[string]interface{}{"membership": "ban"}, - AuthEvents: room.EventIDsOrReferences([]gomatrixserverlib.PDU{ - room.CurrentState("m.room.create", ""), - room.CurrentState("m.room.power_levels", ""), - fredJoinEvent, - }), + AuthEvents: room.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + room.CurrentState("m.room.create", ""), + room.CurrentState("m.room.power_levels", ""), + fredJoinEvent, + } + if gomatrixserverlib.MustGetRoomVersion(room.Version).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) room.Timeline = append(room.Timeline, badKickEvent) room.Depth = badKickEvent.Depth() @@ -3813,7 +3854,8 @@ func TestPartialStateJoin(t *testing.T) { cancel := server.Listen() defer cancel() - serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) + defaultRoomVersion := alice.GetDefaultRoomVersion(t) + serverRoom := createTestRoom(t, server, defaultRoomVersion) t.Log("Alice partial-joins her room") psjResult := beginPartialStateJoin(t, server, serverRoom, alice) @@ -3830,12 +3872,18 @@ func TestPartialStateJoin(t *testing.T) { StateKey: b.Ptr(alice.UserID), Sender: server.UserID("charlie"), Content: map[string]interface{}{"membership": "leave"}, - AuthEvents: serverRoom.EventIDsOrReferences([]gomatrixserverlib.PDU{ - serverRoom.CurrentState("m.room.create", ""), - serverRoom.CurrentState("m.room.power_levels", ""), - serverRoom.CurrentState("m.room.member", alice.UserID), - serverRoom.CurrentState("m.room.member", server.UserID("charlie")), - }), + AuthEvents: serverRoom.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + serverRoom.CurrentState("m.room.create", ""), + serverRoom.CurrentState("m.room.power_levels", ""), + serverRoom.CurrentState("m.room.member", alice.UserID), + serverRoom.CurrentState("m.room.member", server.UserID("charlie")), + } + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) serverRoom.AddEvent(kickEvent) server.MustSendTransaction(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), []json.RawMessage{kickEvent.JSON()}, nil) @@ -3865,7 +3913,8 @@ func TestPartialStateJoin(t *testing.T) { cancel := server.Listen() defer cancel() - serverRoom := createTestRoom(t, server, alice.GetDefaultRoomVersion(t)) + defaultRoomVersion := alice.GetDefaultRoomVersion(t) + serverRoom := createTestRoom(t, server, defaultRoomVersion) t.Log("Alice partial-joins her room") psjResult := beginPartialStateJoin(t, server, serverRoom, alice) @@ -3882,12 +3931,18 @@ func TestPartialStateJoin(t *testing.T) { StateKey: b.Ptr(alice.UserID), Sender: server.UserID("charlie"), Content: map[string]interface{}{"membership": "ban"}, - AuthEvents: serverRoom.EventIDsOrReferences([]gomatrixserverlib.PDU{ - serverRoom.CurrentState("m.room.create", ""), - serverRoom.CurrentState("m.room.power_levels", ""), - serverRoom.CurrentState("m.room.member", alice.UserID), - serverRoom.CurrentState("m.room.member", server.UserID("charlie")), - }), + AuthEvents: serverRoom.EventIDsOrReferences(func() []gomatrixserverlib.PDU { + content := []gomatrixserverlib.PDU{ + serverRoom.CurrentState("m.room.create", ""), + serverRoom.CurrentState("m.room.power_levels", ""), + serverRoom.CurrentState("m.room.member", alice.UserID), + serverRoom.CurrentState("m.room.member", server.UserID("charlie")), + } + if gomatrixserverlib.MustGetRoomVersion(defaultRoomVersion).DomainlessRoomIDs() { + content = content[1:] + } + return content + }()), }) serverRoom.AddEvent(banEvent) server.MustSendTransaction(t, deployment, deployment.GetFullyQualifiedHomeserverName(t, "hs1"), []json.RawMessage{banEvent.JSON()}, nil)