From 9152b877a7f9e2439d5ac1211505e75b58605ffe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timo=20K=C3=B6sters?= Date: Wed, 25 Aug 2021 17:36:10 +0200 Subject: [PATCH] fix: wrong soft fail check, too many events in /sync state response --- src/client_server/sync.rs | 30 +++++------------------------- src/database.rs | 1 + src/database/rooms.rs | 33 +++++++++++++++++++++++++++++++++ src/server_server.rs | 23 ++++++++++++----------- 4 files changed, 51 insertions(+), 36 deletions(-) diff --git a/src/client_server/sync.rs b/src/client_server/sync.rs index 81260479..270a5f06 100644 --- a/src/client_server/sync.rs +++ b/src/client_server/sync.rs @@ -246,31 +246,13 @@ async fn sync_helper( .current_shortstatehash(&room_id)? .expect("All rooms have state"); - let first_pdu_before_since = db - .rooms - .pdus_until(&sender_user, &room_id, since)? - .next() - .transpose()?; - let pdus_after_since = db .rooms .pdus_after(&sender_user, &room_id, since)? .next() .is_some(); - let since_shortstatehash = first_pdu_before_since - .as_ref() - .map(|pdu| { - db.rooms - .pdu_shortstatehash(&pdu.1.event_id) - .transpose() - .ok_or_else(|| { - warn!("PDU without state: {}", pdu.1.event_id); - Error::bad_database("Found PDU without state") - }) - }) - .transpose()? - .transpose()?; + let since_shortstatehash = db.rooms.get_token_shortstatehash(&room_id, since)?; // Calculates joined_member_count, invited_member_count and heroes let calculate_counts = || { @@ -359,7 +341,7 @@ async fn sync_helper( true, state_events, ) - } else if !pdus_after_since || since_shortstatehash == Some(current_shortstatehash) { + } else if !pdus_after_since && since_shortstatehash == Some(current_shortstatehash) { // No state changes (Vec::new(), None, None, false, Vec::new()) } else { @@ -400,11 +382,6 @@ async fn sync_helper( current_state_ids .iter() .filter(|(key, id)| since_state_ids.get(key) != Some(id)) - .filter(|(_, id)| { - !timeline_pdus - .iter() - .any(|(_, timeline_pdu)| timeline_pdu.event_id == **id) - }) .map(|(_, id)| db.rooms.get_pdu(id)) .filter_map(|r| r.ok().flatten()) .collect() @@ -585,6 +562,9 @@ async fn sync_helper( ); } + // Save the state after this sync so we can send the correct state diff next sync + db.rooms.associate_token_shortstatehash(&room_id, next_batch, current_shortstatehash)?; + let joined_room = sync_events::JoinedRoom { account_data: sync_events::RoomAccountData { events: db diff --git a/src/database.rs b/src/database.rs index a6ac67f8..193fcf25 100644 --- a/src/database.rs +++ b/src/database.rs @@ -271,6 +271,7 @@ impl Database { shorteventid_eventid: builder.open_tree("shorteventid_eventid")?, shorteventid_shortstatehash: builder.open_tree("shorteventid_shortstatehash")?, roomid_shortstatehash: builder.open_tree("roomid_shortstatehash")?, + roomsynctoken_shortstatehash: builder.open_tree("roomsynctoken_shortstatehash")?, statehash_shortstatehash: builder.open_tree("statehash_shortstatehash")?, eventid_outlierpdu: builder.open_tree("eventid_outlierpdu")?, diff --git a/src/database/rooms.rs b/src/database/rooms.rs index 0d99c52b..bb27e017 100644 --- a/src/database/rooms.rs +++ b/src/database/rooms.rs @@ -69,6 +69,7 @@ pub struct Rooms { /// Remember the current state hash of a room. pub(super) roomid_shortstatehash: Arc, + pub(super) roomsynctoken_shortstatehash: Arc, /// Remember the state hash at events in the past. pub(super) shorteventid_shortstatehash: Arc, /// StateKey = EventType + StateKey, ShortStateKey = Count @@ -1800,6 +1801,38 @@ impl Rooms { Ok(()) } + pub fn associate_token_shortstatehash( + &self, + room_id: &RoomId, + token: u64, + shortstatehash: u64, + ) -> Result<()> { + let shortroomid = self.get_shortroomid(room_id)?.expect("room exists"); + + let mut key = shortroomid.to_be_bytes().to_vec(); + key.extend_from_slice(&token.to_be_bytes()); + + self.roomsynctoken_shortstatehash + .insert(&key, &shortstatehash.to_be_bytes()) + } + + pub fn get_token_shortstatehash(&self, room_id: &RoomId, token: u64) -> Result> { + let shortroomid = self.get_shortroomid(room_id)?.expect("room exists"); + + let mut key = shortroomid.to_be_bytes().to_vec(); + key.extend_from_slice(&token.to_be_bytes()); + + Ok(self + .roomsynctoken_shortstatehash + .get(&key)? + .map(|bytes| { + utils::u64_from_bytes(&bytes).map_err(|_| { + Error::bad_database("Invalid shortstatehash in roomsynctoken_shortstatehash") + }) + }) + .transpose()?) + } + /// Creates a new persisted data unit and adds it to a room. #[tracing::instrument(skip(self, db, _mutex_lock))] pub fn build_and_append_pdu( diff --git a/src/server_server.rs b/src/server_server.rs index cb89e40d..526ed518 100644 --- a/src/server_server.rs +++ b/src/server_server.rs @@ -1387,6 +1387,17 @@ async fn upgrade_outlier_to_timeline_pdu( .state_full_ids(current_sstatehash) .map_err(|_| "Failed to load room state.")?; + let auth_events = db + .rooms + .get_auth_events( + &room_id, + &incoming_pdu.kind, + &incoming_pdu.sender, + incoming_pdu.state_key.as_deref(), + &incoming_pdu.content, + ) + .map_err(|_| "Failed to get_auth_events.".to_owned())?; + if incoming_pdu.state_key.is_some() { let mut extremity_sstatehashes = HashMap::new(); @@ -1541,18 +1552,8 @@ async fn upgrade_outlier_to_timeline_pdu( extremities.insert(incoming_pdu.event_id.clone()); - debug!("starting soft fail auth check"); // 13. Check if the event passes auth based on the "current state" of the room, if not "soft fail" it - let auth_events = db - .rooms - .get_auth_events( - &room_id, - &incoming_pdu.kind, - &incoming_pdu.sender, - incoming_pdu.state_key.as_deref(), - &incoming_pdu.content, - ) - .map_err(|_| "Failed to get_auth_events.".to_owned())?; + debug!("starting soft fail auth check"); let soft_fail = !state_res::event_auth::auth_check( &room_version,