From 1467cc10d8bd40b9ea85bc4f0aa4644b16cb37bb Mon Sep 17 00:00:00 2001 From: Kegsay Date: Tue, 17 Mar 2020 17:18:48 +0000 Subject: [PATCH 1/2] bugfix: Fix a bug which caused failures to join rooms over federation (#917) * bugfix: Fix a bug which caused failures to join rooms over federation The cause of this was the semantics of `/send_join`'s `auth_chain` response. Previously, we would only send back the auth chain *for the join event* and not the entire room state. However, we would then try to check that the room state is valid, and then be missing auth events. Now, we send back the entire auth chain for all room state in `/send_join`. The spec needs to be clarified that this is what the chain should be. * refactor: split out grabbing state to reduce cyclo complexity --- roomserver/query/query.go | 62 +++++++++++++++++++++++---------------- 1 file changed, 37 insertions(+), 25 deletions(-) diff --git a/roomserver/query/query.go b/roomserver/query/query.go index 52b678ac3..2de8e0d08 100644 --- a/roomserver/query/query.go +++ b/roomserver/query/query.go @@ -637,12 +637,6 @@ func (r *RoomserverQueryAPI) QueryStateAndAuthChain( request *api.QueryStateAndAuthChainRequest, response *api.QueryStateAndAuthChainResponse, ) error { - // TODO: get the correct room version - roomState, err := state.GetStateResolutionAlgorithm(state.StateResolutionAlgorithmV1, r.DB) - if err != nil { - return err - } - response.QueryStateAndAuthChainRequest = *request roomNID, err := r.DB.RoomNID(ctx, request.RoomID) if err != nil { @@ -653,31 +647,21 @@ func (r *RoomserverQueryAPI) QueryStateAndAuthChain( } response.RoomExists = true - prevStates, err := r.DB.StateAtEventIDs(ctx, request.PrevEventIDs) + stateEvents, err := r.loadStateAtEventIDs(ctx, request.PrevEventIDs) if err != nil { - switch err.(type) { - case types.MissingEventError: - return nil - default: - return err - } + return err } response.PrevEventsExist = true - // Look up the currrent state for the requested tuples. - stateEntries, err := roomState.LoadCombinedStateAfterEvents( - ctx, prevStates, - ) - if err != nil { - return err + // add the auth event IDs for the current state events too + var authEventIDs []string + authEventIDs = append(authEventIDs, request.AuthEventIDs...) + for _, se := range stateEvents { + authEventIDs = append(authEventIDs, se.AuthEventIDs()...) } + authEventIDs = util.UniqueStrings(authEventIDs) // de-dupe - stateEvents, err := r.loadStateEvents(ctx, stateEntries) - if err != nil { - return err - } - - authEvents, err := getAuthChain(ctx, r.DB, request.AuthEventIDs) + authEvents, err := getAuthChain(ctx, r.DB, authEventIDs) if err != nil { return err } @@ -699,6 +683,34 @@ func (r *RoomserverQueryAPI) QueryStateAndAuthChain( return err } +func (r *RoomserverQueryAPI) loadStateAtEventIDs(ctx context.Context, eventIDs []string) ([]gomatrixserverlib.Event, error) { + // TODO: get the correct room version + roomState, err := state.GetStateResolutionAlgorithm(state.StateResolutionAlgorithmV1, r.DB) + if err != nil { + return nil, err + } + + prevStates, err := r.DB.StateAtEventIDs(ctx, eventIDs) + if err != nil { + switch err.(type) { + case types.MissingEventError: + return nil, nil + default: + return nil, err + } + } + + // Look up the currrent state for the requested tuples. + stateEntries, err := roomState.LoadCombinedStateAfterEvents( + ctx, prevStates, + ) + if err != nil { + return nil, err + } + + return r.loadStateEvents(ctx, stateEntries) +} + // getAuthChain fetches the auth chain for the given auth events. An auth chain // is the list of all events that are referenced in the auth_events section, and // all their auth_events, recursively. The returned set of events contain the From c2bd0b97b34c9040325bf31c5c4a2a06579239d9 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 17 Mar 2020 18:00:10 +0000 Subject: [PATCH 2/2] Get room versions from database (#918) * Retrieve room version where known in roomserver * Get room versions in alias code * Increase gocyclothreshold to 13, since we hit that number a lot * Remove gocyclo nolint from StoreEvent * Update interface to get room version from room ID instead of NID * Remove new API * Fixed this query for SQLite but not for Postgres --- .golangci.yml | 2 +- roomserver/alias/alias.go | 12 ++++++-- roomserver/alias/alias_test.go | 7 +++++ roomserver/input/events.go | 4 +++ roomserver/input/latest_events.go | 6 ++-- roomserver/query/query.go | 35 ++++++++++++---------- roomserver/storage/interface.go | 2 +- roomserver/storage/postgres/rooms_table.go | 16 +++++----- roomserver/storage/postgres/storage.go | 6 ++-- roomserver/storage/sqlite3/rooms_table.go | 16 +++++----- roomserver/storage/sqlite3/storage.go | 7 ++--- 11 files changed, 68 insertions(+), 45 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index 0d0f51bd2..7fdd4d003 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -102,7 +102,7 @@ linters-settings: #local-prefixes: github.com/org/project gocyclo: # minimal code complexity to report, 30 by default (but we recommend 10-20) - min-complexity: 12 + min-complexity: 13 maligned: # print struct with more effective memory layout or not, false by default suggest-new: true diff --git a/roomserver/alias/alias.go b/roomserver/alias/alias.go index f4f5c20ce..dfb7e76a7 100644 --- a/roomserver/alias/alias.go +++ b/roomserver/alias/alias.go @@ -46,6 +46,10 @@ type RoomserverAliasAPIDatabase interface { // Remove a given room alias. // Returns an error if there was a problem talking to the database. RemoveRoomAlias(ctx context.Context, alias string) error + // Look up the room version for a given room. + GetRoomVersionForRoom( + ctx context.Context, roomID string, + ) (gomatrixserverlib.RoomVersion, error) } // RoomserverAliasAPI is an implementation of alias.RoomserverAliasAPI @@ -240,6 +244,11 @@ func (r *RoomserverAliasAPI) sendUpdatedAliasesEvent( } builder.AuthEvents = refs + roomVersion, err := r.DB.GetRoomVersionForRoom(ctx, roomID) + if err != nil { + return err + } + // Build the event eventID := fmt.Sprintf("$%s:%s", util.RandomString(16), r.Cfg.Matrix.ServerName) now := time.Now() @@ -250,9 +259,6 @@ func (r *RoomserverAliasAPI) sendUpdatedAliasesEvent( return err } - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 - // Create the request ire := roomserverAPI.InputRoomEvent{ Kind: roomserverAPI.KindNew, diff --git a/roomserver/alias/alias_test.go b/roomserver/alias/alias_test.go index 6ddb63a73..0aefa19d9 100644 --- a/roomserver/alias/alias_test.go +++ b/roomserver/alias/alias_test.go @@ -22,6 +22,7 @@ import ( appserviceAPI "github.com/matrix-org/dendrite/appservice/api" roomserverAPI "github.com/matrix-org/dendrite/roomserver/api" + "github.com/matrix-org/gomatrixserverlib" ) type MockRoomserverAliasAPIDatabase struct { @@ -49,6 +50,12 @@ func (db *MockRoomserverAliasAPIDatabase) GetCreatorIDForAlias( return "", nil } +func (db *MockRoomserverAliasAPIDatabase) GetRoomVersionForRoom( + ctx context.Context, roomID string, +) (gomatrixserverlib.RoomVersion, error) { + return gomatrixserverlib.RoomVersionV1, nil +} + // This method needs to change depending on test case func (db *MockRoomserverAliasAPIDatabase) GetRoomIDForAlias( ctx context.Context, diff --git a/roomserver/input/events.go b/roomserver/input/events.go index 7fbc5d8a9..034b06c11 100644 --- a/roomserver/input/events.go +++ b/roomserver/input/events.go @@ -72,6 +72,10 @@ type RoomEventDatabase interface { ctx context.Context, transactionID string, sessionID int64, userID string, ) (string, error) + // Look up the room version for a given room. + GetRoomVersionForRoom( + ctx context.Context, roomID string, + ) (gomatrixserverlib.RoomVersion, error) } // OutputRoomEventWriter has the APIs needed to write an event to the output logs. diff --git a/roomserver/input/latest_events.go b/roomserver/input/latest_events.go index 9a99ad76f..4d75daae9 100644 --- a/roomserver/input/latest_events.go +++ b/roomserver/input/latest_events.go @@ -253,8 +253,10 @@ func (u *latestEventsUpdater) makeOutputNewRoomEvent() (*api.OutputEvent, error) latestEventIDs[i] = u.latest[i].EventID } - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 + roomVersion, err := u.db.GetRoomVersionForRoom(u.ctx, u.event.RoomID()) + if err != nil { + return nil, err + } ore := api.OutputNewRoomEvent{ Event: u.event.Headered(roomVersion), diff --git a/roomserver/query/query.go b/roomserver/query/query.go index 2de8e0d08..3ab12d60a 100644 --- a/roomserver/query/query.go +++ b/roomserver/query/query.go @@ -91,7 +91,7 @@ type RoomserverQueryAPIDatabase interface { ) (map[types.EventStateKeyNID]string, error) // Look up the room version for a given room. GetRoomVersionForRoom( - ctx context.Context, roomNID types.RoomNID, + ctx context.Context, roomID string, ) (gomatrixserverlib.RoomVersion, error) } @@ -121,7 +121,7 @@ func (r *RoomserverQueryAPI) QueryLatestEventsAndState( } response.RoomExists = true - roomVersion, err := r.DB.GetRoomVersionForRoom(ctx, roomNID) + roomVersion, err := r.DB.GetRoomVersionForRoom(ctx, request.RoomID) if err != nil { return err } @@ -174,7 +174,7 @@ func (r *RoomserverQueryAPI) QueryStateAfterEvents( } response.RoomExists = true - roomVersion, err := r.DB.GetRoomVersionForRoom(ctx, roomNID) + roomVersion, err := r.DB.GetRoomVersionForRoom(ctx, request.RoomID) if err != nil { return err } @@ -234,8 +234,10 @@ func (r *RoomserverQueryAPI) QueryEventsByID( } for _, event := range events { - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 + roomVersion, verr := r.DB.GetRoomVersionForRoom(ctx, event.RoomID()) + if verr != nil { + return verr + } response.Events = append(response.Events, event.Headered(roomVersion)) } @@ -516,8 +518,10 @@ func (r *RoomserverQueryAPI) QueryMissingEvents( response.Events = make([]gomatrixserverlib.HeaderedEvent, 0, len(loadedEvents)-len(eventsToFilter)) for _, event := range loadedEvents { if !eventsToFilter[event.EventID()] { - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 + roomVersion, verr := r.DB.GetRoomVersionForRoom(ctx, event.RoomID()) + if verr != nil { + return verr + } response.Events = append(response.Events, event.Headered(roomVersion)) } @@ -562,8 +566,10 @@ func (r *RoomserverQueryAPI) QueryBackfill( } for _, event := range loadedEvents { - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 + roomVersion, verr := r.DB.GetRoomVersionForRoom(ctx, event.RoomID()) + if verr != nil { + return verr + } response.Events = append(response.Events, event.Headered(roomVersion)) } @@ -647,6 +653,11 @@ func (r *RoomserverQueryAPI) QueryStateAndAuthChain( } response.RoomExists = true + roomVersion, err := r.DB.GetRoomVersionForRoom(ctx, request.RoomID) + if err != nil { + return err + } + stateEvents, err := r.loadStateAtEventIDs(ctx, request.PrevEventIDs) if err != nil { return err @@ -667,16 +678,10 @@ func (r *RoomserverQueryAPI) QueryStateAndAuthChain( } for _, event := range stateEvents { - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 - response.StateEvents = append(response.StateEvents, event.Headered(roomVersion)) } for _, event := range authEvents { - // TODO: Room version here - roomVersion := gomatrixserverlib.RoomVersionV1 - response.AuthChainEvents = append(response.AuthChainEvents, event.Headered(roomVersion)) } diff --git a/roomserver/storage/interface.go b/roomserver/storage/interface.go index 7f32b53f8..20db7ef7f 100644 --- a/roomserver/storage/interface.go +++ b/roomserver/storage/interface.go @@ -45,5 +45,5 @@ type Database interface { GetMembership(ctx context.Context, roomNID types.RoomNID, requestSenderUserID string) (membershipEventNID types.EventNID, stillInRoom bool, err error) GetMembershipEventNIDsForRoom(ctx context.Context, roomNID types.RoomNID, joinOnly bool) ([]types.EventNID, error) EventsFromIDs(ctx context.Context, eventIDs []string) ([]types.Event, error) - GetRoomVersionForRoom(ctx context.Context, roomNID types.RoomNID) (gomatrixserverlib.RoomVersion, error) + GetRoomVersionForRoom(ctx context.Context, roomID string) (gomatrixserverlib.RoomVersion, error) } diff --git a/roomserver/storage/postgres/rooms_table.go b/roomserver/storage/postgres/rooms_table.go index ef8b8ecef..6bb96f1de 100644 --- a/roomserver/storage/postgres/rooms_table.go +++ b/roomserver/storage/postgres/rooms_table.go @@ -65,8 +65,8 @@ const selectLatestEventNIDsForUpdateSQL = "" + const updateLatestEventNIDsSQL = "" + "UPDATE roomserver_rooms SET latest_event_nids = $2, last_event_sent_nid = $3, state_snapshot_nid = $4 WHERE room_nid = $1" -const selectRoomVersionForRoomNIDSQL = "" + - "SELECT room_version FROM roomserver_rooms WHERE room_nid = $1" +const selectRoomVersionForRoomIDSQL = "" + + "SELECT room_version FROM roomserver_rooms WHERE room_id = $1" type roomStatements struct { insertRoomNIDStmt *sql.Stmt @@ -74,7 +74,7 @@ type roomStatements struct { selectLatestEventNIDsStmt *sql.Stmt selectLatestEventNIDsForUpdateStmt *sql.Stmt updateLatestEventNIDsStmt *sql.Stmt - selectRoomVersionForRoomNIDStmt *sql.Stmt + selectRoomVersionForRoomIDStmt *sql.Stmt } func (s *roomStatements) prepare(db *sql.DB) (err error) { @@ -88,7 +88,7 @@ func (s *roomStatements) prepare(db *sql.DB) (err error) { {&s.selectLatestEventNIDsStmt, selectLatestEventNIDsSQL}, {&s.selectLatestEventNIDsForUpdateStmt, selectLatestEventNIDsForUpdateSQL}, {&s.updateLatestEventNIDsStmt, updateLatestEventNIDsSQL}, - {&s.selectRoomVersionForRoomNIDStmt, selectRoomVersionForRoomNIDSQL}, + {&s.selectRoomVersionForRoomIDStmt, selectRoomVersionForRoomIDSQL}, }.prepare(db) } @@ -165,11 +165,11 @@ func (s *roomStatements) updateLatestEventNIDs( return err } -func (s *roomStatements) selectRoomVersionForRoomNID( - ctx context.Context, txn *sql.Tx, roomNID types.RoomNID, +func (s *roomStatements) selectRoomVersionForRoomID( + ctx context.Context, txn *sql.Tx, roomID string, ) (gomatrixserverlib.RoomVersion, error) { var roomVersion gomatrixserverlib.RoomVersion - stmt := common.TxStmt(txn, s.selectRoomVersionForRoomNIDStmt) - err := stmt.QueryRowContext(ctx, roomNID).Scan(&roomVersion) + stmt := common.TxStmt(txn, s.selectRoomVersionForRoomIDStmt) + err := stmt.QueryRowContext(ctx, roomID).Scan(&roomVersion) return roomVersion, err } diff --git a/roomserver/storage/postgres/storage.go b/roomserver/storage/postgres/storage.go index b2b4159c9..af6afe5c1 100644 --- a/roomserver/storage/postgres/storage.go +++ b/roomserver/storage/postgres/storage.go @@ -740,10 +740,10 @@ func (d *Database) EventsFromIDs(ctx context.Context, eventIDs []string) ([]type } func (d *Database) GetRoomVersionForRoom( - ctx context.Context, roomNID types.RoomNID, + ctx context.Context, roomID string, ) (gomatrixserverlib.RoomVersion, error) { - return d.statements.selectRoomVersionForRoomNID( - ctx, nil, roomNID, + return d.statements.selectRoomVersionForRoomID( + ctx, nil, roomID, ) } diff --git a/roomserver/storage/sqlite3/rooms_table.go b/roomserver/storage/sqlite3/rooms_table.go index b750f63ea..49fa07ea8 100644 --- a/roomserver/storage/sqlite3/rooms_table.go +++ b/roomserver/storage/sqlite3/rooms_table.go @@ -54,8 +54,8 @@ const selectLatestEventNIDsForUpdateSQL = "" + const updateLatestEventNIDsSQL = "" + "UPDATE roomserver_rooms SET latest_event_nids = $1, last_event_sent_nid = $2, state_snapshot_nid = $3 WHERE room_nid = $4" -const selectRoomVersionForRoomNIDSQL = "" + - "SELECT room_version FROM roomserver_rooms WHERE room_nid = $1" +const selectRoomVersionForRoomIDSQL = "" + + "SELECT room_version FROM roomserver_rooms WHERE room_id = $1" type roomStatements struct { insertRoomNIDStmt *sql.Stmt @@ -63,7 +63,7 @@ type roomStatements struct { selectLatestEventNIDsStmt *sql.Stmt selectLatestEventNIDsForUpdateStmt *sql.Stmt updateLatestEventNIDsStmt *sql.Stmt - selectRoomVersionForRoomNIDStmt *sql.Stmt + selectRoomVersionForRoomIDStmt *sql.Stmt } func (s *roomStatements) prepare(db *sql.DB) (err error) { @@ -77,7 +77,7 @@ func (s *roomStatements) prepare(db *sql.DB) (err error) { {&s.selectLatestEventNIDsStmt, selectLatestEventNIDsSQL}, {&s.selectLatestEventNIDsForUpdateStmt, selectLatestEventNIDsForUpdateSQL}, {&s.updateLatestEventNIDsStmt, updateLatestEventNIDsSQL}, - {&s.selectRoomVersionForRoomNIDStmt, selectRoomVersionForRoomNIDSQL}, + {&s.selectRoomVersionForRoomIDStmt, selectRoomVersionForRoomIDSQL}, }.prepare(db) } @@ -157,11 +157,11 @@ func (s *roomStatements) updateLatestEventNIDs( return err } -func (s *roomStatements) selectRoomVersionForRoomNID( - ctx context.Context, txn *sql.Tx, roomNID types.RoomNID, +func (s *roomStatements) selectRoomVersionForRoomID( + ctx context.Context, txn *sql.Tx, roomID string, ) (gomatrixserverlib.RoomVersion, error) { var roomVersion gomatrixserverlib.RoomVersion - stmt := common.TxStmt(txn, s.selectRoomVersionForRoomNIDStmt) - err := stmt.QueryRowContext(ctx, roomNID).Scan(&roomVersion) + stmt := common.TxStmt(txn, s.selectRoomVersionForRoomIDStmt) + err := stmt.QueryRowContext(ctx, roomID).Scan(&roomVersion) return roomVersion, err } diff --git a/roomserver/storage/sqlite3/storage.go b/roomserver/storage/sqlite3/storage.go index b912b1c0e..ea926ca66 100644 --- a/roomserver/storage/sqlite3/storage.go +++ b/roomserver/storage/sqlite3/storage.go @@ -70,7 +70,6 @@ func Open(dataSourceName string) (*Database, error) { } // StoreEvent implements input.EventDatabase -// nolint:gocyclo func (d *Database) StoreEvent( ctx context.Context, event gomatrixserverlib.Event, txnAndSessionID *api.TransactionID, authEventNIDs []types.EventNID, @@ -895,10 +894,10 @@ func (d *Database) EventsFromIDs(ctx context.Context, eventIDs []string) ([]type } func (d *Database) GetRoomVersionForRoom( - ctx context.Context, roomNID types.RoomNID, + ctx context.Context, roomID string, ) (gomatrixserverlib.RoomVersion, error) { - return d.statements.selectRoomVersionForRoomNID( - ctx, nil, roomNID, + return d.statements.selectRoomVersionForRoomID( + ctx, nil, roomID, ) }