bugfix: ensure global metadata is consistently made correctly

On startup it was possible for it to miss ChildSpaceRooms which
would then cause a nil panic. With regression test.

internal/roomname.go

@@ -25,6 +25,13 @@ type RoomMetadata struct {
 	TypingEvent json.RawMessage
 }
 
+func NewRoomMetadata(roomID string) *RoomMetadata {
+	return &RoomMetadata{
+		RoomID:          roomID,
+		ChildSpaceRooms: make(map[string]struct{}),
+	}
+}
+
 // SameRoomName checks if the fields relevant for room names have changed between the two metadatas.
 // Returns true if there are no changes.
 func (m *RoomMetadata) SameRoomName(other *RoomMetadata) bool {

state/storage.go

@@ -4,10 +4,11 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/getsentry/sentry-go"
 	"os"
 	"strings"
 
+	"github.com/getsentry/sentry-go"
+
 	"github.com/jmoiron/sqlx"
 	"github.com/lib/pq"
 	"github.com/matrix-org/sliding-sync/internal"
@@ -173,7 +174,7 @@ func (s *Storage) MetadataForAllRooms(txn *sqlx.Tx, result map[string]internal.R
 		metadata := result[ev.RoomID]
 		metadata.LastMessageTimestamp = gjson.ParseBytes(ev.JSON).Get("origin_server_ts").Uint()
 		// it's possible the latest event is a brand new room not caught by the first SELECT for joined
-		// rooms e.g when you're invited to a room so we need to make sure to se the metadata again here
+		// rooms e.g when you're invited to a room so we need to make sure to set the metadata again here
 		metadata.RoomID = ev.RoomID
 		result[ev.RoomID] = metadata
 	}
@@ -766,9 +767,9 @@ func (s *Storage) AllJoinedMembers(txn *sqlx.Tx) (result map[string][]string, me
 	}
 	metadata = make(map[string]internal.RoomMetadata)
 	for roomID, joinedMembers := range result {
-		metadata[roomID] = internal.RoomMetadata{
-			JoinCount: len(joinedMembers),
-		}
+		m := internal.NewRoomMetadata(roomID)
+		m.JoinCount = len(joinedMembers)
+		metadata[roomID] = *m
 	}
 	return result, metadata, nil
 }

sync3/caches/global.go

