matrix-org
Element X can overwrite user's DM rooms
We've had multiple bug reports on element-web from different users since ~Sept 2023 complaining that their DM rooms are being wiped. The meta issue for this is https://github.com/element-hq/element-meta/issues/2035
From one of the more recent rageshakes, I have dug into the server-side logs to see which user agent did the PUT to /account_data/m.direct and it implicated "Element X/0.4.9 (Google Pixel 7; Android 14; AP1A.240405.002; Sdk 742700b7f)" [0 dbevts].
In order to set DM rooms correctly, the API is horrible. The SDK MUST ensure that it first does a GET to fetch the latest m.direct event, then adds to the JSON map, then does a PUT of the whole event again. If it can't do the GET, it MUST NOT do the PUT or else it will overwrite the DM rooms on the account.
The offending code seems to be https://github.com/matrix-org/matrix-rust-sdk/blob/cbb92cacce1d16673a949204e4f88b78e9917026/crates/matrix-sdk/src/account.rs#L861 - it is not safe to unwrap_or_default - if the SDK cannot parse the DM event then it must not clobber it regardless.
It would also be nice to provide an error detailing why it cannot parse the JSON rather than silently continuing..
Yeah it seems the doc right above explains the same thing:
https://github.com/matrix-org/matrix-rust-sdk/blob/cbb92cacce1d16673a949204e4f88b78e9917026/crates/matrix-sdk/src/account.rs#L846-L861
If this is a deserialisation error, /sync won't help this. We need to preserve the user's DM information if we fail, not replace it.
I guess /sync will apply only valid events, so events that can be deserialized, and in this case, relying on the result of /sync help making unwrap_or_default() basically unreachable. Anyway, we need to address the problem 🙂.
I guess /sync will apply only valid events, so events that can be deserialized,
This isn't true. The proxy does need to inspect some events like the m.direct account data event, but if it can't do it properly it still passes it to the client regardless.