fix: Partial messages do not change group state (#4900)

This message makes that partial messages do not change the group state.
A simple fix and a comprehensive test is added. This is a follow up to
the former #4841 which took a different approach.

src/message.rs

@@ -1818,6 +1818,14 @@ pub async fn estimate_deletion_cnt(
 pub(crate) async fn rfc724_mid_exists(
     context: &Context,
     rfc724_mid: &str,
+) -> Result<Option<MsgId>> {
+    rfc724_mid_exists_and(context, rfc724_mid, "1").await
+}
+
+pub(crate) async fn rfc724_mid_exists_and(
+    context: &Context,
+    rfc724_mid: &str,
+    cond: &str,
 ) -> Result<Option<MsgId>> {
     let rfc724_mid = rfc724_mid.trim_start_matches('<').trim_end_matches('>');
     if rfc724_mid.is_empty() {
@@ -1828,7 +1836,7 @@ pub(crate) async fn rfc724_mid_exists(
     let res = context
         .sql
         .query_row_optional(
-            "SELECT id FROM msgs WHERE rfc724_mid=?",
+            &("SELECT id FROM msgs WHERE rfc724_mid=? AND ".to_string() + cond),
             (rfc724_mid,),
             |row| {
                 let msg_id: MsgId = row.get(0)?;

src/mimeparser.rs

@@ -59,7 +59,7 @@ pub(crate) struct MimeMessage {
     /// Message headers.
     headers: HashMap<String, String>,
 
-    /// Addresses are normalized and lowercased:
+    /// Addresses are normalized and lowercase
     pub recipients: Vec<SingleInfo>,
 
     /// `From:` address.

src/receive_imf.rs

@@ -26,7 +26,8 @@ use crate::imap::{markseen_on_imap_table, GENERATED_PREFIX};
 use crate::location;
 use crate::log::LogExt;
 use crate::message::{
-    self, rfc724_mid_exists, Message, MessageState, MessengerMessage, MsgId, Viewtype,
+    self, rfc724_mid_exists, rfc724_mid_exists_and, Message, MessageState, MessengerMessage, MsgId,
+    Viewtype,
 };
 use crate::mimeparser::{parse_message_ids, AvatarAction, MimeMessage, SystemMessage};
 use crate::param::{Param, Params};
@@ -503,7 +504,6 @@ async fn add_parts(
     // - incoming messages introduce a chat only for known contacts if they are sent by a messenger
     // (of course, the user can add other chats manually later)
     let to_id: ContactId;
-
     let state: MessageState;
     let mut needs_delete_job = false;
     if incoming {
@@ -656,6 +656,7 @@ async fn add_parts(
                 group_chat_id,
                 from_id,
                 to_ids,
+                is_partial_download.is_some(),
             )
             .await?);
         }
@@ -898,6 +899,7 @@ async fn add_parts(
                 chat_id,
                 from_id,
                 to_ids,
+                is_partial_download.is_some(),
             )
             .await?);
         }
@@ -1693,6 +1695,7 @@ async fn create_or_lookup_group(
 /// Apply group member list, name, avatar and protection status changes from the MIME message.
 ///
 /// Optionally returns better message to replace the original system message.
+/// is_partial_download: whether the message is not fully downloaded.
 async fn apply_group_changes(
     context: &Context,
     mime_parser: &mut MimeMessage,
@@ -1700,6 +1703,7 @@ async fn apply_group_changes(
     chat_id: ChatId,
     from_id: ContactId,
     to_ids: &[ContactId],
+    is_partial_download: bool,
 ) -> Result<Option<String>> {
     let mut chat = Chat::load_from_db(context, chat_id).await?;
     if chat.typ != Chattype::Group {
@@ -1724,7 +1728,8 @@ async fn apply_group_changes(
         !chat_contacts.contains(&ContactId::SELF) || chat_contacts.contains(&from_id);
 
     // Reject group membership changes from non-members and old changes.
-    let allow_member_list_changes = is_from_in_chat
+    let allow_member_list_changes = !is_partial_download
+        && is_from_in_chat
         && chat_id
             .update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
             .await?;
@@ -1738,7 +1743,9 @@ async fn apply_group_changes(
             || match mime_parser.get_header(HeaderDef::InReplyTo) {
                 // If we don't know the referenced message, we missed some messages.
                 // Maybe they added/removed members, so we need to recreate our member list.
-                Some(reply_to) => rfc724_mid_exists(context, reply_to).await?.is_none(),
+                Some(reply_to) => rfc724_mid_exists_and(context, reply_to, "download_state=0")
+                    .await?
+                    .is_none(),
                 None => false,
             }
     } && {

src/receive_imf/tests.rs

@@ -3863,3 +3863,113 @@ async fn test_create_group_with_big_msg() -> Result<()> {
 
     Ok(())
 }
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_partial_group_consistency() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = tcm.alice().await;
+    let bob_id = Contact::create(&alice, "", "bob@example.net").await?;
+    let alice_chat_id = create_group_chat(&alice, ProtectionStatus::Unprotected, "foos").await?;
+    add_contact_to_chat(&alice, alice_chat_id, bob_id).await?;
+
+    send_text_msg(&alice, alice_chat_id, "populate".to_string()).await?;
+    let add = alice.pop_sent_msg().await;
+    let bob = tcm.bob().await;
+    bob.recv_msg(&add).await;
+    let bob_chat_id = bob.get_last_msg().await.chat_id;
+    let contacts = get_chat_contacts(&bob, bob_chat_id).await?;
+    assert_eq!(contacts.len(), 2);
+
+    // Get initial timestamp.
+    let timestamp = bob_chat_id
+        .get_param(&bob)
+        .await?
+        .get_i64(Param::MemberListTimestamp)
+        .unwrap();
+
+    // Bob receives partial message.
+    let msg_id = receive_imf_inner(
+        &bob,
+        "first@example.org",
+        b"From: Alice <alice@example.org>\n\
+To: <bob@example.net>, <charlie@example.com>\n\
+Chat-Version: 1.0\n\
+Subject: subject\n\
+Message-ID: <first@example.org>\n\
+Date: Sun, 14 Nov 2021 00:10:00 +0000\
+Content-Type: text/plain
+Chat-Group-Member-Added: charlie@example.com",
+        false,
+        Some(100000),
+        false,
+    )
+    .await?
+    .context("no received message")?;
+
+    let msg = Message::load_from_db(&bob, msg_id.msg_ids[0]).await?;
+    let timestamp2 = bob_chat_id
+        .get_param(&bob)
+        .await?
+        .get_i64(Param::MemberListTimestamp)
+        .unwrap();
+
+    // Partial download does not change the member list.
+    assert_eq!(msg.download_state, DownloadState::Available);
+    assert_eq!(timestamp, timestamp2);
+    assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?, contacts);
+
+    // Alice sends normal message to bob, adding fiona.
+    add_contact_to_chat(
+        &alice,
+        alice_chat_id,
+        Contact::create(&alice, "fiona", "fiona@example.net").await?,
+    )
+    .await?;
+
+    bob.recv_msg(&alice.pop_sent_msg().await).await;
+
+    let timestamp3 = bob_chat_id
+        .get_param(&bob)
+        .await?
+        .get_i64(Param::MemberListTimestamp)
+        .unwrap();
+
+    // Receiving a message after a partial download recreates the member list because we treat
+    // such messages as if we have not seen them.
+    assert_ne!(timestamp, timestamp3);
+    let contacts = get_chat_contacts(&bob, bob_chat_id).await?;
+    assert_eq!(contacts.len(), 3);
+
+    // Bob fully reives the partial message.
+    let msg_id = receive_imf_inner(
+        &bob,
+        "first@example.org",
+        b"From: Alice <alice@example.org>\n\
+To: Bob <bob@example.net>\n\
+Chat-Version: 1.0\n\
+Subject: subject\n\
+Message-ID: <first@example.org>\n\
+Date: Sun, 14 Nov 2021 00:10:00 +0000\
+Content-Type: text/plain
+Chat-Group-Member-Added: charlie@example.com",
+        false,
+        None,
+        false,
+    )
+    .await?
+    .context("no received message")?;
+
+    let msg = Message::load_from_db(&bob, msg_id.msg_ids[0]).await?;
+    let timestamp4 = bob_chat_id
+        .get_param(&bob)
+        .await?
+        .get_i64(Param::MemberListTimestamp)
+        .unwrap();
+
+    // After full download, the old message should not change group state.
+    assert_eq!(msg.download_state, DownloadState::Done);
+    assert_eq!(timestamp3, timestamp4);
+    assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?, contacts);
+
+    Ok(())
+}