Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Delayed membership responses in /sync cause UTDs #4291

Open
Tracked by #245
kegsay opened this issue Jun 28, 2024 · 2 comments
Open
Tracked by #245

Delayed membership responses in /sync cause UTDs #4291

kegsay opened this issue Jun 28, 2024 · 2 comments
Labels
A-E2EE O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@kegsay
Copy link
Member

kegsay commented Jun 28, 2024

This outlines a race condition in the CSAPI which can cause UTDs.

Consider:

  • Alice is in a E2EE room and invites Bob. The request 200 OKs but has yet to come down /sync.
  • Alice tries to send a message in the room.
  • Alice should encrypt for Bob.

In practice, clients will not encrypt for Bob, causing a UTD if you very quickly send an encrypted message after inviting a user. This can happen due to:

  • A bot is using the SDK so message sending is very quick after an invite
  • DMs: for better UX a room is only made / user invited / message sent, when someone sends a message in a DM room rather than just selects the user's room. This means the message is already ready to go at the same time as invite time.

To fix this, we should be remembering that we, the client, have modified the membership state of the room, and invalidate the room member list (so we hit /members again). We can't assume that a 200 OK to /invite will guarantee that the user is in an invited state, so we still need to defer to the server.

A test for this is in matrix-org/complement-crypto#98

@richvdh
Copy link
Member

richvdh commented Jun 28, 2024

To fix this, we should be remembering that we, the client, have modified the membership state of the room, and invalidate the room member list (so we hit /members again).

Possibly, though the race isn't limited to this case: it's always possible for a membership change to happen in the room at the same time as we send a message, whether or not we caused that membership change.

I'd argue this is just a special-case of element-hq/element-meta#2268 and a fix to that would fix this particular case.

(With that said, fixing this particular case is probably easier than a more general fix so may be worthwhile anyway.)

@kegsay
Copy link
Member Author

kegsay commented Jun 28, 2024

it's always possible for a membership change to happen in the room at the same time as we send a message, whether or not we caused that membership change.

I agree, but that is fundamentally non-deterministic as senders/receivers don't synchronise in any way. For this issue, it can be and bots/scripts/apps expect it to be deterministic.

@dbkr dbkr added A-E2EE O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-E2EE O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Development

No branches or pull requests

3 participants