diff --git a/src/receive_imf.rs b/src/receive_imf.rs index b5652bfdd..526e43f87 100644 --- a/src/receive_imf.rs +++ b/src/receive_imf.rs @@ -1638,47 +1638,35 @@ async fn apply_group_changes( .get_header(HeaderDef::ChatGroupMemberAdded) .is_some() { - apply_group_changes = chat_id - .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) - .await?; + let self_added = + if let Some(added_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) { + if let Ok(self_addr) = context.get_primary_self_addr().await { + self_addr == *added_addr + } else { + false + } + } else { + false + }; + + if !chat::is_contact_in_chat(context, chat_id, ContactId::SELF).await? && !self_added { + false + } else { + apply_group_changes = chat_id + .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) + .await?; - if apply_group_changes { match mime_parser.get_header(HeaderDef::InReplyTo) { // If we don't know the referenced message we, we missed some messages which could be add/delete Some(reply_to) if rfc724_mid_exists(context, reply_to).await?.is_none() => true, - Some(_) => { - // recreate chat if we are getting readded - if let Some(added_addr) = - mime_parser.get_header(HeaderDef::ChatGroupMemberAdded) - { - if let Ok(self_addr) = context.get_primary_self_addr().await { - self_addr == *added_addr - } else { - false - } - } else { - false - } - } + Some(_) => self_added, None => false, } - } else { - false } } else { false }; - // Always use the MUA users contact list if message is up to date - if !mime_parser.has_chat_version() - && chat_id - .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) - .await? - { - recreate_member_list = true; - apply_group_changes = true; - } - if let Some(removed_addr) = mime_parser .get_header(HeaderDef::ChatGroupMemberRemoved) .cloned() @@ -1703,13 +1691,6 @@ async fn apply_group_changes( { better_msg = Some(stock_str::msg_add_member(context, &added_addr, from_id).await); - // If we got added to the group ourselves, we have to recreate the memberlist - if let Some(self_addr) = context.get_config(Config::Addr).await? { - if self_addr == added_addr { - recreate_member_list = true; - } - }; - // add a single member to the chat if !recreate_member_list && apply_group_changes { if let Some(contact_id) = @@ -1775,14 +1756,21 @@ async fn apply_group_changes( chat_id .inner_set_protection(context, ProtectionStatus::Protected) .await?; - recreate_member_list = true; } } + // Recreate member list if message is from a MUA + if !mime_parser.has_chat_version() + && chat_id + .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp) + .await? + { + recreate_member_list = true; + } + // recreate member list - if recreate_member_list && apply_group_changes { - if chat::is_contact_in_chat(context, chat_id, ContactId::SELF).await? - && !chat::is_contact_in_chat(context, chat_id, from_id).await? + if recreate_member_list { + if !chat::is_contact_in_chat(context, chat_id, from_id).await? { warn!( context, @@ -1791,14 +1779,16 @@ async fn apply_group_changes( chat_id ); } else { - // clear chat contacts - context - .sql - .execute( - "DELETE FROM chats_contacts WHERE chat_id=?;", - paramsv![chat_id], - ) - .await?; + // only delete old contacts if the sender is not a MUA + if mime_parser.has_chat_version() { + context + .sql + .execute( + "DELETE FROM chats_contacts WHERE chat_id=?;", + paramsv![chat_id], + ) + .await?; + } let mut members_to_add = HashSet::new(); members_to_add.extend(to_ids); diff --git a/src/receive_imf/tests.rs b/src/receive_imf/tests.rs index c026a4811..a4e8c8793 100644 --- a/src/receive_imf/tests.rs +++ b/src/receive_imf/tests.rs @@ -2,7 +2,10 @@ use tokio::fs; use super::*; use crate::aheader::EncryptPreference; -use crate::chat::get_chat_contacts; +use crate::chat::{ + add_contact_to_chat, add_to_chat_contacts_table, create_group_chat, get_chat_contacts, + is_contact_in_chat, remove_contact_from_chat, send_text_msg, +}; use crate::chat::{get_chat_msgs, ChatItem, ChatVisibility}; use crate::chatlist::Chatlist; use crate::constants::DC_GCL_NO_SPECIALS; @@ -3245,3 +3248,182 @@ async fn test_mua_user_adds_recipient_to_single_chat() -> Result<()> { Ok(()) } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_dont_rebuild_contacts_on_add_remove() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + + let alice_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "Group").await?; + + add_contact_to_chat( + &alice, + alice_chat_id, + Contact::create(&alice, "bob", "bob@example.net").await?, + ) + .await?; + + send_text_msg(&alice, alice_chat_id, "populate".to_string()).await?; + let bob_chat_id = bob.recv_msg(&alice.pop_sent_msg().await).await.chat_id; + bob_chat_id.accept(&bob).await?; + + // alice adds a member + add_contact_to_chat( + &alice, + alice_chat_id, + Contact::create(&alice, "fiona", "fiona@example.net").await?, + ) + .await?; + + // bob adds a member + let bob_blue = Contact::create(&bob, "blue", "blue@example.net").await?; + add_contact_to_chat(&bob, bob_chat_id, bob_blue).await?; + + alice.recv_msg(&bob.pop_sent_msg().await).await; + + // We don't rebuild the contactlist of ours just because we received a new addition + assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 4); + + // alice adds a member + add_contact_to_chat( + &alice, + alice_chat_id, + Contact::create(&alice, "daisy", "daisy@example.net").await?, + ) + .await?; + + // bob removes a member + remove_contact_from_chat(&bob, bob_chat_id, bob_blue).await?; + + alice.recv_msg(&bob.pop_sent_msg().await).await; + + // We don't rebuild the contactlist of ours just because we received a new removal + assert_eq!(get_chat_contacts(&alice, alice_chat_id).await?.len(), 4); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_rebuild_contact_list_on_missing_message() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + + let chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "Group").await?; + + // create chat with three members + add_to_chat_contacts_table( + &alice, + chat_id, + &[ + Contact::create(&alice, "bob", "bob@example.net").await?, + Contact::create(&alice, "fiona", "fiona@example.net").await?, + ], + ) + .await?; + + send_text_msg(&alice, chat_id, "populate".to_string()).await?; + let bob_chat_id = bob.recv_msg(&alice.pop_sent_msg().await).await.chat_id; + bob_chat_id.accept(&bob).await?; + + // bob removes a member + let bob_contact_fiona = Contact::create(&bob, "fiona", "fiona@example.net").await?; + remove_contact_from_chat(&bob, bob_chat_id, bob_contact_fiona).await?; + bob.pop_sent_msg().await; + + // bob ads a new member + let bob_blue = Contact::create(&bob, "blue", "blue@example.net").await?; + add_contact_to_chat(&bob, bob_chat_id, bob_blue).await?; + + let add_msg = bob.pop_sent_msg().await; + + // alice only receives the addition of the member + alice.recv_msg(&add_msg).await; + + // since we missed a message, a new contact list should be build + assert_eq!(get_chat_contacts(&alice, chat_id).await?.len(), 3); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_dont_readd_with_normal_msg() -> Result<()> { + let alice = TestContext::new_alice().await; + let bob = TestContext::new_bob().await; + + let alice_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "Group").await?; + + add_contact_to_chat( + &alice, + alice_chat_id, + Contact::create(&alice, "bob", "bob@example.net").await?, + ) + .await?; + + send_text_msg(&alice, alice_chat_id, "populate".to_string()).await?; + let bob_chat_id = bob.recv_msg(&alice.pop_sent_msg().await).await.chat_id; + bob_chat_id.accept(&bob).await?; + + remove_contact_from_chat(&bob, bob_chat_id, ContactId::SELF).await?; + assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 1); + + add_contact_to_chat( + &alice, + alice_chat_id, + Contact::create(&alice, "fiora", "fiora@example.net").await?, + ) + .await?; + + bob.recv_msg(&alice.pop_sent_msg().await).await; + + // even though the received message contains a header for member addition, + // you are not added to the group + assert!(!is_contact_in_chat(&bob, bob_chat_id, ContactId::SELF).await?); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn test_mua_cant_remove() -> Result<()> { + let alice = TestContext::new_alice().await; + + // Alice creates chat with 2 contacts + let msg = receive_imf( + &alice, + b"Subject: =?utf-8?q?Message_from_alice=40example=2Eorg?=\r\n\ + From: alice@example.org\r\n\ + To: , , \r\n\ + Date: Mon, 12 Dec 2022 14:30:39 +0000\r\n\ + Message-ID: \r\n\ + Chat-Version: 1.0\r\n\ + \r\n\ + tst\r\n", + false, + ) + .await? + .unwrap(); + let single_chat = Chat::load_from_db(&alice, msg.chat_id).await?; + assert_eq!(single_chat.typ, Chattype::Group); + + // Bob uses a classical MUA to answer again, removing a receipient. + let msg3 = receive_imf( + &alice, + b"Subject: Re: Message from alice\r\n\ + From: \r\n\ + To: , \r\n\ + Date: Mon, 12 Dec 2022 14:32:39 +0000\r\n\ + Message-ID: \r\n\ + In-Reply-To: \r\n\ + \r\n\ + Hi back!\r\n", + false, + ) + .await? + .unwrap(); + assert_eq!(msg3.chat_id, single_chat.id); + let group_chat = Chat::load_from_db(&alice, msg3.chat_id).await?; + assert_eq!(group_chat.typ, Chattype::Group); + assert_eq!( + chat::get_chat_contacts(&alice, group_chat.id).await?.len(), + 4 + ); + Ok(()) +}