From ffa586e486b4c4c096c801de21add62eb721b95b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 6 Sep 2024 16:21:55 +0200 Subject: [PATCH 1/7] test: Test that a timeline decrypts an event if a backup got enabled --- .../src/tests/timeline.rs | 174 +++++++++++++++++- 1 file changed, 170 insertions(+), 4 deletions(-) diff --git a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs index 80ebce3b2db..38b96ca3d52 100644 --- a/testing/matrix-sdk-integration-testing/src/tests/timeline.rs +++ b/testing/matrix-sdk-integration-testing/src/tests/timeline.rs @@ -19,12 +19,23 @@ use assert_matches::assert_matches; use assert_matches2::assert_let; use assign::assign; use eyeball_im::{Vector, VectorDiff}; +use futures::pin_mut; use futures_util::{FutureExt, StreamExt}; -use matrix_sdk::ruma::{ - api::client::room::create_room::v3::Request as CreateRoomRequest, - events::room::message::RoomMessageEventContent, MilliSecondsSinceUnixEpoch, +use matrix_sdk::{ + encryption::{backups::BackupState, EncryptionSettings}, + ruma::{ + api::client::room::create_room::v3::Request as CreateRoomRequest, + events::{ + room::{encryption::RoomEncryptionEventContent, message::RoomMessageEventContent}, + InitialStateEvent, + }, + MilliSecondsSinceUnixEpoch, + }, }; -use matrix_sdk_ui::timeline::{EventSendState, ReactionStatus, RoomExt, TimelineItem}; +use matrix_sdk_ui::timeline::{ + EventSendState, ReactionStatus, RoomExt, TimelineItem, TimelineItemContent, +}; +use similar_asserts::assert_eq; use tokio::{ spawn, task::JoinHandle, @@ -294,3 +305,158 @@ async fn test_stale_local_echo_time_abort_edit() { alice_sync.abort(); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn test_enabling_backups_retries_decryption() { + let encryption_settings = EncryptionSettings { + auto_enable_backups: true, + backup_download_strategy: + matrix_sdk::encryption::BackupDownloadStrategy::AfterDecryptionFailure, + ..Default::default() + }; + let alice = TestClientBuilder::new("alice") + .use_sqlite() + .encryption_settings(encryption_settings) + .build() + .await + .unwrap(); + + alice.encryption().wait_for_e2ee_initialization_tasks().await; + alice + .encryption() + .recovery() + .enable() + .with_passphrase("Bomber's code") + .await + .expect("We should be able to enable recovery"); + + let user_id = alice.user_id().expect("We should have access to the user ID now"); + assert_eq!( + alice.encryption().backups().state(), + BackupState::Enabled, + "The backup state should now be BackupState::Enabled" + ); + + // Set up sync for user Alice, and create a room. + let alice_sync = spawn({ + let alice = alice.clone(); + async move { + alice.sync(Default::default()).await.expect("sync failed!"); + } + }); + + debug!("Creating room…"); + + let initial_state = + vec![InitialStateEvent::new(RoomEncryptionEventContent::with_recommended_defaults()) + .to_raw_any()]; + + let room = alice + .create_room(assign!(CreateRoomRequest::new(), { + is_direct: true, + initial_state, + preset: Some(matrix_sdk::ruma::api::client::room::create_room::v3::RoomPreset::PrivateChat) + })) + .await + .unwrap(); + + assert!(room + .is_encrypted() + .await + .expect("We should be able to check that the room is encrypted")); + + let event_id = room + .send(RoomMessageEventContent::text_plain("It's a secret to everybody!")) + .await + .expect("We should be able to send a message to our new room") + .event_id; + + alice + .encryption() + .backups() + .wait_for_steady_state() + .await + .expect("We should be able to wait for our room keys to be uploaded"); + + // We don't need Alice anymore. + alice_sync.abort(); + + // I know that this is Alice again, but it's the Alice's second device which she + // named Bob. + let bob = TestClientBuilder::with_exact_username(user_id.localpart().to_owned()) + .use_sqlite() + .encryption_settings(encryption_settings) + .build() + .await + .unwrap(); + + bob.encryption().wait_for_e2ee_initialization_tasks().await; + assert!(!bob.encryption().backups().are_enabled().await, "Backups shouldn't be enabled yet"); + + // Sync once to get access to the room. + bob.sync_once(Default::default()).await.expect("Bob should be able to perform an initial sync"); + // Let Bob sync continuously now. + let bob_sync = spawn({ + let bob = bob.clone(); + async move { + bob.sync(Default::default()).await.expect("sync failed!"); + } + }); + + // Load the event into the timeline. + let room = bob.get_room(room.room_id()).expect("We should have access to our room now"); + let timeline = room.timeline().await.expect("We should be able to get a timeline for our room"); + timeline + .paginate_backwards(50) + .await + .expect("We should be able to paginate the timeline to fetch the history"); + + let item = + timeline.item_by_event_id(&event_id).await.expect("The event should be in the timeline"); + + // The event is not decrypted yet. + assert_matches!(item.content(), TimelineItemContent::UnableToDecrypt(_)); + + // We now connect to the backup which will not give us the room key right away, + // we first need to encounter a UTD to attempt the download. + bob.encryption() + .recovery() + .recover("Bomber's code") + .await + .expect("We should be able to recover from Bob's device"); + + // Let's subscribe to our timeline so we don't miss the transition from UTD to + // decrypted event. + let (_, mut stream) = timeline + .subscribe_filter_map(|item| { + item.as_event().cloned().filter(|item| item.event_id() == Some(&event_id)) + }) + .await; + + let room_key_stream = bob.encryption().backups().room_keys_for_room_stream(room.room_id()); + pin_mut!(room_key_stream); + + // Wait for the room key to arrive before continuing. + if let Some(update) = room_key_stream.next().await { + let _update = update.expect( + "We should not miss the update since we're only broadcasting a small amount of updates", + ); + } + + // Alright, we should now receive an update that the event had been decrypted. + let _vector_diff = timeout(Duration::from_secs(5), stream.next()).await.unwrap().unwrap(); + + // Let's fetch the event again. + let item = + timeline.item_by_event_id(&event_id).await.expect("The event should be in the timeline"); + + // Yup it's decrypted great. + assert_let!( + TimelineItemContent::Message(message) = item.content(), + "The event should have been decrypted now" + ); + + assert_eq!(message.body(), "It's a secret to everybody!"); + + bob_sync.abort(); +} From 63f305b632bfedf73870bb57a1a6aed685da6d4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 9 Sep 2024 14:38:45 +0200 Subject: [PATCH 2/7] test: Check that we don't mark keys as downloaded before backups were enabled --- crates/matrix-sdk/src/encryption/tasks.rs | 72 +++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/crates/matrix-sdk/src/encryption/tasks.rs b/crates/matrix-sdk/src/encryption/tasks.rs index d4cf23d0feb..295457a7ceb 100644 --- a/crates/matrix-sdk/src/encryption/tasks.rs +++ b/crates/matrix-sdk/src/encryption/tasks.rs @@ -141,6 +141,7 @@ impl Drop for BackupDownloadTask { } impl BackupDownloadTask { + #[cfg(not(test))] const DOWNLOAD_DELAY_MILLIS: u64 = 100; pub(crate) fn new(client: WeakClient) -> Self { @@ -215,6 +216,7 @@ impl BackupDownloadTask { download_request: RoomKeyDownloadRequest, ) { // Wait a bit, perhaps the room key will arrive in the meantime. + #[cfg(not(test))] tokio::time::sleep(Duration::from_millis(Self::DOWNLOAD_DELAY_MILLIS)).await; // Now take the lock, and check that we still want to do a download. If we do, @@ -239,6 +241,7 @@ impl BackupDownloadTask { // Before we drop the lock, indicate to other tasks that may be considering this // session that we're going to go ahead and do a download. state.downloaded_sessions.insert(download_request.to_room_key_info()); + client }; @@ -267,6 +270,7 @@ impl BackupDownloadTask { state.failures_cache.insert(room_key_info); } } + state.active_tasks.remove(&download_request.event_id); } } @@ -371,3 +375,71 @@ impl BackupDownloadTaskListenerState { true } } + +#[cfg(all(test, not(target_arch = "wasm32")))] +mod test { + use matrix_sdk_test::async_test; + use ruma::{event_id, room_id}; + use serde_json::json; + use wiremock::MockServer; + + use super::*; + use crate::test_utils::logged_in_client; + + // Test that, if backups are not enabled, we don't incorrectly mark a room key + // as downloaded. + #[async_test] + async fn test_disabled_backup_does_not_mark_room_key_as_downloaded() { + let room_id = room_id!("!DovneieKSTkdHKpIXy:morpheus.localhost"); + let event_id = event_id!("$JbFHtZpEJiH8uaajZjPLz0QUZc1xtBR9rPGBOjF6WFM"); + let session_id = "session_id"; + + let server = MockServer::start().await; + let client = logged_in_client(Some(server.uri())).await; + let weak_client = WeakClient::from_client(&client); + + let event_content = json!({ + "event_id": event_id, + "origin_server_ts": 1698579035927u64, + "sender": "@example2:morpheus.localhost", + "type": "m.room.encrypted", + "content": { + "algorithm": "m.megolm.v1.aes-sha2", + "ciphertext": "AwgAEpABhetEzzZzyYrxtEVUtlJnZtJcURBlQUQJ9irVeklCTs06LwgTMQj61PMUS4Vy\ + YOX+PD67+hhU40/8olOww+Ud0m2afjMjC3wFX+4fFfSkoWPVHEmRVucfcdSF1RSB4EmK\ + PIP4eo1X6x8kCIMewBvxl2sI9j4VNvDvAN7M3zkLJfFLOFHbBviI4FN7hSFHFeM739Zg\ + iwxEs3hIkUXEiAfrobzaMEM/zY7SDrTdyffZndgJo7CZOVhoV6vuaOhmAy4X2t4UnbuV\ + JGJjKfV57NAhp8W+9oT7ugwO", + "device_id": "KIUVQQSDTM", + "sender_key": "LvryVyoCjdONdBCi2vvoSbI34yTOx7YrCFACUEKoXnc", + "session_id": "64H7XKokIx0ASkYDHZKlT5zd/Zccz/cQspPNdvnNULA" + } + }); + + let event: Raw = + serde_json::from_value(event_content).expect(""); + + let state = Arc::new(Mutex::new(BackupDownloadTaskListenerState::new(weak_client))); + let download_request = RoomKeyDownloadRequest { + room_id: room_id.into(), + megolm_session_id: session_id.to_owned(), + event, + event_id: event_id.into(), + }; + + assert!( + !client.encryption().backups().are_enabled().await, + "Backups should not be enabled." + ); + + BackupDownloadTask::handle_download_request(state.clone(), download_request).await; + + { + let state = state.lock().await; + assert!( + !state.downloaded_sessions.contains(&(room_id.to_owned(), session_id.to_owned())), + "Backups are not enabled, we should not mark any room keys as downloaded." + ) + } + } +} From 8aec0febf8fba3830523f490c5405e71213193e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 6 Sep 2024 16:23:08 +0200 Subject: [PATCH 3/7] timeline: Retry decryption if a room key backup gets enabled --- crates/matrix-sdk-ui/src/timeline/builder.rs | 40 ++++++++++++++++++++ crates/matrix-sdk-ui/src/timeline/mod.rs | 4 ++ 2 files changed, 44 insertions(+) diff --git a/crates/matrix-sdk-ui/src/timeline/builder.rs b/crates/matrix-sdk-ui/src/timeline/builder.rs index 23b14c13199..04bfcad83fe 100644 --- a/crates/matrix-sdk-ui/src/timeline/builder.rs +++ b/crates/matrix-sdk-ui/src/timeline/builder.rs @@ -16,6 +16,7 @@ use std::{collections::BTreeSet, sync::Arc}; use futures_util::{pin_mut, StreamExt}; use matrix_sdk::{ + encryption::backups::BackupState, event_cache::{EventsOrigin, RoomEventCacheUpdate}, executor::spawn, Room, @@ -370,6 +371,44 @@ impl TimelineBuilder { }) }; + let room_key_backup_enabled_join_handle = { + let inner = controller.clone(); + let stream = client.encryption().backups().state_stream(); + + spawn(async move { + pin_mut!(stream); + + while let Some(update) = stream.next().await { + match update { + // If the backup got enabled, or we lagged and thus missed that the backup + // might be enabled, retry to decrypt all the events. Please note, depending + // on the backup download strategy, this might do two things under the + // assumption that the backup contains the relevant room keys: + // + // 1. It will decrypt the events, if `BackupDownloadStrategy` has been set + // to `OneShot`. + // 2. It will fail to decrypt the event, but try to download the room key to + // decrypt it if the `BackupDownloadStrategy` has been set to + // `AfterDecryptionFailure`. + Ok(BackupState::Enabled) | Err(_) => { + let room = inner.room(); + inner.retry_event_decryption(room, None).await; + } + // The other states aren't interesting since they are either still enabling + // the backup or have the backup in the disabled state. + Ok( + BackupState::Unknown + | BackupState::Creating + | BackupState::Resuming + | BackupState::Disabling + | BackupState::Downloading + | BackupState::Enabling, + ) => (), + } + } + }) + }; + let timeline = Timeline { controller, event_cache: room_event_cache, @@ -379,6 +418,7 @@ impl TimelineBuilder { room_update_join_handle, pinned_events_join_handle, room_key_from_backups_join_handle, + room_key_backup_enabled_join_handle, local_echo_listener_handle, _event_cache_drop_handle: event_cache_drop, }), diff --git a/crates/matrix-sdk-ui/src/timeline/mod.rs b/crates/matrix-sdk-ui/src/timeline/mod.rs index 584d9aeb826..2e346bebb27 100644 --- a/crates/matrix-sdk-ui/src/timeline/mod.rs +++ b/crates/matrix-sdk-ui/src/timeline/mod.rs @@ -851,6 +851,7 @@ struct TimelineDropHandle { room_update_join_handle: JoinHandle<()>, pinned_events_join_handle: Option>, room_key_from_backups_join_handle: JoinHandle<()>, + room_key_backup_enabled_join_handle: JoinHandle<()>, local_echo_listener_handle: JoinHandle<()>, _event_cache_drop_handle: Arc, } @@ -860,12 +861,15 @@ impl Drop for TimelineDropHandle { for handle in self.event_handler_handles.drain(..) { self.client.remove_event_handler(handle); } + if let Some(handle) = self.pinned_events_join_handle.take() { handle.abort() }; + self.local_echo_listener_handle.abort(); self.room_update_join_handle.abort(); self.room_key_from_backups_join_handle.abort(); + self.room_key_backup_enabled_join_handle.abort(); } } From 599da0ede8bb97a80d7a6db6991a0b6282931e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 6 Sep 2024 16:24:21 +0200 Subject: [PATCH 4/7] backups: Don't mark a room key as downloaded if we did not attempt to download it --- crates/matrix-sdk/CHANGELOG.md | 8 ++++++++ .../matrix-sdk/src/encryption/backups/mod.rs | 19 ++++++++++++++++--- crates/matrix-sdk/src/encryption/tasks.rs | 8 +++++++- 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/crates/matrix-sdk/CHANGELOG.md b/crates/matrix-sdk/CHANGELOG.md index 19d82de5184..3705158a858 100644 --- a/crates/matrix-sdk/CHANGELOG.md +++ b/crates/matrix-sdk/CHANGELOG.md @@ -45,6 +45,14 @@ Additions: - WidgetDriver: Support the `"delay"` field in the `send_event` widget actions. This allows to send delayed events, as defined in [MSC4157](https://github.com/matrix-org/matrix-spec-proposals/pull/4157) +Bug fixes: + +- Fix a bug where room keys were considered to be downloaded before backups were + enabled. This bug only affects the + `BackupDownloadStrategy::AfterDecryptionFailure`, where no attempt would be + made to download a room key, if a decryption failure with a given room key + would have been encountered before the backups were enabled. + # 0.7.0 Breaking changes: diff --git a/crates/matrix-sdk/src/encryption/backups/mod.rs b/crates/matrix-sdk/src/encryption/backups/mod.rs index d863f62a96b..59457221d10 100644 --- a/crates/matrix-sdk/src/encryption/backups/mod.rs +++ b/crates/matrix-sdk/src/encryption/backups/mod.rs @@ -445,7 +445,16 @@ impl Backups { } /// Download a single room key from the server-side key backup. - pub async fn download_room_key(&self, room_id: &RoomId, session_id: &str) -> Result<(), Error> { + /// + /// Returns `true` if we managed to download a room key, `false` or an error + /// if we failed to download it. `false` indicates that there was no + /// error, we just don't have backups enabled so we can't download a + /// room key. + pub async fn download_room_key( + &self, + room_id: &RoomId, + session_id: &str, + ) -> Result { let olm_machine = self.client.olm_machine().await; let olm_machine = olm_machine.as_ref().ok_or(Error::NoOlmMachine)?; @@ -471,10 +480,14 @@ impl Backups { self.handle_downloaded_room_keys(response, decryption_key, &version, olm_machine) .await?; + + Ok(true) + } else { + Ok(false) } + } else { + Ok(false) } - - Ok(()) } /// Set the state of the backup. diff --git a/crates/matrix-sdk/src/encryption/tasks.rs b/crates/matrix-sdk/src/encryption/tasks.rs index 295457a7ceb..c5a43b962d5 100644 --- a/crates/matrix-sdk/src/encryption/tasks.rs +++ b/crates/matrix-sdk/src/encryption/tasks.rs @@ -256,12 +256,18 @@ impl BackupDownloadTask { { let mut state = state.lock().await; let room_key_info = download_request.to_room_key_info(); + match result { - Ok(_) => { + Ok(true) => { // We successfully downloaded the session. We can clear any record of previous // backoffs from the failures cache, because we won't be needing them again. state.failures_cache.remove(std::iter::once(&room_key_info)) } + Ok(false) => { + // We did not find a valid backup decryption key or backup version, we did not + // even attempt to download the room key. + state.downloaded_sessions.remove(&room_key_info); + } Err(_) => { // We were unable to download the session. Update the failure cache so that we // back off from more requests, and also remove the entry from the list of From af767ceaaa4395f131f665da3323937a05ef2607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 6 Sep 2024 16:24:43 +0200 Subject: [PATCH 5/7] backups: Don't queue up room keys to be downloaded if backups aren't enabled --- crates/matrix-sdk/src/encryption/tasks.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/crates/matrix-sdk/src/encryption/tasks.rs b/crates/matrix-sdk/src/encryption/tasks.rs index c5a43b962d5..e2c84f41761 100644 --- a/crates/matrix-sdk/src/encryption/tasks.rs +++ b/crates/matrix-sdk/src/encryption/tasks.rs @@ -344,6 +344,16 @@ impl BackupDownloadTaskListenerState { return false; }; + // If backups aren't enabled, there's no point in trying to download a room key. + if !client.encryption().backups().are_enabled().await { + debug!( + ?download_request, + "Not performing backup download because backups are not enabled" + ); + + return false; + } + // Check if the keys for this message have arrived in the meantime. // If we get a StoreError doing the lookup, we assume the keys haven't arrived // (though if the store is returning errors, probably something else is From a744d9101691eea321baeaa58e15af187540cd56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Fri, 6 Sep 2024 16:49:17 +0200 Subject: [PATCH 6/7] backups: Expire downloaded room keys so they get retried if a better one is found --- crates/matrix-sdk/src/encryption/tasks.rs | 27 ++++++++++------------- 1 file changed, 12 insertions(+), 15 deletions(-) diff --git a/crates/matrix-sdk/src/encryption/tasks.rs b/crates/matrix-sdk/src/encryption/tasks.rs index e2c84f41761..43c8acd4636 100644 --- a/crates/matrix-sdk/src/encryption/tasks.rs +++ b/crates/matrix-sdk/src/encryption/tasks.rs @@ -12,11 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::{ - collections::{BTreeMap, HashSet}, - sync::Arc, - time::Duration, -}; +use std::{collections::BTreeMap, sync::Arc, time::Duration}; use matrix_sdk_common::failures_cache::FailuresCache; use ruma::{ @@ -37,6 +33,9 @@ use crate::{ Client, }; +/// A cache of room keys we already downloaded. +type DownloadCache = FailuresCache; + #[derive(Default)] pub(crate) struct ClientTasks { #[cfg(feature = "e2e-encryption")] @@ -266,13 +265,13 @@ impl BackupDownloadTask { Ok(false) => { // We did not find a valid backup decryption key or backup version, we did not // even attempt to download the room key. - state.downloaded_sessions.remove(&room_key_info); + state.downloaded_sessions.remove(std::iter::once(&room_key_info)); } Err(_) => { // We were unable to download the session. Update the failure cache so that we // back off from more requests, and also remove the entry from the list of // sessions that we are downloading. - state.downloaded_sessions.remove(&room_key_info); + state.downloaded_sessions.remove(std::iter::once(&room_key_info)); state.failures_cache.insert(room_key_info); } } @@ -300,12 +299,7 @@ struct BackupDownloadTaskListenerState { /// The idea here is that once we've (successfully) downloaded a session /// from the backup, there's not much point trying again even if we get /// another UTD event that uses the same session. - /// - /// TODO: that's not quite right though. In theory another client could - /// update the backup with an earlier ratchet state, giving us access - /// to earlier messages in the session. In which case, maybe this - /// should expire? - downloaded_sessions: HashSet, + downloaded_sessions: DownloadCache, } impl BackupDownloadTaskListenerState { @@ -320,7 +314,10 @@ impl BackupDownloadTaskListenerState { client, failures_cache: FailuresCache::with_settings(Duration::from_secs(60 * 60 * 24), 60), active_tasks: Default::default(), - downloaded_sessions: Default::default(), + downloaded_sessions: DownloadCache::with_settings( + Duration::from_secs(60 * 60 * 24), + 60, + ), } } @@ -373,7 +370,7 @@ impl BackupDownloadTaskListenerState { if self.downloaded_sessions.contains(&room_key_info) { debug!( ?download_request, - "Not performing backup download because this session has already been downloaded" + "Not performing backup download because this session has already been downloaded recently" ); return false; }; From 7df232912f3d687dee80d13a0bd62b44610fd7f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Damir=20Jeli=C4=87?= Date: Mon, 9 Sep 2024 14:48:14 +0200 Subject: [PATCH 7/7] backups: Rename the term session to room key The term session is usually only used in the crypto crate to reference a Megolm session, the rest of the SDK uses the name from the event and the Matrix spec, this should lower the amount of confusion since the main crate has already a session concept and its unrelated to end-to-end encryption. --- crates/matrix-sdk/src/encryption/tasks.rs | 46 +++++++++++------------ 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/crates/matrix-sdk/src/encryption/tasks.rs b/crates/matrix-sdk/src/encryption/tasks.rs index 43c8acd4636..5955a1ea961 100644 --- a/crates/matrix-sdk/src/encryption/tasks.rs +++ b/crates/matrix-sdk/src/encryption/tasks.rs @@ -113,7 +113,7 @@ struct RoomKeyDownloadRequest { /// The event we could not decrypt. event: Raw, - /// The megolm session that the event was encrypted with. + /// The unique ID of the room key that the event was encrypted with. megolm_session_id: String, } @@ -206,7 +206,7 @@ impl BackupDownloadTask { } } - /// Handle a request to download a session for a given event. + /// Handle a request to download a room key for a given event. /// /// Sleeps for a while to see if the key turns up; then checks if we still /// want to do a download, and does the download if so. @@ -238,8 +238,8 @@ impl BackupDownloadTask { } // Before we drop the lock, indicate to other tasks that may be considering this - // session that we're going to go ahead and do a download. - state.downloaded_sessions.insert(download_request.to_room_key_info()); + // room key, that we're going to go ahead and do a download. + state.downloaded_room_keys.insert(download_request.to_room_key_info()); client }; @@ -258,20 +258,20 @@ impl BackupDownloadTask { match result { Ok(true) => { - // We successfully downloaded the session. We can clear any record of previous + // We successfully downloaded the room key. We can clear any record of previous // backoffs from the failures cache, because we won't be needing them again. state.failures_cache.remove(std::iter::once(&room_key_info)) } Ok(false) => { // We did not find a valid backup decryption key or backup version, we did not // even attempt to download the room key. - state.downloaded_sessions.remove(std::iter::once(&room_key_info)); + state.downloaded_room_keys.remove(std::iter::once(&room_key_info)); } Err(_) => { - // We were unable to download the session. Update the failure cache so that we + // We were unable to download the room key. Update the failure cache so that we // back off from more requests, and also remove the entry from the list of - // sessions that we are downloading. - state.downloaded_sessions.remove(std::iter::once(&room_key_info)); + // room keys that we are downloading. + state.downloaded_room_keys.remove(std::iter::once(&room_key_info)); state.failures_cache.insert(room_key_info); } } @@ -293,13 +293,13 @@ struct BackupDownloadTaskListenerState { /// Map from event ID to download task active_tasks: BTreeMap>, - /// A list of megolm sessions that we have already downloaded, or are about - /// to download. + /// A list of room keys that we have already downloaded, or are about to + /// download. /// - /// The idea here is that once we've (successfully) downloaded a session + /// The idea here is that once we've (successfully) downloaded a room key /// from the backup, there's not much point trying again even if we get - /// another UTD event that uses the same session. - downloaded_sessions: DownloadCache, + /// another UTD event that uses the same room key. + downloaded_room_keys: DownloadCache, } impl BackupDownloadTaskListenerState { @@ -314,7 +314,7 @@ impl BackupDownloadTaskListenerState { client, failures_cache: FailuresCache::with_settings(Duration::from_secs(60 * 60 * 24), 60), active_tasks: Default::default(), - downloaded_sessions: DownloadCache::with_settings( + downloaded_room_keys: DownloadCache::with_settings( Duration::from_secs(60 * 60 * 24), 60, ), @@ -325,8 +325,8 @@ impl BackupDownloadTaskListenerState { /// /// Checks if: /// * we already have the key, - /// * we have already downloaded this session, or are about to do so, or - /// * we've backed off from trying to download this session. + /// * we have already downloaded this room key, or are about to do so, or + /// * we've backed off from trying to download this room key. /// /// If any of the above are true, returns `false`. Otherwise, returns /// `true`. @@ -364,22 +364,22 @@ impl BackupDownloadTaskListenerState { return false; } - // Check if we already downloaded this session, or another task is in the + // Check if we already downloaded this room key, or another task is in the // process of doing so. let room_key_info = download_request.to_room_key_info(); - if self.downloaded_sessions.contains(&room_key_info) { + if self.downloaded_room_keys.contains(&room_key_info) { debug!( ?download_request, - "Not performing backup download because this session has already been downloaded recently" + "Not performing backup download because this room key has already been downloaded recently" ); return false; }; - // Check if we're backing off from attempts to download this session + // Check if we're backing off from attempts to download this room key if self.failures_cache.contains(&room_key_info) { debug!( ?download_request, - "Not performing backup download because this session failed to download recently" + "Not performing backup download because this room key failed to download recently" ); return false; } @@ -450,7 +450,7 @@ mod test { { let state = state.lock().await; assert!( - !state.downloaded_sessions.contains(&(room_id.to_owned(), session_id.to_owned())), + !state.downloaded_room_keys.contains(&(room_id.to_owned(), session_id.to_owned())), "Backups are not enabled, we should not mark any room keys as downloaded." ) }