Bugfix: don't return extra member count or e2ee device updates from sync #389
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we were returning redundant member count updates or encrypted device updates from the /sync endpoint in some cases. The extra member count updates are spec-compliant, but unnecessary, while the extra encrypted device updates violate the spec.
The refactor necessary to fix this bug is also necessary to support filtering on state events in sync.
Details
Joined room incremental sync needs to examine state events for four purposes:
device_lists
)rooms.joined.*.state
)The state events that we need to examine for the first two cases is member events in the delta between
since
and the end oftimeline
. For the second two cases, we need the delta betweensince
and the start oftimeline
, plus contextual member events for any senders that occur intimeline
. The second list is subject to filtering, while the first is not.Before this change, we were using the same set of state events that we are returning to the client (cases 3/4) to do the analysis for cases 1/2. In a compliant implementation, this would result in us missing some relevant member events in 1/2 in addition to seeing redundant member events. In current conduwuit this is not the case because the set of events that we return to the client is always a superset of the set that is needed for cases 1/2. This is because we don't support filtering, and we have an existing bug where we are returning the delta between
since
and the end oftimeline
rather than the start.Fixing this is necessary to implement filtering because otherwise we would start missing some member events for member count or encrypted device updates if the relevant member events are rejected by the filter. This would be much worse than our current behavior.