Don't limit "state" (#2849)

This is apparently some incorrect behaviour that we built as a result of
a spec bug (matrix-org/matrix-spec#1314) where we were applying a filter
to the `"state"` section of the `/sync` response incorrectly. The client
then has no way to know that the state was limited.

This PR removes the state limiting, which probably also helps #2842.
This commit is contained in:
Neil Alexander 2022-11-02 09:34:19 +00:00 committed by GitHub
parent 8a1904ffe5
commit 3db9e98456
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 15 additions and 21 deletions

View File

@ -93,7 +93,6 @@ func Context(
} }
stateFilter := gomatrixserverlib.StateFilter{ stateFilter := gomatrixserverlib.StateFilter{
Limit: 100,
NotSenders: filter.NotSenders, NotSenders: filter.NotSenders,
NotTypes: filter.NotTypes, NotTypes: filter.NotTypes,
Senders: filter.Senders, Senders: filter.Senders,

View File

@ -294,7 +294,7 @@ type SearchRequest struct {
BeforeLimit int `json:"before_limit,omitempty"` BeforeLimit int `json:"before_limit,omitempty"`
IncludeProfile bool `json:"include_profile,omitempty"` IncludeProfile bool `json:"include_profile,omitempty"`
} `json:"event_context"` } `json:"event_context"`
Filter gomatrixserverlib.StateFilter `json:"filter"` Filter gomatrixserverlib.RoomEventFilter `json:"filter"`
Groupings struct { Groupings struct {
GroupBy []struct { GroupBy []struct {
Key string `json:"key"` Key string `json:"key"`

View File

@ -91,8 +91,7 @@ const selectCurrentStateSQL = "" +
" AND ( $4::text[] IS NULL OR type LIKE ANY($4) )" + " AND ( $4::text[] IS NULL OR type LIKE ANY($4) )" +
" AND ( $5::text[] IS NULL OR NOT(type LIKE ANY($5)) )" + " AND ( $5::text[] IS NULL OR NOT(type LIKE ANY($5)) )" +
" AND ( $6::bool IS NULL OR contains_url = $6 )" + " AND ( $6::bool IS NULL OR contains_url = $6 )" +
" AND (event_id = ANY($7)) IS NOT TRUE" + " AND (event_id = ANY($7)) IS NOT TRUE"
" LIMIT $8"
const selectJoinedUsersSQL = "" + const selectJoinedUsersSQL = "" +
"SELECT room_id, state_key FROM syncapi_current_room_state WHERE type = 'm.room.member' AND membership = 'join'" "SELECT room_id, state_key FROM syncapi_current_room_state WHERE type = 'm.room.member' AND membership = 'join'"
@ -290,7 +289,6 @@ func (s *currentRoomStateStatements) SelectCurrentState(
pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)),
stateFilter.ContainsURL, stateFilter.ContainsURL,
pq.StringArray(excludeEventIDs), pq.StringArray(excludeEventIDs),
stateFilter.Limit,
) )
if err != nil { if err != nil {
return nil, err return nil, err

View File

@ -144,8 +144,7 @@ const selectStateInRangeFilteredSQL = "" +
" AND ( $6::text[] IS NULL OR type LIKE ANY($6) )" + " AND ( $6::text[] IS NULL OR type LIKE ANY($6) )" +
" AND ( $7::text[] IS NULL OR NOT(type LIKE ANY($7)) )" + " AND ( $7::text[] IS NULL OR NOT(type LIKE ANY($7)) )" +
" AND ( $8::bool IS NULL OR contains_url = $8 )" + " AND ( $8::bool IS NULL OR contains_url = $8 )" +
" ORDER BY id ASC" + " ORDER BY id ASC"
" LIMIT $9"
// In order for us to apply the state updates correctly, rows need to be ordered in the order they were received (id). // In order for us to apply the state updates correctly, rows need to be ordered in the order they were received (id).
const selectStateInRangeSQL = "" + const selectStateInRangeSQL = "" +
@ -153,8 +152,7 @@ const selectStateInRangeSQL = "" +
" FROM syncapi_output_room_events" + " FROM syncapi_output_room_events" +
" WHERE (id > $1 AND id <= $2) AND (add_state_ids IS NOT NULL OR remove_state_ids IS NOT NULL)" + " WHERE (id > $1 AND id <= $2) AND (add_state_ids IS NOT NULL OR remove_state_ids IS NOT NULL)" +
" AND room_id = ANY($3)" + " AND room_id = ANY($3)" +
" ORDER BY id ASC" + " ORDER BY id ASC"
" LIMIT $4"
const deleteEventsForRoomSQL = "" + const deleteEventsForRoomSQL = "" +
"DELETE FROM syncapi_output_room_events WHERE room_id = $1" "DELETE FROM syncapi_output_room_events WHERE room_id = $1"
@ -264,13 +262,11 @@ func (s *outputRoomEventsStatements) SelectStateInRange(
pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.Types)), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.Types)),
pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)), pq.StringArray(filterConvertTypeWildcardToSQL(stateFilter.NotTypes)),
stateFilter.ContainsURL, stateFilter.ContainsURL,
stateFilter.Limit,
) )
} else { } else {
stmt := sqlutil.TxStmt(txn, s.selectStateInRangeStmt) stmt := sqlutil.TxStmt(txn, s.selectStateInRangeStmt)
rows, err = stmt.QueryContext( rows, err = stmt.QueryContext(
ctx, r.Low(), r.High(), pq.StringArray(roomIDs), ctx, r.Low(), r.High(), pq.StringArray(roomIDs),
r.High()-r.Low(),
) )
} }

View File

