From f33e21ccb9f3a115b7d73a458f96c93c789542cc Mon Sep 17 00:00:00 2001 From: link2xt Date: Mon, 6 Apr 2026 04:46:30 +0200 Subject: [PATCH] fix: trash message about group name change from non-member --- src/receive_imf.rs | 129 ++++++++++++++++----------- src/receive_imf/receive_imf_tests.rs | 4 +- 2 files changed, 81 insertions(+), 52 deletions(-) diff --git a/src/receive_imf.rs b/src/receive_imf.rs index 2438cfed6..efce9d9eb 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -14,7 +14,9 @@ use mailparse::SingleInfo; use num_traits::FromPrimitive; use regex::Regex; -use crate::chat::{self, Chat, ChatId, ChatIdBlocked, ChatVisibility, save_broadcast_secret}; +use crate::chat::{ + self, Chat, ChatId, ChatIdBlocked, ChatVisibility, is_contact_in_chat, save_broadcast_secret, +}; use crate::config::Config; use crate::constants::{self, Blocked, Chattype, DC_CHAT_ID_TRASH, EDITED_PREFIX, ShowEmails}; use crate::contact::{self, Contact, ContactId, Origin, mark_contact_id_as_verified}; @@ -3153,17 +3155,18 @@ async fn apply_group_changes( } } - if is_from_in_chat { - apply_chat_name_avatar_and_description_changes( - context, - mime_parser, - from_id, - chat, - &mut send_event_chat_modified, - &mut better_msg, - ) - .await?; + apply_chat_name_avatar_and_description_changes( + context, + mime_parser, + from_id, + is_from_in_chat, + chat, + &mut send_event_chat_modified, + &mut better_msg, + ) + .await?; + if is_from_in_chat { // Avoid insertion of `from_id` into a group with inappropriate encryption state. if from_is_key_contact != chat.grpid.is_empty() && chat.member_list_is_stale(context).await? @@ -3337,6 +3340,7 @@ async fn apply_chat_name_avatar_and_description_changes( context: &Context, mime_parser: &MimeMessage, from_id: ContactId, + is_from_in_chat: bool, chat: &mut Chat, send_event_chat_modified: &mut bool, better_msg: &mut Option, @@ -3365,7 +3369,8 @@ async fn apply_chat_name_avatar_and_description_changes( let chat_group_name_timestamp = chat.param.get_i64(Param::GroupNameTimestamp).unwrap_or(0); let group_name_timestamp = group_name_timestamp.unwrap_or(mime_parser.timestamp_sent); // To provide group name consistency, compare names if timestamps are equal. - if (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name) + if is_from_in_chat + && (chat_group_name_timestamp, grpname) < (group_name_timestamp, &chat.name) && chat .id .update_timestamp(context, Param::GroupNameTimestamp, group_name_timestamp) @@ -3386,14 +3391,19 @@ async fn apply_chat_name_avatar_and_description_changes( .get_header(HeaderDef::ChatGroupNameChanged) .is_some() { - let old_name = &sanitize_single_line(old_name); - better_msg.get_or_insert( - if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) { - stock_str::msg_broadcast_name_changed(context, old_name, grpname) - } else { - stock_str::msg_grp_name(context, old_name, grpname, from_id).await - }, - ); + if is_from_in_chat { + let old_name = &sanitize_single_line(old_name); + better_msg.get_or_insert( + if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) { + stock_str::msg_broadcast_name_changed(context, old_name, grpname) + } else { + stock_str::msg_grp_name(context, old_name, grpname, from_id).await + }, + ); + } else { + // Attempt to change group name by non-member, trash it. + *better_msg = Some(String::new()); + } } } @@ -3416,7 +3426,8 @@ async fn apply_chat_name_avatar_and_description_changes( let new_timestamp = timestamp_in_header.unwrap_or(mime_parser.timestamp_sent); // To provide consistency, compare descriptions if timestamps are equal. - if (old_timestamp, &old_description) < (new_timestamp, &new_description) + if is_from_in_chat + && (old_timestamp, &old_description) < (new_timestamp, &new_description) && chat .id .update_timestamp(context, Param::GroupDescriptionTimestamp, new_timestamp) @@ -3437,8 +3448,13 @@ async fn apply_chat_name_avatar_and_description_changes( .get_header(HeaderDef::ChatGroupDescriptionChanged) .is_some() { - better_msg - .get_or_insert(stock_str::msg_chat_description_changed(context, from_id).await); + if is_from_in_chat { + better_msg + .get_or_insert(stock_str::msg_chat_description_changed(context, from_id).await); + } else { + // Attempt to change group description by non-member, trash it. + *better_msg = Some(String::new()); + } } } @@ -3448,39 +3464,46 @@ async fn apply_chat_name_avatar_and_description_changes( && value == "group-avatar-changed" && let Some(avatar_action) = &mime_parser.group_avatar { - // this is just an explicit message containing the group-avatar, - // apart from that, the group-avatar is send along with various other messages - better_msg.get_or_insert( - if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) { - stock_str::msg_broadcast_img_changed(context) - } else { - match avatar_action { - AvatarAction::Delete => stock_str::msg_grp_img_deleted(context, from_id).await, - AvatarAction::Change(_) => { - stock_str::msg_grp_img_changed(context, from_id).await + if is_from_in_chat { + // this is just an explicit message containing the group-avatar, + // apart from that, the group-avatar is send along with various other messages + better_msg.get_or_insert( + if matches!(chat.typ, Chattype::InBroadcast | Chattype::OutBroadcast) { + stock_str::msg_broadcast_img_changed(context) + } else { + match avatar_action { + AvatarAction::Delete => { + stock_str::msg_grp_img_deleted(context, from_id).await + } + AvatarAction::Change(_) => { + stock_str::msg_grp_img_changed(context, from_id).await + } } - } - }, - ); + }, + ); + } else { + // Attempt to change group avatar by non-member, trash it. + *better_msg = Some(String::new()); + } } - if let Some(avatar_action) = &mime_parser.group_avatar { - info!(context, "Group-avatar change for {}.", chat.id); - if chat + if let Some(avatar_action) = &mime_parser.group_avatar + && is_from_in_chat + && chat .param .update_timestamp(Param::AvatarTimestamp, mime_parser.timestamp_sent)? - { - 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; - } + { + info!(context, "Group-avatar change for {}.", chat.id); + 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; } Ok(()) @@ -3787,10 +3810,12 @@ async fn apply_out_broadcast_changes( let mut added_removed_id: Option = None; if from_id == ContactId::SELF { + let is_from_in_chat = true; apply_chat_name_avatar_and_description_changes( context, mime_parser, from_id, + is_from_in_chat, chat, &mut send_event_chat_modified, &mut better_msg, @@ -3879,10 +3904,12 @@ async fn apply_in_broadcast_changes( let mut send_event_chat_modified = false; let mut better_msg = None; + let is_from_in_chat = is_contact_in_chat(context, chat.id, from_id).await?; apply_chat_name_avatar_and_description_changes( context, mime_parser, from_id, + is_from_in_chat, chat, &mut send_event_chat_modified, &mut better_msg, diff --git a/src/receive_imf/receive_imf_tests.rs b/src/receive_imf/receive_imf_tests.rs index 4e105c007..dd3ea1203 100644 --- a/src/receive_imf/receive_imf_tests.rs +++ b/src/receive_imf/receive_imf_tests.rs @@ -4403,7 +4403,9 @@ async fn test_keep_member_list_if_possibly_nomember() -> Result<()> { SystemTime::shift(Duration::from_secs(60)); chat::set_chat_name(fiona, fiona_chat_id, "Renamed").await?; - bob.recv_msg(&fiona.pop_sent_msg().await).await; + + // Message about chat name change from non-member is trashed. + bob.recv_msg_trash(&fiona.pop_sent_msg().await).await; // Bob missed the message adding fiona, but mustn't recreate the member list or apply the group // name change.