Reintroduce #19714 - Send a SSS response immediately if the config has changed#19792
Conversation
…re new results to sync (#19714) This fixes the bug described in #19713 (and double-checked against the SDK integration test, which now passes with this change). A sync response must be returned immediately if a room subscription configuration change caused a new non-empty response (checked with `if response` in the code) to be produced. Fixes #19713. Fixes #18844. --------- Co-authored-by: Erik Johnston <erik@matrix.org>
| # Note that "signed_curve25519" is always returned in key count responses | ||
| # regardless of whether we uploaded any keys for it. This is necessary until | ||
| # https://github.com/matrix-org/matrix-doc/issues/3298 is fixed. | ||
| # | ||
| # Also related: | ||
| # https://github.com/element-hq/element-android/issues/3725 and | ||
| # https://github.com/matrix-org/synapse/issues/10456 | ||
| default_otk = self.device_one_time_keys_count.get("signed_curve25519") | ||
| more_than_default_otk = len(self.device_one_time_keys_count) > 1 or ( | ||
| default_otk is not None and default_otk > 0 | ||
| ) | ||
| # | ||
| # This is why we don't incorporate `device_one_time_keys_count` | ||
| # (or `device_unused_fallback_key_types`) into the | ||
| # `__bool__` check. |
There was a problem hiding this comment.
This seems more like a workaround than a fix. Ideally, we would immediately respond if these things changed. The risk is that the one-time keys could be exhausted without the users /sync waking up and knowing to add more. We should at-least add a FIXME here and justification for why we think it's fine.
There was a problem hiding this comment.
(I guess that justification would be that 30s is a short window of staleness and that we have fallback keys available.)
There was a problem hiding this comment.
I've noted that this is not the ideal situation in 2edfbe2
There was a problem hiding this comment.
We can do better:
- There should be a
FIXMEin the comment to clearly point out the wrong-ness - We should explain the risk and the justification for why we think its fine.
There was a problem hiding this comment.
591f8c7 has a fixme and roughly says why its fine for the changes to be delayed
Follow on from #19714, where we should have had an extra comment and test. Co-authored-by: Eric Eastwood <madlittlemods@gmail.com>
|
I've merged in #19734 |
erikjohnston
force-pushed
the
erikj/sss_immediately
branch
from
d21146d to
2edfbe2
Compare
MadLittleMods
left a comment
MadLittleMods
left a comment
There was a problem hiding this comment.
(approved in terms of the specific review things I pointed out)
Co-authored-by: Eric Eastwood <erice@element.io>
4 participants
Reintroduces #19714, after being reverted in #19784.
Fix #18844
Fix #19783
Fix #18880
This PR also adds a fix so that we don't always return immediately when using the e2ee extension.