@@ -208,10 +208,7 @@ func (c *GlobalCache) OnEphemeralEvent(ctx context.Context, roomID string, ephEv
 	defer c.roomIDToMetadataMu.Unlock()
 	metadata := c.roomIDToMetadata[roomID]
 	if metadata == nil {
-		metadata = &internal.RoomMetadata{
-			RoomID:          roomID,
-			ChildSpaceRooms: make(map[string]struct{}),
-		}
+		metadata = internal.NewRoomMetadata(roomID)
 	}
 
 	switch evType {
@@ -233,10 +230,7 @@ func (c *GlobalCache) OnNewEvent(
 	defer c.roomIDToMetadataMu.Unlock()
 	metadata := c.roomIDToMetadata[ed.RoomID]
 	if metadata == nil {
-		metadata = &internal.RoomMetadata{
-			RoomID:          ed.RoomID,
-			ChildSpaceRooms: make(map[string]struct{}),
-		}
+		metadata = internal.NewRoomMetadata(ed.RoomID)
 	}
 	switch ed.EventType {
 	case "m.room.name":

sync3/caches/user.go

@@ -164,17 +164,16 @@ func (i *InviteData) RoomMetadata() *internal.RoomMetadata {
 	if i.RoomType != "" {
 		roomType = &i.RoomType
 	}
-	return &internal.RoomMetadata{
-		RoomID:               i.roomID,
-		Heroes:               i.Heroes,
-		NameEvent:            i.NameEvent,
-		CanonicalAlias:       i.CanonicalAlias,
-		InviteCount:          1,
-		JoinCount:            1,
-		LastMessageTimestamp: i.LastMessageTimestamp,
-		Encrypted:            i.Encrypted,
-		RoomType:             roomType,
-	}
+	metadata := internal.NewRoomMetadata(i.roomID)
+	metadata.Heroes = i.Heroes
+	metadata.NameEvent = i.NameEvent
+	metadata.CanonicalAlias = i.CanonicalAlias
+	metadata.InviteCount = 1
+	metadata.JoinCount = 1
+	metadata.LastMessageTimestamp = i.LastMessageTimestamp
+	metadata.Encrypted = i.Encrypted
+	metadata.RoomType = roomType
+	return metadata
 }
 
 type UserCacheListener interface {
@@ -423,9 +422,7 @@ func (c *UserCache) newRoomUpdate(ctx context.Context, roomID string) RoomUpdate
 		// this can happen when we join a room we didn't know about because we process unread counts
 		// before the timeline events. Warn and send a stub
 		logger.Warn().Str("room", roomID).Msg("UserCache update: room doesn't exist in global cache yet, generating stub")
-		r = &internal.RoomMetadata{
-			RoomID: roomID,
-		}
+		r = internal.NewRoomMetadata(roomID)
 	} else {
 		r = globalRooms[roomID]
 	}
@@ -671,10 +668,8 @@ func (c *UserCache) OnLeftRoom(ctx context.Context, roomID string) {
 			roomID: roomID,
 			// do NOT pull from the global cache as it is a snapshot of the room at the point of
 			// the invite: don't leak additional data!!!
-			globalRoomData: &internal.RoomMetadata{
-				RoomID: roomID,
-			},
-			userRoomData: &urd,
+			globalRoomData: internal.NewRoomMetadata(roomID),
+			userRoomData:   &urd,
 		},
 	}
 	c.emitOnRoomUpdate(ctx, up)

tests-integration/space_test.go

@@ -0,0 +1,66 @@
+package syncv3
+
+import (
+	"encoding/json"
+	"testing"
+	"time"
+
+	"github.com/matrix-org/sliding-sync/sync2"
+	"github.com/matrix-org/sliding-sync/sync3"
+	"github.com/matrix-org/sliding-sync/testutils"
+	"github.com/matrix-org/sliding-sync/testutils/m"
+)
+
+// Regression test for a panic in the wild when we tried to write to internal.RoomMetadata.ChildSpaceRooms and the map didn't exist.
+func TestBecomingASpaceDoesntCrash(t *testing.T) {
+	pqString := testutils.PrepareDBConnectionString()
+	v2 := runTestV2Server(t)
+	v3 := runTestServer(t, v2, pqString)
+	defer v2.close()
+	defer v3.close()
+	roomID := "!foo:bar"
+	v2.addAccount(alice, aliceToken)
+	v2.queueResponse(alice, sync2.SyncResponse{
+		Rooms: sync2.SyncRoomsResponse{
+			Join: v2JoinTimeline(roomEvents{
+				roomID: roomID,
+				events: createRoomState(t, alice, time.Now()),
+			}),
+		},
+	})
+	// let the proxy store the room
+	v3.mustDoV3Request(t, aliceToken, sync3.Request{})
+	// restart the proxy: at this point we may have a nil ChildSpaceRooms map
+	v3.restart(t, v2, pqString)
+
+	// check it by injecting a space child
+	spaceChildEvent := testutils.NewStateEvent(t, "m.space.child", "!somewhere:else", alice, map[string]interface{}{
+		"via": []string{"example.com"},
+	})
+	// TODO: we inject bob here because alice's sync stream seems to discard this response post-restart for unknown reasons
+	v2.addAccount(bob, bobToken)
+	v2.queueResponse(bob, sync2.SyncResponse{
+		Rooms: sync2.SyncRoomsResponse{
+			Join: v2JoinTimeline(roomEvents{
+				roomID: roomID,
+				events: []json.RawMessage{
+					spaceChildEvent,
+				},
+			}),
+		},
+	})
+
+	// we should be able to request the room without crashing
+	v3.mustDoV3Request(t, bobToken, sync3.Request{})
+	// we should see the data
+	res := v3.mustDoV3Request(t, aliceToken, sync3.Request{
+		RoomSubscriptions: map[string]sync3.RoomSubscription{
+			roomID: {TimelineLimit: 1},
+		},
+	})
+	m.MatchResponse(t, res, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		roomID: {
+			m.MatchRoomTimeline([]json.RawMessage{spaceChildEvent}),
+		},
+	}))
+}