diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index bc7a0f390..ca01f715d 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -2,7 +2,7 @@ use std::convert::TryFrom; -use anyhow::{bail, ensure, format_err, Result}; +use anyhow::{bail, ensure, Result}; use itertools::join; use mailparse::SingleInfo; use num_traits::FromPrimitive; @@ -19,7 +19,7 @@ use crate::constants::{ use crate::contact::{addr_cmp, normalize_name, Contact, Origin, VerifiedStatus}; use crate::context::Context; use crate::dc_tools::{ - dc_create_smeared_timestamp, dc_extract_grpid_from_rfc724_mid, dc_smeared_time, time, + dc_create_smeared_timestamp, dc_extract_grpid_from_rfc724_mid, dc_smeared_time, }; use crate::ephemeral::{stock_ephemeral_timer_changed, Timer as EphemeralTimer}; use crate::events::EventType; @@ -210,27 +210,38 @@ pub(crate) async fn dc_receive_imf_inner( } if let Some(avatar_action) = &mime_parser.user_avatar { - match contact::set_profile_image( - context, - from_id, - avatar_action, - mime_parser.was_encrypted(), - ) - .await + if from_id != 0 + && context + .update_contacts_timestamp(from_id, Param::AvatarTimestamp, sent_timestamp) + .await? { - Ok(()) => { - context.emit_event(EventType::ChatModified(chat_id)); - } - Err(err) => { - warn!(context, "receive_imf cannot update profile image: {}", err); - } - }; + match contact::set_profile_image( + context, + from_id, + avatar_action, + mime_parser.was_encrypted(), + ) + .await + { + Ok(()) => { + context.emit_event(EventType::ChatModified(chat_id)); + } + Err(err) => { + warn!(context, "receive_imf cannot update profile image: {}", err); + } + }; + } } // Always update the status, even if there is no footer, to allow removing the status. // // Ignore MDNs though, as they never contain the signature even if user has set it. - if mime_parser.mdn_reports.is_empty() { + if mime_parser.mdn_reports.is_empty() + && from_id != 0 + && context + .update_contacts_timestamp(from_id, Param::StatusTimestamp, sent_timestamp) + .await? + { if let Err(err) = contact::set_status( context, from_id, @@ -410,6 +421,9 @@ async fn add_parts( } } + let rcvd_timestamp = dc_smeared_time(context).await; + *sent_timestamp = std::cmp::min(*sent_timestamp, rcvd_timestamp); + // check if the message introduces a new chat: // - outgoing messages introduce a chat with the first to: address if they are sent by a messenger // - incoming messages introduce a chat only for known contacts if they are sent by a messenger @@ -483,6 +497,7 @@ async fn add_parts( if let Some((new_chat_id, new_chat_id_blocked)) = create_or_lookup_group( context, &mut mime_parser, + *sent_timestamp, if test_normal_chat.is_none() { allow_creation } else { @@ -712,6 +727,7 @@ async fn add_parts( if let Some((new_chat_id, new_chat_id_blocked)) = create_or_lookup_group( context, &mut mime_parser, + *sent_timestamp, allow_creation, Blocked::Not, from_id, @@ -805,9 +821,8 @@ async fn add_parts( let location_kml_is = mime_parser.location_kml.is_some(); // correct message_timestamp, it should not be used before, - // however, we cannot do this earlier as we need from_id to be set + // however, we cannot do this earlier as we need chat_id to be set let in_fresh = state == MessageState::InFresh; - let rcvd_timestamp = time(); let sort_timestamp = calc_sort_timestamp(context, *sent_timestamp, chat_id, in_fresh).await?; // Apply ephemeral timer changes to the chat. @@ -823,6 +838,9 @@ async fn add_parts( || parent.is_none() || parent.unwrap().ephemeral_timer != ephemeral_timer) && chat_id.get_ephemeral_timer(context).await? != ephemeral_timer + && chat_id + .update_timestamp(context, Param::EphemeralSettingsTimestamp, *sent_timestamp) + .await? { if let Err(err) = chat_id .inner_set_ephemeral_timer(context, ephemeral_timer) @@ -868,6 +886,10 @@ async fn add_parts( }; if chat.is_protected() || new_status.is_some() { + // TODO: on partial downloads, do not try to check verified properties, + // eg. the Chat-Verified header is in the encrypted part and we would always get warnings. + // however, this seems not to be a big deal as we even show "failed verifications" messages in-chat - + // nothing else is done for "not yet downloaded content". if let Err(err) = check_verified_properties(context, mime_parser, from_id, to_ids).await { warn!(context, "verification problem: {}", err); @@ -876,15 +898,24 @@ async fn add_parts( } else { // change chat protection only when verification check passes if let Some(new_status) = new_status { - if let Err(e) = chat_id.inner_set_protection(context, new_status).await { - chat::add_info_msg( + if chat_id + .update_timestamp( context, - chat_id, - format!("Cannot set protection: {}", e), - sort_timestamp, + Param::ProtectionSettingsTimestamp, + *sent_timestamp, ) - .await; - return Ok(chat_id); // do not return an error as this would result in retrying the message + .await? + { + if let Err(e) = chat_id.inner_set_protection(context, new_status).await { + chat::add_info_msg( + context, + chat_id, + format!("Cannot set protection: {}", e), + sort_timestamp, + ) + .await; + return Ok(chat_id); // do not return an error as this would result in retrying the message + } } set_better_msg( mime_parser, @@ -910,8 +941,6 @@ async fn add_parts( std::cmp::max(sort_timestamp, parent_timestamp) }); - *sent_timestamp = std::cmp::min(*sent_timestamp, rcvd_timestamp); - // if the mime-headers should be saved, find out its size // (the mime-header ends with an empty line) let save_mime_headers = context.get_config_bool(Config::SaveMimeHeaders).await?; @@ -1097,25 +1126,23 @@ INSERT INTO msgs } } - async fn update_last_subject( - context: &Context, - chat_id: ChatId, - mime_parser: &MimeMessage, - ) -> Result<()> { - let mut chat = Chat::load_from_db(context, chat_id).await?; - chat.param.set( - Param::LastSubject, - mime_parser - .get_subject() - .ok_or_else(|| format_err!("No subject in email"))?, - ); - chat.update_param(context).await?; - Ok(()) - } if !is_mdn { - update_last_subject(context, chat_id, mime_parser) - .await - .ok_or_log_msg(context, "Could not update LastSubject of chat"); + let mut chat = Chat::load_from_db(context, chat_id).await?; + + // In contrast to most other update-timestamps, + // use `sort_timestamp` instead of `sent_timestamp` for the subject-timestamp comparison. + // This way, `LastSubject` actually refers to the most recent message _shown_ in the chat. + if chat + .param + .update_timestamp(Param::SubjectTimestamp, sort_timestamp)? + { + // write the last subject even if empty - + // otherwise a reply may get an outdated subject. + let subject = mime_parser.get_subject().unwrap_or_default(); + + chat.param.set(Param::LastSubject, subject); + chat.update_param(context).await?; + } } Ok(chat_id) @@ -1314,6 +1341,7 @@ async fn is_probably_private_reply( async fn create_or_lookup_group( context: &Context, mime_parser: &mut MimeMessage, + sent_timestamp: i64, allow_creation: bool, create_blocked: Blocked, from_id: u32, @@ -1323,7 +1351,6 @@ async fn create_or_lookup_group( let mut recreate_member_list = false; let mut send_EVENT_CHAT_MODIFIED = false; let mut X_MrAddToGrp = None; - let mut X_MrGrpNameChanged = false; let mut better_msg: String = From::from(""); if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled { @@ -1401,7 +1428,6 @@ async fn create_or_lookup_group( better_msg = stock_str::msg_add_member(context, &added_member, from_id).await; X_MrAddToGrp = Some(added_member); } else if let Some(old_name) = mime_parser.get_header(HeaderDef::ChatGroupNameChanged) { - X_MrGrpNameChanged = true; better_msg = stock_str::msg_grp_name( context, old_name, @@ -1531,9 +1557,16 @@ async fn create_or_lookup_group( // execute group commands if X_MrAddToGrp.is_some() { recreate_member_list = true; - } else if X_MrGrpNameChanged { + } else if mime_parser + .get_header(HeaderDef::ChatGroupNameChanged) + .is_some() + { if let Some(ref grpname) = grpname { - if grpname.len() < 200 { + if grpname.len() < 200 + && chat_id + .update_timestamp(context, Param::GroupNameTimestamp, sent_timestamp) + .await? + { info!(context, "updating grpname for chat {}", chat_id); if context .sql @@ -1555,54 +1588,69 @@ async fn create_or_lookup_group( if let Some(avatar_action) = &mime_parser.group_avatar { info!(context, "group-avatar change for {}", chat_id); if let Ok(mut chat) = Chat::load_from_db(context, chat_id).await { - match avatar_action { - AvatarAction::Change(profile_image) => { - chat.param.set(Param::ProfileImage, profile_image); - } - AvatarAction::Delete => { - chat.param.remove(Param::ProfileImage); - } - }; - chat.update_param(context).await?; - send_EVENT_CHAT_MODIFIED = true; + if chat + .param + .update_timestamp(Param::AvatarTimestamp, sent_timestamp)? + { + match avatar_action { + AvatarAction::Change(profile_image) => { + chat.param.set(Param::ProfileImage, profile_image); + } + AvatarAction::Delete => { + chat.param.remove(Param::ProfileImage); + } + }; + chat.update_param(context).await?; + send_EVENT_CHAT_MODIFIED = true; + } } } // add members to group/check members if recreate_member_list { - if !chat::is_contact_in_chat(context, chat_id, DC_CONTACT_ID_SELF).await { - // Members could have been removed while we were - // absent. We can't use existing member list and need to - // start from scratch. - context - .sql - .execute( - "DELETE FROM chats_contacts WHERE chat_id=?;", - paramsv![chat_id], - ) - .await - .ok(); - - chat::add_to_chat_contacts_table(context, chat_id, DC_CONTACT_ID_SELF).await; - } - if from_id > DC_CONTACT_ID_LAST_SPECIAL - && !Contact::addr_equals_contact(context, &self_addr, from_id).await - && !chat::is_contact_in_chat(context, chat_id, from_id).await + if chat_id + .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) + .await? { - chat::add_to_chat_contacts_table(context, chat_id, from_id).await; - } - for &to_id in to_ids.iter() { - info!(context, "adding to={:?} to chat id={}", to_id, chat_id); - if !Contact::addr_equals_contact(context, &self_addr, to_id).await - && !chat::is_contact_in_chat(context, chat_id, to_id).await - { - chat::add_to_chat_contacts_table(context, chat_id, to_id).await; + if !chat::is_contact_in_chat(context, chat_id, DC_CONTACT_ID_SELF).await { + // Members could have been removed while we were + // absent. We can't use existing member list and need to + // start from scratch. + context + .sql + .execute( + "DELETE FROM chats_contacts WHERE chat_id=?;", + paramsv![chat_id], + ) + .await + .ok(); + + chat::add_to_chat_contacts_table(context, chat_id, DC_CONTACT_ID_SELF).await; } + if from_id > DC_CONTACT_ID_LAST_SPECIAL + && !Contact::addr_equals_contact(context, &self_addr, from_id).await + && !chat::is_contact_in_chat(context, chat_id, from_id).await + { + chat::add_to_chat_contacts_table(context, chat_id, from_id).await; + } + for &to_id in to_ids.iter() { + info!(context, "adding to={:?} to chat id={}", to_id, chat_id); + if !Contact::addr_equals_contact(context, &self_addr, to_id).await + && !chat::is_contact_in_chat(context, chat_id, to_id).await + { + chat::add_to_chat_contacts_table(context, chat_id, to_id).await; + } + } + send_EVENT_CHAT_MODIFIED = true; } - send_EVENT_CHAT_MODIFIED = true; } else if let Some(contact_id) = removed_id { - chat::remove_from_chat_contacts_table(context, chat_id, contact_id).await; - send_EVENT_CHAT_MODIFIED = true; + if chat_id + .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) + .await? + { + chat::remove_from_chat_contacts_table(context, chat_id, contact_id).await; + send_EVENT_CHAT_MODIFIED = true; + } } if send_EVENT_CHAT_MODIFIED { diff --git a/src/lib.rs b/src/lib.rs index 6d9220696..8a8f1ffb0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -83,6 +83,7 @@ mod simplify; mod smtp; pub mod stock_str; mod token; +mod update_helper; #[macro_use] mod dehtml; mod color; diff --git a/src/param.rs b/src/param.rs index 3e2278a59..cecc36911 100644 --- a/src/param.rs +++ b/src/param.rs @@ -139,6 +139,27 @@ pub enum Param { /// For MDN-sending job MsgId = b'I', + + /// For Contacts: timestamp of status (aka signature or footer) update. + StatusTimestamp = b'j', + + /// For Contacts and Chats: timestamp of avatar update. + AvatarTimestamp = b'J', + + /// For Chats: timestamp of status/signature/footer update. + EphemeralSettingsTimestamp = b'B', + + /// For Chats: timestamp of subject update. + SubjectTimestamp = b'C', + + /// For Chats: timestamp of group name update. + GroupNameTimestamp = b'g', + + /// For Chats: timestamp of group name update. + MemberListTimestamp = b'k', + + /// For Chats: timestamp of protection settings update. + ProtectionSettingsTimestamp = b'L', } /// An object for handling key=value parameter lists. @@ -245,6 +266,11 @@ impl Params { self.get(key).and_then(|s| s.parse().ok()) } + /// Get the given parameter and parse as `i64`. + pub fn get_i64(&self, key: Param) -> Option { + self.get(key).and_then(|s| s.parse().ok()) + } + /// Get the given parameter and parse as `bool`. pub fn get_bool(&self, key: Param) -> Option { self.get_int(key).map(|v| v != 0) @@ -346,6 +372,12 @@ impl Params { self } + /// Set the given paramter to the passed in `i64`. + pub fn set_i64(&mut self, key: Param, value: i64) -> &mut Self { + self.set(key, value.to_string()); + self + } + /// Set the given parameter to the passed in `f64` . pub fn set_float(&mut self, key: Param, value: f64) -> &mut Self { self.set(key, format!("{}", value)); diff --git a/src/update_helper.rs b/src/update_helper.rs new file mode 100644 index 000000000..72b7240a2 --- /dev/null +++ b/src/update_helper.rs @@ -0,0 +1,197 @@ +//! # Functions to update timestamps. + +use crate::chat::{Chat, ChatId}; +use crate::contact::Contact; +use crate::context::Context; +use crate::param::{Param, Params}; +use anyhow::Result; + +impl Context { + /// Updates a contact's timestamp, if reasonable. + /// Returns true if the caller shall update the settings belonging to the scope. + /// (if we have a ContactId type at some point, the function should go there) + pub(crate) async fn update_contacts_timestamp( + &self, + contact_id: u32, + scope: Param, + new_timestamp: i64, + ) -> Result { + let mut contact = Contact::load_from_db(self, contact_id).await?; + if contact.param.update_timestamp(scope, new_timestamp)? { + contact.update_param(self).await?; + return Ok(true); + } + Ok(false) + } +} + +impl ChatId { + /// Updates a chat id's timestamp on disk, if reasonable. + /// Returns true if the caller shall update the settings belonging to the scope. + pub(crate) async fn update_timestamp( + &self, + context: &Context, + scope: Param, + new_timestamp: i64, + ) -> Result { + let mut chat = Chat::load_from_db(context, *self).await?; + if chat.param.update_timestamp(scope, new_timestamp)? { + chat.update_param(context).await?; + return Ok(true); + } + Ok(false) + } +} + +impl Params { + /// Updates a param's timestamp in memory, if reasonable. + /// Returns true if the caller shall update the settings belonging to the scope. + pub(crate) fn update_timestamp(&mut self, scope: Param, new_timestamp: i64) -> Result { + let old_timestamp = self.get_i64(scope).unwrap_or_default(); + if new_timestamp >= old_timestamp { + self.set_i64(scope, new_timestamp); + return Ok(true); + } + Ok(false) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::dc_receive_imf::dc_receive_imf; + use crate::dc_tools::time; + use crate::test_utils::TestContext; + + #[async_std::test] + async fn test_params_update_timestamp() -> Result<()> { + let mut params = Params::new(); + let ts = time(); + + assert!(params.update_timestamp(Param::LastSubject, ts)?); + assert!(params.update_timestamp(Param::LastSubject, ts)?); // same timestamp -> update + assert!(params.update_timestamp(Param::LastSubject, ts + 10)?); + assert!(!params.update_timestamp(Param::LastSubject, ts)?); // `ts` is now too old + assert!(!params.update_timestamp(Param::LastSubject, 0)?); + assert_eq!(params.get_i64(Param::LastSubject).unwrap(), ts + 10); + + assert!(params.update_timestamp(Param::GroupNameTimestamp, 0)?); // stay unset -> update ... + assert!(params.update_timestamp(Param::GroupNameTimestamp, 0)?); // ... also on multiple calls + assert_eq!(params.get_i64(Param::GroupNameTimestamp).unwrap(), 0); + + assert!(!params.update_timestamp(Param::AvatarTimestamp, -1)?); + assert_eq!(params.get_i64(Param::AvatarTimestamp), None); + + Ok(()) + } + + #[async_std::test] + async fn test_out_of_order_subject() -> Result<()> { + let t = TestContext::new_alice().await; + + dc_receive_imf( + &t, + b"From: Bob Authname \n\ + To: alice@example.com\n\ + Subject: updated subject\n\ + Message-ID: \n\ + Chat-Version: 1.0\n\ + Date: Sun, 22 Mar 2021 23:37:57 +0000\n\ + \n\ + second message\n", + "INBOX", + 1, + false, + ) + .await?; + dc_receive_imf( + &t, + b"From: Bob Authname \n\ + To: alice@example.com\n\ + Subject: original subject\n\ + Message-ID: \n\ + Chat-Version: 1.0\n\ + Date: Sun, 22 Mar 2021 22:37:57 +0000\n\ + \n\ + first message\n", + "INBOX", + 2, + false, + ) + .await?; + + let msg = t.get_last_msg().await; + let chat = Chat::load_from_db(&t, msg.chat_id).await?; + assert_eq!( + chat.param.get(Param::LastSubject).unwrap(), + "updated subject" + ); + + Ok(()) + } + + #[async_std::test] + async fn test_out_of_order_group_name() -> Result<()> { + let t = TestContext::new_alice().await; + + dc_receive_imf( + &t, + b"From: Bob Authname \n\ + To: alice@example.com\n\ + Message-ID: \n\ + Chat-Version: 1.0\n\ + Chat-Group-ID: abcde\n\ + Chat-Group-Name: initial name\n\ + Date: Sun, 22 Mar 2021 01:00:00 +0000\n\ + \n\ + first message\n", + "INBOX", + 1, + false, + ) + .await?; + let msg = t.get_last_msg().await; + let chat = Chat::load_from_db(&t, msg.chat_id).await?; + assert_eq!(chat.name, "initial name"); + + dc_receive_imf( + &t, + b"From: Bob Authname \n\ + To: alice@example.com\n\ + Message-ID: \n\ + Chat-Version: 1.0\n\ + Chat-Group-ID: abcde\n\ + Chat-Group-Name: another name update\n\ + Chat-Group-Name-Changed: a name update\n\ + Date: Sun, 22 Mar 2021 03:00:00 +0000\n\ + \n\ + third message\n", + "INBOX", + 2, + false, + ) + .await?; + dc_receive_imf( + &t, + b"From: Bob Authname \n\ + To: alice@example.com\n\ + Message-ID: \n\ + Chat-Version: 1.0\n\ + Chat-Group-ID: abcde\n\ + Chat-Group-Name: a name update\n\ + Chat-Group-Name-Changed: initial name\n\ + Date: Sun, 22 Mar 2021 02:00:00 +0000\n\ + \n\ + second message\n", + "INBOX", + 3, + false, + ) + .await?; + let msg = t.get_last_msg().await; + let chat = Chat::load_from_db(&t, msg.chat_id).await?; + assert_eq!(chat.name, "another name update"); + + Ok(()) + } +}