@ -277,7 +277,8 @@ func (s *currentRoomStateStatements) SelectCurrentState(
}, },
stateFilter.Senders, stateFilter.NotSenders, stateFilter.Senders, stateFilter.NotSenders,
stateFilter.Types, stateFilter.NotTypes, stateFilter.Types, stateFilter.NotTypes,
excludeEventIDs, stateFilter.ContainsURL, stateFilter.Limit, FilterOrderNone, excludeEventIDs, stateFilter.ContainsURL, 0,
FilterOrderNone,
) )
if err != nil { if err != nil {
return nil, fmt.Errorf("s.prepareWithFilters: %w", err) return nil, fmt.Errorf("s.prepareWithFilters: %w", err)

View File

@ -84,8 +84,10 @@ func prepareWithFilters(
case FilterOrderDesc: case FilterOrderDesc:
query += " ORDER BY id DESC" query += " ORDER BY id DESC"
} }
query += fmt.Sprintf(" LIMIT $%d", offset+1) if limit > 0 {
params = append(params, limit) query += fmt.Sprintf(" LIMIT $%d", offset+1)
params = append(params, limit)
}
var stmt *sql.Stmt var stmt *sql.Stmt
var err error var err error

View File

@ -200,7 +200,7 @@ func (s *outputRoomEventsStatements) SelectStateInRange(
s.db, txn, stmtSQL, inputParams, s.db, txn, stmtSQL, inputParams,
stateFilter.Senders, stateFilter.NotSenders, stateFilter.Senders, stateFilter.NotSenders,
stateFilter.Types, stateFilter.NotTypes, stateFilter.Types, stateFilter.NotTypes,
nil, stateFilter.ContainsURL, stateFilter.Limit, FilterOrderAsc, nil, stateFilter.ContainsURL, 0, FilterOrderAsc,
) )
} else { } else {
stmt, params, err = prepareWithFilters( stmt, params, err = prepareWithFilters(

View File

@ -301,7 +301,7 @@ func (p *PDUStreamProvider) addRoomDeltaToResponse(
} }
// Applies the history visibility rules // Applies the history visibility rules
events, err := applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, delta.RoomID, device.UserID, eventFilter.Limit, recentEvents) events, err := applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, delta.RoomID, device.UserID, recentEvents)
if err != nil { if err != nil {
logrus.WithError(err).Error("unable to apply history visibility filter") logrus.WithError(err).Error("unable to apply history visibility filter")
} }
@ -378,12 +378,12 @@ func applyHistoryVisibilityFilter(
snapshot storage.DatabaseTransaction, snapshot storage.DatabaseTransaction,
rsAPI roomserverAPI.SyncRoomserverAPI, rsAPI roomserverAPI.SyncRoomserverAPI,
roomID, userID string, roomID, userID string,
limit int,
recentEvents []*gomatrixserverlib.HeaderedEvent, recentEvents []*gomatrixserverlib.HeaderedEvent,
) ([]*gomatrixserverlib.HeaderedEvent, error) { ) ([]*gomatrixserverlib.HeaderedEvent, error) {
// We need to make sure we always include the latest states events, if they are in the timeline. // We need to make sure we always include the latest states events, if they are in the timeline.
// We grep at least limit * 2 events, to ensure we really get the needed events. // We grep at least limit * 2 events, to ensure we really get the needed events.
stateEvents, err := snapshot.CurrentState(ctx, roomID, &gomatrixserverlib.StateFilter{Limit: limit * 2}, nil) filter := gomatrixserverlib.DefaultStateFilter()
stateEvents, err := snapshot.CurrentState(ctx, roomID, &filter, nil)
if err != nil { if err != nil {
// Not a fatal error, we can continue without the stateEvents, // Not a fatal error, we can continue without the stateEvents,
// they are only needed if there are state events in the timeline. // they are only needed if there are state events in the timeline.
@ -521,7 +521,7 @@ func (p *PDUStreamProvider) getJoinResponseForCompleteSync(
events := recentEvents events := recentEvents
// Only apply history visibility checks if the response is for joined rooms // Only apply history visibility checks if the response is for joined rooms
if !isPeek { if !isPeek {
events, err = applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, roomID, device.UserID, eventFilter.Limit, recentEvents) events, err = applyHistoryVisibilityFilter(ctx, snapshot, p.rsAPI, roomID, device.UserID, recentEvents)
if err != nil { if err != nil {
logrus.WithError(err).Error("unable to apply history visibility filter") logrus.WithError(err).Error("unable to apply history visibility filter")
} }
@ -601,7 +601,6 @@ func (p *PDUStreamProvider) lazyLoadMembers(
} }
// Query missing membership events // Query missing membership events
filter := gomatrixserverlib.DefaultStateFilter() filter := gomatrixserverlib.DefaultStateFilter()
filter.Limit = stateFilter.Limit
filter.Senders = &wantUsers filter.Senders = &wantUsers
filter.Types = &[]string{gomatrixserverlib.MRoomMember} filter.Types = &[]string{gomatrixserverlib.MRoomMember}
memberships, err := snapshot.GetStateEventsForRoom(ctx, roomID, &filter) memberships, err := snapshot.GetStateEventsForRoom(ctx, roomID, &filter)

View File

@ -79,7 +79,6 @@ func newSyncRequest(req *http.Request, device userapi.Device, syncDB storage.Dat
// for the rest of the data to trickle down. // for the rest of the data to trickle down.
filter.AccountData.Limit = math.MaxInt32 filter.AccountData.Limit = math.MaxInt32
filter.Room.AccountData.Limit = math.MaxInt32 filter.Room.AccountData.Limit = math.MaxInt32
filter.Room.State.Limit = math.MaxInt32
} }
logger := util.GetLogger(req.Context()).WithFields(logrus.Fields{ logger := util.GetLogger(req.Context()).WithFields(logrus.Fields{