From 68cb87be6682c04977995443e3a55d6896ae9a0b Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 7 Sep 2016 14:46:32 +0100 Subject: [PATCH 1/2] Do not process /sync responses for rooms the bot has just joined Else we can process !commands multiple times and/or process !commands from before we were in the room (if `history_visibility` allows it). --- .../matrix-org/go-neb/matrix/matrix.go | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/src/github.com/matrix-org/go-neb/matrix/matrix.go b/src/github.com/matrix-org/go-neb/matrix/matrix.go index 7d85fb9..c197d57 100644 --- a/src/github.com/matrix-org/go-neb/matrix/matrix.go +++ b/src/github.com/matrix-org/go-neb/matrix/matrix.go @@ -252,9 +252,7 @@ func (cli *Client) Sync() { return } - isFirstSync := nextToken == "" - - // Update client state + processResponse := cli.shouldProcessResponse(nextToken, &syncResponse) nextToken = syncResponse.NextBatch logger.WithField("next_batch", nextToken).Print("Received sync response") @@ -263,12 +261,54 @@ func (cli *Client) Sync() { // a malformed/buggy event which keeps making us panic. cli.NextBatchStorer.Save(cli.UserID, nextToken) - if !isFirstSync { + if processResponse { + // Update client state channel <- syncResponse } } } +// shouldProcessResponse returns true if the response should be processed. May modify the response to remove +// stuff that shouldn't be processed. +func (cli *Client) shouldProcessResponse(tokenOnSync string, syncResponse *syncHTTPResponse) bool { + if tokenOnSync == "" { + return false + } + // This is a horrible hack because /sync will return the most recent messages for a room + // as soon as you /join it. We do NOT want to process those events in that particular room + // because they may have already been processed (if you toggle the bot in/out of the room). + // + // Work around this by inspecting each room's timeline and seeing if an m.room.member event for us + // exists and is "join" and then discard processing that room entirely if so. + for roomID, roomData := range syncResponse.Rooms.Join { + for i := len(roomData.Timeline.Events) - 1; i >= 0; i-- { + e := roomData.Timeline.Events[i] + if e.Type == "m.room.member" && e.StateKey == cli.UserID { + m := e.Content["membership"] + mship, ok := m.(string) + if !ok { + continue + } + if mship == "join" { + log.WithFields(log.Fields{ + "room_id": roomID, + "user_id": cli.UserID, + "start_token": tokenOnSync, + }).Info("Discarding /sync events in room: just joined it.") + _, ok := syncResponse.Rooms.Join[roomID] + if !ok { + panic("room " + roomID + " does not exist in Join?!") + } + delete(syncResponse.Rooms.Join, roomID) // don't re-process !commands + delete(syncResponse.Rooms.Invite, roomID) // don't re-process invites + break + } + } + } + } + return true +} + func (cli *Client) incrementSyncingID() uint32 { cli.syncingMutex.Lock() defer cli.syncingMutex.Unlock() From ce2af7ed71e9e2300cf8d3d910031dada81a8fd8 Mon Sep 17 00:00:00 2001 From: Kegan Dougal Date: Wed, 7 Sep 2016 15:29:31 +0100 Subject: [PATCH 2/2] Add TODO marker --- src/github.com/matrix-org/go-neb/matrix/matrix.go | 1 + 1 file changed, 1 insertion(+) diff --git a/src/github.com/matrix-org/go-neb/matrix/matrix.go b/src/github.com/matrix-org/go-neb/matrix/matrix.go index c197d57..d2e318b 100644 --- a/src/github.com/matrix-org/go-neb/matrix/matrix.go +++ b/src/github.com/matrix-org/go-neb/matrix/matrix.go @@ -280,6 +280,7 @@ func (cli *Client) shouldProcessResponse(tokenOnSync string, syncResponse *syncH // // Work around this by inspecting each room's timeline and seeing if an m.room.member event for us // exists and is "join" and then discard processing that room entirely if so. + // TODO: We probably want to process the !commands from after the last join event in the timeline. for roomID, roomData := range syncResponse.Rooms.Join { for i := len(roomData.Timeline.Events) - 1; i >= 0; i-- { e := roomData.Timeline.Events[i]