From b98e50f5443c28beca3ea1edbdaef1ee2fa549b6 Mon Sep 17 00:00:00 2001 From: Sam Wedgwood Date: Thu, 6 Jul 2023 13:38:10 +0100 Subject: [PATCH] Improve room hiearchy error handling + comments --- clientapi/routing/room_hierarchy.go | 18 +++++++++++++++++- clientapi/routing/routing.go | 1 + federationapi/routing/query.go | 18 +++++++++++++++++- roomserver/api/api.go | 11 +++++++++++ roomserver/api/query.go | 8 ++++++++ .../internal/query/query_room_hierarchy.go | 8 ++++++-- 6 files changed, 60 insertions(+), 4 deletions(-) diff --git a/clientapi/routing/room_hierarchy.go b/clientapi/routing/room_hierarchy.go index e646461e5..c9f53b17e 100644 --- a/clientapi/routing/room_hierarchy.go +++ b/clientapi/routing/room_hierarchy.go @@ -26,6 +26,7 @@ import ( "github.com/matrix-org/gomatrixserverlib/fclient" "github.com/matrix-org/gomatrixserverlib/spec" "github.com/matrix-org/util" + log "github.com/sirupsen/logrus" ) type RoomHierarchyPaginationCache struct { @@ -54,6 +55,9 @@ func (c *RoomHierarchyPaginationCache) AddLine(line roomserverAPI.CachedRoomHier return token } +// Query the hierarchy of a room/space +// +// Implements /_matrix/client/v1/rooms/{roomID}/hierarchy func QueryRoomHierarchy(req *http.Request, device *userapi.Device, roomIDStr string, rsAPI roomserverAPI.ClientRoomserverAPI, paginationCache *RoomHierarchyPaginationCache) util.JSONResponse { parsedRoomID, err := spec.NewRoomID(roomIDStr) if err != nil { @@ -127,7 +131,19 @@ func QueryRoomHierarchy(req *http.Request, device *userapi.Device, roomIDStr str discoveredRooms, err := walker.NextPage(limit) if err != nil { - // TODO + switch err.(type) { + case roomserverAPI.ErrRoomUnknownOrNotAllowed: + return util.JSONResponse{ + Code: http.StatusForbidden, + JSON: spec.Forbidden("room is unknown/forbidden"), + } + default: + log.WithError(err).Errorf("failed to fetch next page of room hierarchy (CS API)") + return util.JSONResponse{ + Code: http.StatusInternalServerError, + JSON: spec.Unknown("internal server error"), + } + } } nextBatch := "" diff --git a/clientapi/routing/routing.go b/clientapi/routing/routing.go index dbd607eaf..5e691e8ec 100644 --- a/clientapi/routing/routing.go +++ b/clientapi/routing/routing.go @@ -508,6 +508,7 @@ func Setup( ).Methods(http.MethodPut, http.MethodOptions) // Defined outside of handler to persist between calls + // TODO: clear based on some criteria roomHierarchyPaginationCache := new(RoomHierarchyPaginationCache) v1mux.Handle("/rooms/{roomID}/hierarchy", httputil.MakeAuthAPI("spaces", userAPI, func(req *http.Request, device *userapi.Device) util.JSONResponse { diff --git a/federationapi/routing/query.go b/federationapi/routing/query.go index c8e466431..d52695d0f 100644 --- a/federationapi/routing/query.go +++ b/federationapi/routing/query.go @@ -27,6 +27,7 @@ import ( "github.com/matrix-org/gomatrixserverlib/fclient" "github.com/matrix-org/gomatrixserverlib/spec" "github.com/matrix-org/util" + log "github.com/sirupsen/logrus" ) // RoomAliasToID converts the queried alias into a room ID and returns it @@ -118,6 +119,9 @@ func RoomAliasToID( } } +// Query the immediate children of a room/space +// +// Implements /_matrix/federation/v1/hierarchy/{roomID} func QueryRoomHierarchy(httpReq *http.Request, request *fclient.FederationRequest, roomIDStr string, rsAPI roomserverAPI.FederationRoomserverAPI) util.JSONResponse { parsedRoomID, err := spec.NewRoomID(roomIDStr) if err != nil { @@ -146,7 +150,19 @@ func QueryRoomHierarchy(httpReq *http.Request, request *fclient.FederationReques discoveredRooms, err := walker.NextPage(-1) if err != nil { - // TODO + switch err.(type) { + case roomserverAPI.ErrRoomUnknownOrNotAllowed: + return util.JSONResponse{ + Code: http.StatusNotFound, + JSON: spec.NotFound("room is unknown/forbidden"), + } + default: + log.WithError(err).Errorf("failed to fetch next page of room hierarchy (SS API)") + return util.JSONResponse{ + Code: http.StatusInternalServerError, + JSON: spec.Unknown("internal server error"), + } + } } if len(discoveredRooms) == 0 { diff --git a/roomserver/api/api.go b/roomserver/api/api.go index c10a8bded..a6e0d5d75 100644 --- a/roomserver/api/api.go +++ b/roomserver/api/api.go @@ -34,6 +34,17 @@ func (e ErrNotAllowed) Error() string { return e.Err.Error() } +// ErrRoomUnknownOrNotAllowed is an error return if either the provided +// room ID does not exist, or points to a room that the requester does +// not have access to. +type ErrRoomUnknownOrNotAllowed struct { + Err error +} + +func (e ErrRoomUnknownOrNotAllowed) Error() string { + return e.Err.Error() +} + type RestrictedJoinAPI interface { CurrentStateEvent(ctx context.Context, roomID spec.RoomID, eventType string, stateKey string) (gomatrixserverlib.PDU, error) InvitePending(ctx context.Context, roomID spec.RoomID, senderID spec.SenderID) (bool, error) diff --git a/roomserver/api/query.go b/roomserver/api/query.go index ded27b853..a88a40239 100644 --- a/roomserver/api/query.go +++ b/roomserver/api/query.go @@ -512,9 +512,17 @@ type QueryRoomHierarchyRequest struct { From int `json:"json"` } +// An iterator-like interface for walking a room/space hierarchy, returning each rooms information. +// +// Used for implementing space summaries / room hierarchies type RoomHierarchyWalker interface { + // Walk the room hierarchy to retrieve room information until either + // no room left, or provided limit reached. If limit provided is -1, then this is + // treated as no limit. NextPage(limit int) ([]fclient.MSC2946Room, error) + // Returns true if there are no more rooms left to walk Done() bool + // Returns a stripped down version of the hiearchy walker suitable for pagination caching GetCached() CachedRoomHierarchyWalker } diff --git a/roomserver/internal/query/query_room_hierarchy.go b/roomserver/internal/query/query_room_hierarchy.go index 9d49622a6..1c2eabdcd 100644 --- a/roomserver/internal/query/query_room_hierarchy.go +++ b/roomserver/internal/query/query_room_hierarchy.go @@ -17,6 +17,7 @@ package query import ( "context" "encoding/json" + "fmt" "sort" "strings" @@ -92,9 +93,12 @@ const ( ConstSpaceParentEventType = "m.space.parent" ) +// Walk the room hierarchy to retrieve room information until either +// no room left, or provided limit reached. If limit provided is -1, then this is +// treated as no limit. func (w *RoomHierarchyWalker) NextPage(limit int) ([]fclient.MSC2946Room, error) { if authorised, _ := w.authorised(w.rootRoomID, ""); !authorised { - return nil, spec.Forbidden("room is unknown/forbidden") + return nil, roomserver.ErrRoomUnknownOrNotAllowed{Err: fmt.Errorf("room is unknown/forbidden")} } var discoveredRooms []fclient.MSC2946Room @@ -277,7 +281,7 @@ func (w *RoomHierarchyWalker) federatedRoomInfo(roomID string, vias []string) *f } res, err := w.fsAPI.RoomHierarchies(ctx, w.thisServer, spec.ServerName(serverName), roomID, w.suggestedOnly) if err != nil { - util.GetLogger(w.ctx).WithError(err).Warnf("failed to call MSC2946Spaces on server %s", serverName) + util.GetLogger(w.ctx).WithError(err).Warnf("failed to call RoomHierarchies on server %s", serverName) continue } // ensure nil slices are empty as we send this to the client sometimes