Merge pull request #423 from matrix-org/kegan/msc4115

Remove MSC4115 unsigned.membership field

sync2/handler2/handler.go

@@ -21,6 +21,7 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/rs/zerolog"
 	"github.com/tidwall/gjson"
+	"github.com/tidwall/sjson"
 )
 
 var logger = zerolog.New(os.Stdout).With().Timestamp().Logger().Output(zerolog.ConsoleWriter{
@@ -270,8 +271,10 @@ func (h *Handler) Accumulate(ctx context.Context, userID, deviceID, roomID strin
 	// Also remember events which were sent by this user but lack a transaction ID.
 	eventIDsLackingTxns := make([]string, 0, len(timeline.Events))
 
-	for _, e := range timeline.Events {
-		parsed := gjson.ParseBytes(e)
+	for i := range timeline.Events {
+		// Delete MSC4115 field as it isn't accurate when we reuse the same event for >1 user
+		timeline.Events[i], _ = sjson.DeleteBytes(timeline.Events[i], "unsigned.membership")
+		parsed := gjson.ParseBytes(timeline.Events[i])
 		eventID := parsed.Get("event_id").Str
 
 		if txnID := parsed.Get("unsigned.transaction_id"); txnID.Exists() {
@@ -371,6 +374,9 @@ func (h *Handler) Accumulate(ctx context.Context, userID, deviceID, roomID strin
 }
 
 func (h *Handler) Initialise(ctx context.Context, roomID string, state []json.RawMessage) error {
+	for i := range state { // Delete MSC4115 field as it isn't accurate when we reuse the same event for >1 user
+		state[i], _ = sjson.DeleteBytes(state[i], "unsigned.membership")
+	}
 	res, err := h.Store.Initialise(roomID, state)
 	if err != nil {
 		logger.Err(err).Int("state", len(state)).Str("room", roomID).Msg("V2: failed to initialise room")

tests-integration/lists_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/matrix-org/sliding-sync/sync3"
 	"github.com/matrix-org/sliding-sync/testutils"
 	"github.com/matrix-org/sliding-sync/testutils/m"
+	"github.com/tidwall/sjson"
 )
 
 func TestListsAsKeys(t *testing.T) {
@@ -361,3 +362,89 @@ func TestBumpEventTypesOnStartup(t *testing.T) {
 		))
 	}
 }
+
+func TestDeleteMSC4115Field(t *testing.T) {
+	rig := NewTestRig(t)
+	defer rig.Finish()
+	roomID := "!TestDeleteMSC4115Field:localhost"
+	rig.SetupV2RoomsForUser(t, alice, NoFlush, map[string]RoomDescriptor{
+		roomID: {},
+	})
+	aliceToken := rig.Token(alice)
+	res := rig.V3.mustDoV3Request(t, aliceToken, sync3.Request{
+		Lists: map[string]sync3.RequestList{
+			"a": {
+				Ranges: sync3.SliceRanges{{0, 20}},
+				RoomSubscription: sync3.RoomSubscription{
+					TimelineLimit: 10,
+					RequiredState: [][2]string{{"m.room.name", "*"}},
+				},
+			},
+		},
+	})
+	m.MatchResponse(t, res, m.MatchLists(map[string][]m.ListMatcher{
+		"a": {
+			m.MatchV3Count(1),
+		},
+	}), m.MatchRoomSubscription(roomID))
+
+	// ensure live events remove the field.
+	liveEvent := testutils.NewMessageEvent(t, alice, "live event", testutils.WithUnsigned(map[string]interface{}{
+		"membership": "join",
+	}))
+	liveEventWithoutMembership := make(json.RawMessage, len(liveEvent))
+	copy(liveEventWithoutMembership, liveEvent)
+	liveEventWithoutMembership, err := sjson.DeleteBytes(liveEventWithoutMembership, "unsigned.membership")
+	if err != nil {
+		t.Fatalf("failed to delete unsigned.membership field")
+	}
+	rig.FlushEvent(t, alice, roomID, liveEvent)
+
+	res = rig.V3.mustDoV3RequestWithPos(t, aliceToken, res.Pos, sync3.Request{})
+	m.MatchResponse(t, res, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		roomID: {
+			m.MatchRoomTimelineMostRecent(1, []json.RawMessage{liveEventWithoutMembership}),
+		},
+	}))
+
+	// ensure state events remove the field.
+	stateEvent := testutils.NewStateEvent(t, "m.room.name", "", alice, map[string]interface{}{
+		"name": "Room Name",
+	}, testutils.WithUnsigned(map[string]interface{}{
+		"membership": "join",
+	}))
+	stateEventWithoutMembership := make(json.RawMessage, len(stateEvent))
+	copy(stateEventWithoutMembership, stateEvent)
+	stateEventWithoutMembership, err = sjson.DeleteBytes(stateEventWithoutMembership, "unsigned.membership")
+	if err != nil {
+		t.Fatalf("failed to delete unsigned.membership field")
+	}
+	rig.V2.queueResponse(alice, sync2.SyncResponse{
+		Rooms: sync2.SyncRoomsResponse{
+			Join: v2JoinTimeline(roomEvents{
+				roomID: roomID,
+				state:  []json.RawMessage{stateEvent},
+				events: []json.RawMessage{testutils.NewMessageEvent(t, alice, "dummy")},
+			}),
+		},
+	})
+	rig.V2.waitUntilEmpty(t, alice)
+
+	// sending v2 state invalidates the SS connection so start again pre-emptively.
+	res = rig.V3.mustDoV3Request(t, aliceToken, sync3.Request{
+		Lists: map[string]sync3.RequestList{
+			"a": {
+				Ranges: sync3.SliceRanges{{0, 20}},
+				RoomSubscription: sync3.RoomSubscription{
+					TimelineLimit: 10,
+					RequiredState: [][2]string{{"m.room.name", "*"}},
+				},
+			},
+		},
+	})
+	m.MatchResponse(t, res, m.MatchRoomSubscriptionsStrict(map[string][]m.RoomMatcher{
+		roomID: {
+			m.MatchRoomRequiredState([]json.RawMessage{stateEventWithoutMembership}),
+		},
+	}))
+}