Skip to content

Commit

Permalink
fix(base): Fix a bug when an invite has no timestamp.
Browse files Browse the repository at this point in the history
The sliding sync proxy has a bug: despite the presence of
`m.room.create` in `bump_event_types`, an invite with an `m.room.create`
event will not have a `timestamp`. Thus, such a room cannot be sorted
reliably.

This patch fixes this problem with a terrible hack, where it tries to
find an `origin_server_ts` value from within the `invite_state` state
events.

Please read the comment to learn more.
  • Loading branch information
Hywan authored and poljar committed Jul 19, 2024
1 parent 23a232e commit 730d5a3
Showing 1 changed file with 126 additions and 6 deletions.
132 changes: 126 additions & 6 deletions crates/matrix-sdk-base/src/sliding_sync/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@
pub mod http;

use std::collections::BTreeMap;
#[cfg(feature = "e2e-encryption")]
use std::ops::Deref;
use std::{borrow::Cow, collections::BTreeMap};

#[cfg(feature = "e2e-encryption")]
use matrix_sdk_common::deserialized_responses::SyncTimelineEvent;
Expand All @@ -28,9 +28,9 @@ use ruma::{
api::client::sync::sync_events::v3::{self, InvitedRoom},
events::{AnyRoomAccountDataEvent, AnySyncStateEvent, AnySyncTimelineEvent},
serde::Raw,
JsOption, OwnedRoomId, RoomId,
JsOption, OwnedRoomId, RoomId, UInt,
};
use tracing::{instrument, trace, warn};
use tracing::{debug, error, instrument, trace, warn};

use super::BaseClient;
#[cfg(feature = "e2e-encryption")]
Expand Down Expand Up @@ -356,6 +356,11 @@ impl BaseClient {
from_simplified_sliding_sync: bool,
) -> Result<(RoomInfo, Option<JoinedRoomUpdate>, Option<LeftRoomUpdate>, Option<InvitedRoom>)>
{
// This method may change `room_data` (see the terrible hack describes below)
// with `timestamp` and `invite_state. We don't want to change the `room_data`
// from outside this method, hence `Cow` is perfectly suited here.
let mut room_data = Cow::Borrowed(room_data);

let (raw_state_events, state_events): (Vec<_>, Vec<_>) = {
let mut state_events = Vec::new();

Expand All @@ -371,9 +376,48 @@ impl BaseClient {
// Find or create the room in the store
let is_new_room = !store.room_exists(room_id);

// Terrible hack I (Ivan) am ashamed to write, but there is a bug in the sliding
// sync proxy… When a user receive an invite to a room, the room has no
// `timestamp` despites having `m.room.create` in `bump_event_types`. The result
// of this is that an invite cannot be sorted. This horrible hack will fix that.
//
// The SDK manipulates Simplified MSC3575 `Request` and `Response` though. In
// Simplified MSC3575, `bump_stamp` replaces `timestamp`, which does NOT
// represent a time! This hack must really, only, apply to the proxy, so to
// MSC3575 strictly (hence the `from_simplified_sliding_sync` argument).
//
// The proxy uses the `origin_server_ts` event's value to fill the `timestamp`
// room's value (which is a bad idea[^1]). If `timestamp` is `None`, let's find
// out which event in `invite_state` can be a candidate: we take the latest one.
//
// [^1]: using `origin_server_ts` for `timestamp` is a bad idea because
// this value can be forged by a malicious user. Anyway, that's how it works
// in the proxy. Simplified MSC3575 has another mechanism which fixes the
// problem.
if !from_simplified_sliding_sync && room_data.bump_stamp.is_none() {
if let Some(invite_state) = &room_data.invite_state {
room_data.to_mut().bump_stamp =
invite_state.iter().rev().find_map(|invite_state| {
invite_state.get_field::<UInt>("origin_server_ts").ok().flatten()
});

debug!(
?room_id,
timestamp = ?room_data.bump_stamp,
"Received a room with no `timestamp`; looked for a replacement value"
);
} else {
error!(?room_id, "Received a room with no `timestamp` and no `invite_state`");
}
}

#[allow(unused_mut)] // Required for some feature flag combinations
let (mut room, mut room_info, invited_room) =
self.process_sliding_sync_room_membership(room_data, &state_events, store, room_id);
let (mut room, mut room_info, invited_room) = self.process_sliding_sync_room_membership(
room_data.as_ref(),
&state_events,
store,
room_id,
);

room_info.mark_state_partially_synced();

Expand Down Expand Up @@ -406,7 +450,7 @@ impl BaseClient {

process_room_properties(
room_id,
room_data,
room_data.as_ref(),
&mut room_info,
is_new_room,
room_info_notable_updates,
Expand Down Expand Up @@ -1370,6 +1414,76 @@ mod tests {
assert!(!sync_resp.rooms.join.contains_key(room_id));
}

#[async_test]
async fn test_invitation_room_receive_a_default_timestamp_on_not_simplified_sliding_sync() {
const NOT_SIMPLIFIED_SLIDING_SYNC: bool = false;

// Given a logged-in client
let client = logged_in_base_client(None).await;
let room_id = room_id!("!r:e.uk");
let user_id = user_id!("@u:e.uk");

// When I send sliding sync response containing an invited room with no
// `origin_server_ts` from any events in `invite_state`
let mut room = http::response::Room::new();
set_room_invited(&mut room, user_id, user_id);
let response = response_with_room(room_id, room);
let _sync_resp = client
.process_sliding_sync(&response, &(), NOT_SIMPLIFIED_SLIDING_SYNC)
.await
.expect("Failed to process sync");

// Then the room has no recency stamp
let client_room = client.get_room(room_id).expect("No room found");
assert!(client_room.recency_stamp().is_none());

// When I send sliding sync response containing an invited room with an
// `origin_server_ts` from an event in `invite_state`
let mut room = http::response::Room::new();
set_room_invited(&mut room, user_id, user_id);

if let Some(invite_state) = room.invite_state.as_mut() {
invite_state.push(
Raw::new(&json!({
"type": "m.room.member",
"sender": user_id,
"content": {
"is_direct": true,
"membership": "invite",
},
"state_key": user_id,
// The `origin_server_ts` is in the middle of other events.
"origin_server_ts": 123456789,
}))
.expect("Failed to make raw event")
.cast(),
);
invite_state.push(
Raw::new(&json!({
"type": "m.room.member",
"sender": user_id,
"content": {
"is_direct": true,
"membership": "invite",
},
"state_key": user_id,
}))
.expect("Failed to make raw event")
.cast(),
);
}

let response = response_with_room(room_id, room);
let _sync_resp = client
.process_sliding_sync(&response, &(), NOT_SIMPLIFIED_SLIDING_SYNC)
.await
.expect("Failed to process sync");

// Then the room has a recency stamp!
let client_room = client.get_room(room_id).expect("No room found");
assert_eq!(client_room.recency_stamp(), Some(123456789));
}

#[async_test]
async fn test_avatar_is_found_in_invitation_room_when_processing_sliding_sync_response() {
// Given a logged-in client
Expand Down Expand Up @@ -2280,6 +2394,12 @@ mod tests {

let evt = Raw::new(&json!({
"type": "m.room.member",
"sender": inviter,
"content": {
"is_direct": true,
"membership": "invite",
},
"state_key": invitee,
}))
.expect("Failed to make raw event")
.cast();
Expand Down

0 comments on commit 730d5a3

Please sign in to comment.