From 1c282c01d1b29aac588436d508ca7e6bb0d5d358 Mon Sep 17 00:00:00 2001 From: iequidoo Date: Mon, 18 Aug 2025 10:30:33 -0300 Subject: [PATCH] feat: Don't reset key-contact status if Chat-User-Avatar header is absent (#7002) This prepares for sending self-status only together with self-avatar in encrypted messages. The idea is that self-status normally doesn't change frequently, so it's not a problem to re-send the whole profile. Self-status is rather a biography, it even goes to "NOTE:" in vCards, so it's not a contact status at a particular moment like "online" or "busy", and to see it one should go to the contact profile. Don't check for "Chat-Version" header though. So if a non- Delta Chat key-contact removes footer, its "status" remains, but this shouldn't be a problem. For unencrypted messages self-status will still be always attached except MDNs, reactions and SecureJoin messages, so that it's visible as the message footer in other MUAs. --- src/chat.rs | 10 ++++---- src/chat/chat_tests.rs | 12 +++++----- src/config.rs | 10 ++++++++ src/headerdef.rs | 5 ++++ src/mimefactory.rs | 26 ++++++++++++--------- src/receive_imf.rs | 9 ++++--- src/receive_imf/receive_imf_tests.rs | 35 ++++++++++++++++++++++++++++ 7 files changed, 82 insertions(+), 25 deletions(-) diff --git a/src/chat.rs b/src/chat.rs index 0526eed3d..570f5a01a 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -3719,12 +3719,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 @@ -3739,8 +3739,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 45c228834..fff255e7f 100644 --- a/src/chat/chat_tests.rs +++ b/src/chat/chat_tests.rs @@ -1531,23 +1531,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(alice, "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(()) } @@ -1571,7 +1571,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 9f071c85d..4b992a9fe 100644 --- a/src/config.rs +++ b/src/config.rs @@ -758,6 +758,16 @@ impl Context { let better_value; match key { + Config::Selfstatus => { + // Currently we send the self-status in every appropriate message, but in the future + // (when most users upgrade to "feat: Don't reset key-contact status if + // Chat-User-Avatar header is absent") we want to send it periodically together with + // the self-avatar. This ensures the correct behavior after a possible Core upgrade. + 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/headerdef.rs b/src/headerdef.rs index 8102bcb81..c4a8fae9c 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 a0517979b..672339384 100644 --- a/src/mimefactory.rs +++ b/src/mimefactory.rs @@ -182,7 +182,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?; @@ -194,7 +194,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(), }; @@ -302,7 +302,7 @@ impl MimeFactory { } else { addr }; - let name = match attach_profile_data { + let name = match can_transfer_profile { true => authname, false => "".to_string(), }; @@ -451,14 +451,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() @@ -551,7 +555,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: @@ -566,14 +570,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 } @@ -638,7 +642,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/receive_imf.rs b/src/receive_imf.rs index 100c2874b..7239399b8 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -908,8 +908,11 @@ pub(crate) async fn receive_imf_inner( } } - // 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 && context @@ -923,7 +926,7 @@ pub(crate) async fn receive_imf_inner( if let Err(err) = contact::set_status( context, from_id, - footer.to_string(), + footer.clone(), mime_parser.was_encrypted(), mime_parser.has_chat_version(), ) diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index f70c9fc81..893222264 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] {