diff --git a/src/chat.rs b/src/chat.rs index 7f49a4ace6..4ab87b1c87 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -4025,12 +4025,12 @@ pub(crate) async fn add_contact_to_chat_ex( Ok(true) } -/// Returns true if an avatar should be attached in the given chat. +/// Returns whether profile data should be attached when sending to the given chat. /// -/// This function does not check if the avatar is set. +/// This function does not check if the avatar/status is set. /// If avatar is not set and this function returns `true`, /// a `Chat-User-Avatar: 0` header should be sent to reset the avatar. -pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId) -> Result { +pub(crate) async fn should_attach_profile(context: &Context, chat_id: ChatId) -> Result { let timestamp_some_days_ago = time() - DC_RESEND_USER_AVATAR_DAYS * 24 * 60 * 60; let needs_attach = context .sql @@ -4045,8 +4045,8 @@ pub(crate) async fn shall_attach_selfavatar(context: &Context, chat_id: ChatId) let mut needs_attach = false; for row in rows { let row = row?; - let selfavatar_sent = row?; - if selfavatar_sent < timestamp_some_days_ago { + let profile_sent = row?; + if profile_sent < timestamp_some_days_ago { needs_attach = true; } } diff --git a/src/chat/chat_tests.rs b/src/chat/chat_tests.rs index e9980f97e9..4759744faf 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1540,23 +1540,23 @@ async fn test_create_same_chat_twice() { } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn test_shall_attach_selfavatar() -> Result<()> { +async fn test_should_attach_profile() -> Result<()> { let mut tcm = TestContextManager::new(); let alice = &tcm.alice().await; let bob = &tcm.bob().await; let chat_id = create_group_chat(alice, ProtectionStatus::Unprotected, "foo").await?; - assert!(!shall_attach_selfavatar(alice, chat_id).await?); + assert!(!should_attach_profile(alice, chat_id).await?); let contact_id = alice.add_or_lookup_contact_id(bob).await; add_contact_to_chat(alice, chat_id, contact_id).await?; - assert!(shall_attach_selfavatar(alice, chat_id).await?); + assert!(should_attach_profile(alice, chat_id).await?); chat_id.set_selfavatar_timestamp(alice, time()).await?; - assert!(!shall_attach_selfavatar(alice, chat_id).await?); + assert!(!should_attach_profile(alice, chat_id).await?); alice.set_config(Config::Selfavatar, None).await?; // setting to None also forces re-sending - assert!(shall_attach_selfavatar(alice, chat_id).await?); + assert!(should_attach_profile(alice, chat_id).await?); Ok(()) } @@ -1580,7 +1580,7 @@ async fn test_profile_data_on_group_leave() -> Result<()> { tokio::fs::write(&file, bytes).await?; t.set_config(Config::Selfavatar, Some(file.to_str().unwrap())) .await?; - assert!(shall_attach_selfavatar(t, chat_id).await?); + assert!(should_attach_profile(t, chat_id).await?); remove_contact_from_chat(t, chat_id, ContactId::SELF).await?; let sent_msg = t.pop_sent_msg().await; diff --git a/src/config.rs b/src/config.rs index 6cd1705b6d..533e15bbd5 100644 --- a/src/config.rs +++ b/src/config.rs @@ -759,6 +759,14 @@ impl Context { let better_value; match key { + Config::Selfstatus => { + // Ensure that future versions send the self-status that after upgrade. Currently we + // send it in every appropriate message. + self.sql + .execute("UPDATE contacts SET selfavatar_sent=0", ()) + .await?; + self.sql.set_raw_config(key.as_ref(), value).await?; + } Config::Selfavatar => { self.sql .execute("UPDATE contacts SET selfavatar_sent=0;", ()) diff --git a/src/config/config_tests.rs b/src/config/config_tests.rs index fc0a296cf5..bec0fa2f78 100644 --- a/src/config/config_tests.rs +++ b/src/config/config_tests.rs @@ -278,7 +278,6 @@ async fn test_sync() -> Result<()> { Ok(()) } -/// Sync message mustn't be sent if self-{status,avatar} is changed by a self-sent message. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_no_sync_on_self_sent_msg() -> Result<()> { let mut tcm = TestContextManager::new(); @@ -288,7 +287,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> { a.set_config_bool(Config::SyncMsgs, true).await?; } - let status = "Synced via usual message"; + let status = "Sent via usual message"; alice0.set_config(Config::Selfstatus, Some(status)).await?; alice0.send_sync_msg().await?; alice0.pop_sent_sync_msg().await; @@ -297,7 +296,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> { tcm.send_recv(alice0, alice1, "hi Alice!").await; assert_eq!( alice1.get_config(Config::Selfstatus).await?, - Some(status.to_string()) + Some(status1.to_string()) ); sync(alice1, alice0).await; assert_eq!( @@ -328,7 +327,7 @@ async fn test_no_sync_on_self_sent_msg() -> Result<()> { alice1 .get_config(Config::Selfavatar) .await? - .filter(|path| path.ends_with(".png")) + .filter(|path| path.ends_with(".jpg")) .is_some() ); sync(alice1, alice0).await; diff --git a/src/contact.rs b/src/contact.rs index 18893e98e5..e0c1def6e2 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -383,11 +383,7 @@ async fn import_vcard_contact(context: &Context, contact: &VcardContact) -> Resu None => None, }; if let Some(path) = path { - // Currently this value doesn't matter as we don't import the contact of self. - let was_encrypted = false; - if let Err(e) = - set_profile_image(context, id, &AvatarAction::Change(path), was_encrypted).await - { + if let Err(e) = set_profile_image(context, id, &AvatarAction::Change(path)).await { warn!( context, "import_vcard_contact: Could not set avatar for {}: {e:#}.", contact.addr @@ -395,7 +391,7 @@ async fn import_vcard_contact(context: &Context, contact: &VcardContact) -> Resu } } if let Some(biography) = &contact.biography { - if let Err(e) = set_status(context, id, biography.to_owned(), false, false).await { + if let Err(e) = set_status(context, id, biography.to_owned()).await { warn!( context, "import_vcard_contact: Could not set biography for {}: {e:#}.", contact.addr @@ -1835,25 +1831,19 @@ WHERE type=? AND id IN ( /// The given profile image is expected to be already in the blob directory /// as profile images can be set only by receiving messages, this should be always the case, however. /// -/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar; -/// this typically happens if we see message with our own profile image. +/// For contact SELF, the image is not saved in the contact-database but as Config::Selfavatar. pub(crate) async fn set_profile_image( context: &Context, contact_id: ContactId, profile_image: &AvatarAction, - was_encrypted: bool, ) -> Result<()> { let mut contact = Contact::get_by_id(context, contact_id).await?; let changed = match profile_image { AvatarAction::Change(profile_image) => { if contact_id == ContactId::SELF { - if was_encrypted { - context - .set_config_ex(Nosync, Config::Selfavatar, Some(profile_image)) - .await?; - } else { - info!(context, "Do not use unencrypted selfavatar."); - } + context + .set_config_ex(Nosync, Config::Selfavatar, Some(profile_image)) + .await?; } else { contact.param.set(Param::ProfileImage, profile_image); } @@ -1861,13 +1851,9 @@ pub(crate) async fn set_profile_image( } AvatarAction::Delete => { if contact_id == ContactId::SELF { - if was_encrypted { - context - .set_config_ex(Nosync, Config::Selfavatar, None) - .await?; - } else { - info!(context, "Do not use unencrypted selfavatar deletion."); - } + context + .set_config_ex(Nosync, Config::Selfavatar, None) + .await?; } else { contact.param.remove(Param::ProfileImage); } @@ -1884,22 +1870,16 @@ pub(crate) async fn set_profile_image( /// Sets contact status. /// -/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus. This -/// is only done if message is sent from Delta Chat and it is encrypted, to synchronize signature -/// between Delta Chat devices. +/// For contact SELF, the status is not saved in the contact table, but as Config::Selfstatus. pub(crate) async fn set_status( context: &Context, contact_id: ContactId, status: String, - encrypted: bool, - has_chat_version: bool, ) -> Result<()> { if contact_id == ContactId::SELF { - if encrypted && has_chat_version { - context - .set_config_ex(Nosync, Config::Selfstatus, Some(&status)) - .await?; - } + context + .set_config_ex(Nosync, Config::Selfstatus, Some(&status)) + .await?; } else { let mut contact = Contact::get_by_id(context, contact_id).await?; diff --git a/src/contact/contact_tests.rs b/src/contact/contact_tests.rs index 14ccc4d93b..56c2168d40 100644 --- a/src/contact/contact_tests.rs +++ b/src/contact/contact_tests.rs @@ -4,7 +4,7 @@ use super::*; use crate::chat::{Chat, get_chat_contacts, send_text_msg}; use crate::chatlist::Chatlist; use crate::receive_imf::receive_imf; -use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote}; +use crate::test_utils::{self, TestContext, TestContextManager, TimeShiftFalsePositiveNote, sync}; #[test] fn test_contact_id_values() { @@ -831,8 +831,7 @@ CCCB 5AA9 F6E1 141C 9431 Ok(()) } -/// Tests that status is synchronized when sending encrypted BCC-self messages and not -/// synchronized when the message is not encrypted. +/// Tests that self-status is not synchronized from outgoing messages. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_synchronize_status() -> Result<()> { let mut tcm = TestContextManager::new(); @@ -851,21 +850,12 @@ async fn test_synchronize_status() -> Result<()> { .await?; let chat = alice1.create_email_chat(bob).await; - // Alice sends a message to Bob from the first device. + // Alice sends an unencrypted message to Bob from the first device. send_text_msg(alice1, chat.id, "Hello".to_string()).await?; let sent_msg = alice1.pop_sent_msg().await; - // Message is not encrypted. - let message = sent_msg.load_from_db().await; - assert!(!message.get_showpadlock()); - // Alice's second devices receives a copy of outgoing message. alice2.recv_msg(&sent_msg).await; - - // Bob receives message. - bob.recv_msg(&sent_msg).await; - - // Message was not encrypted, so status is not copied. assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status); // Alice sends encrypted message. @@ -873,17 +863,9 @@ async fn test_synchronize_status() -> Result<()> { send_text_msg(alice1, chat.id, "Hello".to_string()).await?; let sent_msg = alice1.pop_sent_msg().await; - // Second message is encrypted. - let message = sent_msg.load_from_db().await; - assert!(message.get_showpadlock()); - // Alice's second devices receives a copy of second outgoing message. alice2.recv_msg(&sent_msg).await; - - assert_eq!( - alice2.get_config(Config::Selfstatus).await?, - Some("New status".to_string()) - ); + assert_eq!(alice2.get_config(Config::Selfstatus).await?, default_status); Ok(()) } @@ -896,9 +878,9 @@ async fn test_selfavatar_changed_event() -> Result<()> { // Alice has two devices. let alice1 = &tcm.alice().await; let alice2 = &tcm.alice().await; - - // Bob has one device. - let bob = &tcm.bob().await; + for a in [alice1, alice2] { + a.set_config_bool(Config::SyncMsgs, true).await?; + } assert_eq!(alice1.get_config(Config::Selfavatar).await?, None); @@ -914,17 +896,7 @@ async fn test_selfavatar_changed_event() -> Result<()> { .get_matching(|e| matches!(e, EventType::SelfavatarChanged)) .await; - // Alice sends a message. - let alice1_chat_id = alice1.create_chat(bob).await.id; - send_text_msg(alice1, alice1_chat_id, "Hello".to_string()).await?; - let sent_msg = alice1.pop_sent_msg().await; - - // The message is encrypted. - let message = sent_msg.load_from_db().await; - assert!(message.get_showpadlock()); - - // Alice's second device receives a copy of the outgoing message. - alice2.recv_msg(&sent_msg).await; + sync(alice1, alice2).await; // Alice's second device applies the selfavatar. assert!(alice2.get_config(Config::Selfavatar).await?.is_some()); diff --git a/src/headerdef.rs b/src/headerdef.rs index 330a4d9ba0..115efa1237 100644 --- a/src/headerdef.rs +++ b/src/headerdef.rs @@ -60,7 +60,12 @@ pub enum HeaderDef { ChatGroupNameTimestamp, ChatVerified, ChatGroupAvatar, + + /// If present, contact's avatar and status should be applied from the message. + /// "Chat-User-Avatar: 0" means that the contact has no avatar. Contact's status is transferred + /// in the message footer. ChatUserAvatar, + ChatVoiceMessage, ChatGroupMemberRemoved, ChatGroupMemberAdded, diff --git a/src/mimefactory.rs b/src/mimefactory.rs index 0d971bf243..cf17398b7b 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -181,7 +181,7 @@ impl MimeFactory { pub async fn from_msg(context: &Context, msg: Message) -> Result { let now = time(); let chat = Chat::load_from_db(context, msg.chat_id).await?; - let attach_profile_data = Self::should_attach_profile_data(&msg); + let can_transfer_profile = Self::can_transfer_profile(&msg); let undisclosed_recipients = chat.typ == Chattype::OutBroadcast; let from_addr = context.get_primary_self_addr().await?; @@ -193,7 +193,7 @@ impl MimeFactory { if let Some(override_name) = msg.param.get(Param::OverrideSenderDisplayname) { (override_name.to_string(), Some(config_displayname)) } else { - let name = match attach_profile_data { + let name = match can_transfer_profile { true => config_displayname, false => "".to_string(), }; @@ -301,7 +301,7 @@ impl MimeFactory { } else { addr }; - let name = match attach_profile_data { + let name = match can_transfer_profile { true => authname, false => "".to_string(), }; @@ -453,14 +453,18 @@ impl MimeFactory { .split_ascii_whitespace() .map(|s| s.trim_start_matches('<').trim_end_matches('>').to_string()) .collect(); - let selfstatus = match attach_profile_data { + let should_attach_profile = Self::should_attach_profile(context, &msg).await; + // TODO: (2025-08) Attach self-status in every message for compatibility with older + // versions. Should be replaced with + // `should_attach_profile || !is_encrypted && can_transfer_profile`. + let selfstatus = match can_transfer_profile { true => context .get_config(Config::Selfstatus) .await? .unwrap_or_default(), false => "".to_string(), }; - let attach_selfavatar = Self::should_attach_selfavatar(context, &msg).await; + let attach_selfavatar = should_attach_profile; ensure_and_debug_assert!( member_timestamps.is_empty() @@ -553,7 +557,7 @@ impl MimeFactory { } } - fn should_attach_profile_data(msg: &Message) -> bool { + fn can_transfer_profile(msg: &Message) -> bool { msg.param.get_cmd() != SystemMessage::SecurejoinMessage || { let step = msg.param.get(Param::Arg).unwrap_or_default(); // Don't attach profile data at the earlier SecureJoin steps: @@ -568,14 +572,14 @@ impl MimeFactory { } } - async fn should_attach_selfavatar(context: &Context, msg: &Message) -> bool { - Self::should_attach_profile_data(msg) - && match chat::shall_attach_selfavatar(context, msg.chat_id).await { + async fn should_attach_profile(context: &Context, msg: &Message) -> bool { + Self::can_transfer_profile(msg) + && match chat::should_attach_profile(context, msg.chat_id).await { Ok(should) => should, Err(err) => { warn!( context, - "should_attach_selfavatar: cannot get selfavatar state: {err:#}." + "should_attach_profile: chat::should_attach_profile: {err:#}." ); false } @@ -640,7 +644,7 @@ impl MimeFactory { return Ok(format!("Re: {}", remove_subject_prefix(last_subject))); } - let self_name = match Self::should_attach_profile_data(msg) { + let self_name = match Self::can_transfer_profile(msg) { true => context.get_config(Config::Displayname).await?, false => None, }; diff --git a/src/reaction.rs b/src/reaction.rs index 685f811c95..2b9b58952c 100644 --- a/src/reaction.rs +++ b/src/reaction.rs @@ -962,42 +962,6 @@ Here's my footer -- bob@example.net" Ok(()) } - /// Regression test for reaction resetting self-status. - /// - /// Reactions do not contain the status, - /// but should not result in self-status being reset on other devices. - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn test_reaction_status_multidevice() -> Result<()> { - let mut tcm = TestContextManager::new(); - let alice1 = tcm.alice().await; - let alice2 = tcm.alice().await; - - alice1 - .set_config(Config::Selfstatus, Some("New status")) - .await?; - - let alice2_msg = tcm.send_recv(&alice1, &alice2, "Hi!").await; - assert_eq!( - alice2.get_config(Config::Selfstatus).await?.as_deref(), - Some("New status") - ); - - // Alice reacts to own message from second device, - // first device receives rection. - { - send_reaction(&alice2, alice2_msg.id, "👍").await?; - let msg = alice2.pop_sent_msg().await; - alice1.recv_msg_hidden(&msg).await; - } - - // Check that the status is still the same. - assert_eq!( - alice1.get_config(Config::Selfstatus).await?.as_deref(), - Some("New status") - ); - Ok(()) - } - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_send_reaction_multidevice() -> Result<()> { let mut tcm = TestContextManager::new(); diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 185a2626d0..27e5cfe130 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -918,7 +918,7 @@ pub(crate) async fn receive_imf_inner( } if let Some(avatar_action) = &mime_parser.user_avatar { - if from_id != ContactId::UNDEFINED + if !matches!(from_id, ContactId::UNDEFINED | ContactId::SELF) && context .update_contacts_timestamp( from_id, @@ -927,23 +927,19 @@ pub(crate) async fn receive_imf_inner( ) .await? { - if let Err(err) = contact::set_profile_image( - context, - from_id, - avatar_action, - mime_parser.was_encrypted(), - ) - .await - { + if let Err(err) = contact::set_profile_image(context, from_id, avatar_action).await { warn!(context, "receive_imf cannot update profile image: {err:#}."); }; } } - // Ignore footers from mailinglists as they are often created or modified by the mailinglist software. - if let Some(footer) = &mime_parser.footer { + if let Some(footer) = mime_parser.footer.as_ref().filter(|footer| { + !footer.is_empty() || mime_parser.user_avatar.is_some() || !mime_parser.was_encrypted() + }) { + // Ignore footers from mailinglists as they are often created or modified by the mailinglist + // software. if !mime_parser.is_mailinglist_message() - && from_id != ContactId::UNDEFINED + && !matches!(from_id, ContactId::UNDEFINED | ContactId::SELF) && context .update_contacts_timestamp( from_id, @@ -952,15 +948,7 @@ pub(crate) async fn receive_imf_inner( ) .await? { - if let Err(err) = contact::set_status( - context, - from_id, - footer.to_string(), - mime_parser.was_encrypted(), - mime_parser.has_chat_version(), - ) - .await - { + if let Err(err) = contact::set_status(context, from_id, footer.clone()).await { warn!(context, "Cannot update contact status: {err:#}."); } } diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 7f777742a8..3dd2575a22 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -2254,6 +2254,41 @@ sig thursday", Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_contact_status_from_encrypted_msg() -> Result<()> { + let mut tcm = TestContextManager::new(); + let alice = &tcm.alice().await; + let bob = &tcm.bob().await; + + let alice_chat_id = alice.create_chat(bob).await.id; + let sent = alice.send_text(alice_chat_id, "hi").await; + bob.recv_msg(&sent).await; + alice.set_config(Config::Selfstatus, Some("status")).await?; + let sent = alice.send_text(alice_chat_id, "I've set self-status").await; + bob.recv_msg(&sent).await; + let bob_alice = bob.add_or_lookup_contact(alice).await; + assert_eq!(bob_alice.get_status(), "status"); + + alice + .set_config(Config::Selfstatus, Some("status1")) + .await?; + alice + .send_text(alice_chat_id, "I changed self-status") + .await; + + // Currently we send self-status in every appropriate message. + let sent = alice + .send_text(alice_chat_id, "This message also contains my status") + .await; + let parsed_msg = bob.parse_msg(&sent).await; + assert!(parsed_msg.was_encrypted()); + assert!(parsed_msg.get_header(HeaderDef::ChatUserAvatar).is_none()); + bob.recv_msg(&sent).await; + let bob_alice = bob.add_or_lookup_contact(alice).await; + assert_eq!(bob_alice.get_status(), "status1"); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn test_chat_assignment_private_classical_reply() { for outgoing_is_classical in &[true, false] {