Synapse implementation of MSC2946 Spaces Server-server API is against spec?

[jhulkko]

Description

Synapse fails to respond to a Spaces Summary request from Dendrite and returns M_BAD_JSON: bad value for 'exclude_rooms' instead.

According to the specification exclude_rooms is an optional field.

exclude_rooms: Optional. A list of room IDs that can be omitted from the response.

ref. https://github.com/matrix-org/matrix-doc/pull/2946/files

Dendrite currently only sends suggested_only and max_rooms_per_space fields on the request.

ref: https://github.com/matrix-org/dendrite/blob/a7f2845a6a97af0c1855eb3c8d81cb4ca348d176/setup/mscs/msc2946/msc2946.go#L416

Steps to reproduce

• Have account on a HS running Dendrite
• Join a space that only points to a synapse based HS (eg. #community:nixos.org )
• Try exploring suggested rooms on a client
• Client fails to load the listing that should be there

How to circumvent

Add empty exclude_rooms list to the outgoing JSON request on Dendrite:

+++ b/setup/mscs/msc2946/msc2946.go
@@ -401,6 +401,7 @@ func (w *walker) federatedRoomInfo(roomID string) (*gomatrixserverlib.MSC2946Spa
                res, err := w.fsAPI.MSC2946Spaces(ctx, gomatrixserverlib.ServerName(serverName), roomID, gomatrixserverlib.MSC2946SpacesRequest{
                        Limit:            w.req.Limit,
                        MaxRoomsPerSpace: w.req.MaxRoomsPerSpace,
+                       ExcludeRooms:     []string{""},
                })
                if err != nil {
                        util.GetLogger(w.ctx).WithError(err).Warnf("failed to call MSC2946Spaces on server %s", serverName)

After adding the empty list to the request, synapse will respond to the request as intended.

Suggested fix

Do not require exclude_rooms on synapse's side, or propose a change of spec if it should be needed. The other optional fields might currently be required by the synapse implementation as well.

ref. for code on exclude_rooms:

synapse/synapse/federation/transport/server.py

Lines 1418 to 1422 in 3e831f2

	exclude_rooms = content.get("exclude_rooms", [])
	if not isinstance(exclude_rooms, list) or any(
	not isinstance(x, str) for x in exclude_rooms
	):
	raise SynapseError(400, "bad value for 'exclude_rooms'", Codes.BAD_JSON)

Version information

Encountered against:

• matrix.org: Synapse 1.33.2 (b=matrix-org-hotfixes,019ed44b8,dirty)
• nixos.org: Synapse 1.34.0

[babolivier]

It looks like gomatrixserverlib has a bogus declaration for its request structure. We can see in the code that only Limit and MaxRoomPerSpace are defined:

gomatrixserverlib.MSC2946SpacesRequest{
    Limit:            w.req.Limit,
    MaxRoomsPerSpace: w.req.MaxRoomsPerSpace
}

ExcludeRooms is defined as a slice of strings ([]string): https://github.com/matrix-org/gomatrixserverlib/blob/6142fe3f8c2c836d0cfcaf2c9766ab3fc77c1e27/federationtypes.go#L980 - therefore sending the request like this would mean it would include exclude_rooms: null (since the zero value of a slice in Go is nil), which would fail the type check in Synapse.

The right fix should be to add a omitempty annotation so the JSON marshaller omits it if not provided in the struct instance.

[babolivier]

I've opened matrix-org/gomatrixserverlib#259 to fix it on Dendrite/gomatrixserverlib. Since this issue isn't related to Synapse (and the gomatrixserverlib PR has been merged while I was writing this), I'm closing this issue. Thanks for reporting it!