From 69f2ff7c82abe0731a05febde88098f4cd34ab8d Mon Sep 17 00:00:00 2001 From: Till <2353100+S7evinK@users.noreply.github.com> Date: Mon, 11 Apr 2022 09:05:23 +0200 Subject: [PATCH] Correctly use provided filters (#2339) * Apply filters correctly * Fix issues; Use prepareWithFilters * Update gmsl & tests * go.mod.. * PR comments --- go.mod | 3 +- go.sum | 7 +-- syncapi/routing/context.go | 4 +- .../postgres/current_room_state_table.go | 5 +- syncapi/storage/postgres/filtering.go | 32 ++++++++++-- .../postgres/output_room_events_table.go | 26 ++++++---- syncapi/storage/sqlite3/account_data_table.go | 39 +++++---------- syncapi/storage/sqlite3/filtering.go | 50 ++++++++++++------- syncapi/streams/stream_pdu.go | 6 ++- sytest-whitelist | 4 +- 10 files changed, 109 insertions(+), 67 deletions(-) diff --git a/go.mod b/go.mod index f4ac8d123..070f2b5b9 100644 --- a/go.mod +++ b/go.mod @@ -38,7 +38,7 @@ require ( github.com/matrix-org/go-http-js-libp2p v0.0.0-20200518170932-783164aeeda4 github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 - github.com/matrix-org/gomatrixserverlib v0.0.0-20220405134050-301e340659d5 + github.com/matrix-org/gomatrixserverlib v0.0.0-20220408160933-cf558306b56f github.com/matrix-org/pinecone v0.0.0-20220408153826-2999ea29ed48 github.com/matrix-org/util v0.0.0-20200807132607-55161520e1d4 github.com/mattn/go-sqlite3 v1.14.10 @@ -64,6 +64,7 @@ require ( golang.org/x/image v0.0.0-20220321031419-a8550c1d254a golang.org/x/mobile v0.0.0-20220407111146-e579adbbc4a2 golang.org/x/net v0.0.0-20220407224826-aac1ed45d8e3 + golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12 // indirect golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 gopkg.in/h2non/bimg.v1 v1.1.9 gopkg.in/yaml.v2 v2.4.0 diff --git a/go.sum b/go.sum index 063365168..464f774be 100644 --- a/go.sum +++ b/go.sum @@ -1112,8 +1112,8 @@ github.com/matrix-org/go-sqlite3-js v0.0.0-20210709140738-b0d1ba599a6d/go.mod h1 github.com/matrix-org/gomatrix v0.0.0-20190528120928-7df988a63f26/go.mod h1:3fxX6gUjWyI/2Bt7J1OLhpCzOfO/bB3AiX0cJtEKud0= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16 h1:ZtO5uywdd5dLDCud4r0r55eP4j9FuUNpl60Gmntcop4= github.com/matrix-org/gomatrix v0.0.0-20210324163249-be2af5ef2e16/go.mod h1:/gBX06Kw0exX1HrwmoBibFA98yBk/jxKpGVeyQbff+s= -github.com/matrix-org/gomatrixserverlib v0.0.0-20220405134050-301e340659d5 h1:Fkennny7+Z/5pygrhjFMZbz1j++P2hhhLoT7NO3p8DQ= -github.com/matrix-org/gomatrixserverlib v0.0.0-20220405134050-301e340659d5/go.mod h1:V5eO8rn/C3rcxig37A/BCeKerLFS+9Avg/77FIeTZ48= +github.com/matrix-org/gomatrixserverlib v0.0.0-20220408160933-cf558306b56f h1:MZrl4TgTnlaOn2Cu9gJCoJ3oyW5mT4/3QIZGgZXzKl4= +github.com/matrix-org/gomatrixserverlib v0.0.0-20220408160933-cf558306b56f/go.mod h1:V5eO8rn/C3rcxig37A/BCeKerLFS+9Avg/77FIeTZ48= github.com/matrix-org/pinecone v0.0.0-20220408153826-2999ea29ed48 h1:W0sjjC6yjskHX4mb0nk3p0fXAlbU5bAFUFeEtlrPASE= github.com/matrix-org/pinecone v0.0.0-20220408153826-2999ea29ed48/go.mod h1:ulJzsVOTssIVp1j/m5eI//4VpAGDkMt5NrRuAVX7wpc= github.com/matrix-org/util v0.0.0-20190711121626-527ce5ddefc7/go.mod h1:vVQlW/emklohkZnOPwD3LrZUBqdfsbiyO3p1lNV8F6U= @@ -1977,8 +1977,9 @@ golang.org/x/sys v0.0.0-20211116061358-0a5406a5449c/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20211216021012-1d35b9e2eb4e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220111092808-5a964db01320/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220114195835-da31bd327af9/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.0.0-20220405052023-b1e9470b6e64 h1:D1v9ucDTYBtbz5vNuBbAhIMAGhQhJ6Ym5ah3maMVNX4= golang.org/x/sys v0.0.0-20220405052023-b1e9470b6e64/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= +golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12 h1:QyVthZKMsyaQwBTJE04jdNN0Pp5Fn9Qga0mrgxyERQM= +golang.org/x/sys v0.0.0-20220406163625-3f8b81556e12/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210220032956-6a3ed077a48d/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= diff --git a/syncapi/routing/context.go b/syncapi/routing/context.go index 2412bc2ae..aaa0c61bf 100644 --- a/syncapi/routing/context.go +++ b/syncapi/routing/context.go @@ -60,7 +60,9 @@ func Context( Headers: nil, } } - filter.Rooms = append(filter.Rooms, roomID) + if filter.Rooms != nil { + *filter.Rooms = append(*filter.Rooms, roomID) + } ctx := req.Context() membershipRes := roomserver.QueryMembershipForUserResponse{} diff --git a/syncapi/storage/postgres/current_room_state_table.go b/syncapi/storage/postgres/current_room_state_table.go index 69e6e30ec..fe68788d1 100644 --- a/syncapi/storage/postgres/current_room_state_table.go +++ b/syncapi/storage/postgres/current_room_state_table.go @@ -233,9 +233,10 @@ func (s *currentRoomStateStatements) SelectCurrentState( excludeEventIDs []string, ) ([]*gomatrixserverlib.HeaderedEvent, error) { stmt := sqlutil.TxStmt(txn, s.selectCurrentStateStmt) + senders, notSenders := getSendersStateFilterFilter(stateFilter) rows, err := stmt.QueryContext(ctx, roomID, - pq.StringArray(stateFilter.Senders), - pq.StringArray(stateFilter.NotSenders), + pq.StringArray(senders), + pq.StringArray(notSenders), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)), stateFilter.ContainsURL, diff --git a/syncapi/storage/postgres/filtering.go b/syncapi/storage/postgres/filtering.go index dcc421362..a2ca42156 100644 --- a/syncapi/storage/postgres/filtering.go +++ b/syncapi/storage/postgres/filtering.go @@ -16,21 +16,45 @@ package postgres import ( "strings" + + "github.com/matrix-org/gomatrixserverlib" ) // filterConvertWildcardToSQL converts wildcards as defined in // https://matrix.org/docs/spec/client_server/r0.3.0.html#post-matrix-client-r0-user-userid-filter // to SQL wildcards that can be used with LIKE() -func filterConvertTypeWildcardToSQL(values []string) []string { +func filterConvertTypeWildcardToSQL(values *[]string) []string { if values == nil { // Return nil instead of []string{} so IS NULL can work correctly when // the return value is passed into SQL queries return nil } - ret := make([]string, len(values)) - for i := range values { - ret[i] = strings.Replace(values[i], "*", "%", -1) + v := *values + ret := make([]string, len(v)) + for i := range v { + ret[i] = strings.Replace(v[i], "*", "%", -1) } return ret } + +// TODO: Replace when Dendrite uses Go 1.18 +func getSendersRoomEventFilter(filter *gomatrixserverlib.RoomEventFilter) (senders []string, notSenders []string) { + if filter.Senders != nil { + senders = *filter.Senders + } + if filter.NotSenders != nil { + notSenders = *filter.NotSenders + } + return senders, notSenders +} + +func getSendersStateFilterFilter(filter *gomatrixserverlib.StateFilter) (senders []string, notSenders []string) { + if filter.Senders != nil { + senders = *filter.Senders + } + if filter.NotSenders != nil { + notSenders = *filter.NotSenders + } + return senders, notSenders +} diff --git a/syncapi/storage/postgres/output_room_events_table.go b/syncapi/storage/postgres/output_room_events_table.go index a30e220ba..269cd4494 100644 --- a/syncapi/storage/postgres/output_room_events_table.go +++ b/syncapi/storage/postgres/output_room_events_table.go @@ -204,11 +204,11 @@ func (s *outputRoomEventsStatements) SelectStateInRange( stateFilter *gomatrixserverlib.StateFilter, roomIDs []string, ) (map[string]map[string]bool, map[string]types.StreamEvent, error) { stmt := sqlutil.TxStmt(txn, s.selectStateInRangeStmt) - + senders, notSenders := getSendersStateFilterFilter(stateFilter) rows, err := stmt.QueryContext( ctx, r.Low(), r.High(), pq.StringArray(roomIDs), - pq.StringArray(stateFilter.Senders), - pq.StringArray(stateFilter.NotSenders), + pq.StringArray(senders), + pq.StringArray(notSenders), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)), stateFilter.ContainsURL, @@ -353,10 +353,11 @@ func (s *outputRoomEventsStatements) SelectRecentEvents( } else { stmt = sqlutil.TxStmt(txn, s.selectRecentEventsStmt) } + senders, notSenders := getSendersRoomEventFilter(eventFilter) rows, err := stmt.QueryContext( ctx, roomID, r.Low(), r.High(), - pq.StringArray(eventFilter.Senders), - pq.StringArray(eventFilter.NotSenders), + pq.StringArray(senders), + pq.StringArray(notSenders), pq.StringArray(filterConvertTypeWildcardToSQL(eventFilter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(eventFilter.NotTypes)), eventFilter.Limit+1, @@ -398,11 +399,12 @@ func (s *outputRoomEventsStatements) SelectEarlyEvents( ctx context.Context, txn *sql.Tx, roomID string, r types.Range, eventFilter *gomatrixserverlib.RoomEventFilter, ) ([]types.StreamEvent, error) { + senders, notSenders := getSendersRoomEventFilter(eventFilter) stmt := sqlutil.TxStmt(txn, s.selectEarlyEventsStmt) rows, err := stmt.QueryContext( ctx, roomID, r.Low(), r.High(), - pq.StringArray(eventFilter.Senders), - pq.StringArray(eventFilter.NotSenders), + pq.StringArray(senders), + pq.StringArray(notSenders), pq.StringArray(filterConvertTypeWildcardToSQL(eventFilter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(eventFilter.NotTypes)), eventFilter.Limit, @@ -480,10 +482,11 @@ func (s *outputRoomEventsStatements) SelectContextEvent(ctx context.Context, txn func (s *outputRoomEventsStatements) SelectContextBeforeEvent( ctx context.Context, txn *sql.Tx, id int, roomID string, filter *gomatrixserverlib.RoomEventFilter, ) (evts []*gomatrixserverlib.HeaderedEvent, err error) { + senders, notSenders := getSendersRoomEventFilter(filter) rows, err := sqlutil.TxStmt(txn, s.selectContextBeforeEventStmt).QueryContext( ctx, roomID, id, filter.Limit, - pq.StringArray(filter.Senders), - pq.StringArray(filter.NotSenders), + pq.StringArray(senders), + pq.StringArray(notSenders), pq.StringArray(filterConvertTypeWildcardToSQL(filter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(filter.NotTypes)), ) @@ -512,10 +515,11 @@ func (s *outputRoomEventsStatements) SelectContextBeforeEvent( func (s *outputRoomEventsStatements) SelectContextAfterEvent( ctx context.Context, txn *sql.Tx, id int, roomID string, filter *gomatrixserverlib.RoomEventFilter, ) (lastID int, evts []*gomatrixserverlib.HeaderedEvent, err error) { + senders, notSenders := getSendersRoomEventFilter(filter) rows, err := sqlutil.TxStmt(txn, s.selectContextAfterEventStmt).QueryContext( ctx, roomID, id, filter.Limit, - pq.StringArray(filter.Senders), - pq.StringArray(filter.NotSenders), + pq.StringArray(senders), + pq.StringArray(notSenders), pq.StringArray(filterConvertTypeWildcardToSQL(filter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(filter.NotTypes)), ) diff --git a/syncapi/storage/sqlite3/account_data_table.go b/syncapi/storage/sqlite3/account_data_table.go index 5b2287e6d..b0aeb70f2 100644 --- a/syncapi/storage/sqlite3/account_data_table.go +++ b/syncapi/storage/sqlite3/account_data_table.go @@ -41,10 +41,10 @@ const insertAccountDataSQL = "" + " ON CONFLICT (user_id, room_id, type) DO UPDATE" + " SET id = $5" +// further parameters are added by prepareWithFilters const selectAccountDataInRangeSQL = "" + "SELECT room_id, type FROM syncapi_account_data_type" + - " WHERE user_id = $1 AND id > $2 AND id <= $3" + - " ORDER BY id ASC" + " WHERE user_id = $1 AND id > $2 AND id <= $3" const selectMaxAccountDataIDSQL = "" + "SELECT MAX(id) FROM syncapi_account_data_type" @@ -94,18 +94,25 @@ func (s *accountDataStatements) SelectAccountDataInRange( ctx context.Context, userID string, r types.Range, - accountDataFilterPart *gomatrixserverlib.EventFilter, + filter *gomatrixserverlib.EventFilter, ) (data map[string][]string, err error) { data = make(map[string][]string) + stmt, params, err := prepareWithFilters( + s.db, nil, selectAccountDataInRangeSQL, + []interface{}{ + userID, r.Low(), r.High(), + }, + filter.Senders, filter.NotSenders, + filter.Types, filter.NotTypes, + []string{}, filter.Limit, FilterOrderAsc, + ) - rows, err := s.selectAccountDataInRangeStmt.QueryContext(ctx, userID, r.Low(), r.High()) + rows, err := stmt.QueryContext(ctx, params...) if err != nil { return } defer internal.CloseAndLogIfError(ctx, rows, "selectAccountDataInRange: rows.close() failed") - var entries int - for rows.Next() { var dataType string var roomID string @@ -114,31 +121,11 @@ func (s *accountDataStatements) SelectAccountDataInRange( return } - // check if we should add this by looking at the filter. - // It would be nice if we could do this in SQL-land, but the mix of variadic - // and positional parameters makes the query annoyingly hard to do, it's easier - // and clearer to do it in Go-land. If there are no filters for [not]types then - // this gets skipped. - for _, includeType := range accountDataFilterPart.Types { - if includeType != dataType { // TODO: wildcard support - continue - } - } - for _, excludeType := range accountDataFilterPart.NotTypes { - if excludeType == dataType { // TODO: wildcard support - continue - } - } - if len(data[roomID]) > 0 { data[roomID] = append(data[roomID], dataType) } else { data[roomID] = []string{dataType} } - entries++ - if entries >= accountDataFilterPart.Limit { - break - } } return data, nil diff --git a/syncapi/storage/sqlite3/filtering.go b/syncapi/storage/sqlite3/filtering.go index 11f3e647b..54b12ddf8 100644 --- a/syncapi/storage/sqlite3/filtering.go +++ b/syncapi/storage/sqlite3/filtering.go @@ -25,32 +25,48 @@ const ( // parts. func prepareWithFilters( db *sql.DB, txn *sql.Tx, query string, params []interface{}, - senders, notsenders, types, nottypes []string, excludeEventIDs []string, + senders, notsenders, types, nottypes *[]string, excludeEventIDs []string, limit int, order FilterOrder, ) (*sql.Stmt, []interface{}, error) { offset := len(params) - if count := len(senders); count > 0 { - query += " AND sender IN " + sqlutil.QueryVariadicOffset(count, offset) - for _, v := range senders { - params, offset = append(params, v), offset+1 + if senders != nil { + if count := len(*senders); count > 0 { + query += " AND sender IN " + sqlutil.QueryVariadicOffset(count, offset) + for _, v := range *senders { + params, offset = append(params, v), offset+1 + } + } else { + query += ` AND sender = ""` } } - if count := len(notsenders); count > 0 { - query += " AND sender NOT IN " + sqlutil.QueryVariadicOffset(count, offset) - for _, v := range notsenders { - params, offset = append(params, v), offset+1 + if notsenders != nil { + if count := len(*notsenders); count > 0 { + query += " AND sender NOT IN " + sqlutil.QueryVariadicOffset(count, offset) + for _, v := range *notsenders { + params, offset = append(params, v), offset+1 + } + } else { + query += ` AND sender NOT = ""` } } - if count := len(types); count > 0 { - query += " AND type IN " + sqlutil.QueryVariadicOffset(count, offset) - for _, v := range types { - params, offset = append(params, v), offset+1 + if types != nil { + if count := len(*types); count > 0 { + query += " AND type IN " + sqlutil.QueryVariadicOffset(count, offset) + for _, v := range *types { + params, offset = append(params, v), offset+1 + } + } else { + query += ` AND type = ""` } } - if count := len(nottypes); count > 0 { - query += " AND type NOT IN " + sqlutil.QueryVariadicOffset(count, offset) - for _, v := range nottypes { - params, offset = append(params, v), offset+1 + if nottypes != nil { + if count := len(*nottypes); count > 0 { + query += " AND type NOT IN " + sqlutil.QueryVariadicOffset(count, offset) + for _, v := range *nottypes { + params, offset = append(params, v), offset+1 + } + } else { + query += ` AND type NOT = ""` } } if count := len(excludeEventIDs); count > 0 { diff --git a/syncapi/streams/stream_pdu.go b/syncapi/streams/stream_pdu.go index ab200e007..bcaf6ca31 100644 --- a/syncapi/streams/stream_pdu.go +++ b/syncapi/streams/stream_pdu.go @@ -423,8 +423,12 @@ func (p *PDUStreamProvider) addIgnoredUsersToFilter(ctx context.Context, req *ty return err } req.IgnoredUsers = *ignores + userList := make([]string, 0, len(ignores.List)) for userID := range ignores.List { - eventFilter.NotSenders = append(eventFilter.NotSenders, userID) + userList = append(userList, userID) + } + if len(userList) > 0 { + eventFilter.NotSenders = &userList } return nil } diff --git a/sytest-whitelist b/sytest-whitelist index dc67c9935..a7aea05ed 100644 --- a/sytest-whitelist +++ b/sytest-whitelist @@ -696,4 +696,6 @@ Room state after a rejected message event is the same as before Room state after a rejected state event is the same as before Ignore user in existing room Ignore invite in full sync -Ignore invite in incremental sync \ No newline at end of file +Ignore invite in incremental sync +A filtered timeline reaches its limit +A change to displayname should not result in a full state sync \ No newline at end of file