Skip to content

Commit

Permalink
Remove MSC4115 unsigned.membership field
Browse files Browse the repository at this point in the history
With integration tests. This is required because this field
is scoped per-v2-user, but because the proxy deduplicates events
it means we might see the membership value for a different user.
We can't set this field correctly as we lack that information, so
rather than lie to clients instead just delete the field.

Fixes #421
  • Loading branch information
kegsay committed Apr 24, 2024
1 parent c18961b commit c4f2617
Show file tree
Hide file tree
Showing 2 changed files with 95 additions and 2 deletions.
10 changes: 8 additions & 2 deletions sync2/handler2/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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")
Expand Down
87 changes: 87 additions & 0 deletions tests-integration/lists_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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}),
},
}))
}

0 comments on commit c4f2617

Please sign in to comment.