Expire tombstones after 60 days

src/chat.rs

@@ -24,6 +24,7 @@ use crate::config::Config;
 use crate::constants::{
     self, Blocked, Chattype, DC_CHAT_ID_ALLDONE_HINT, DC_CHAT_ID_ARCHIVED_LINK,
     DC_CHAT_ID_LAST_SPECIAL, DC_CHAT_ID_TRASH, DC_RESEND_USER_AVATAR_DAYS,
+    TIMESTAMP_SENT_TOLERANCE,
 };
 use crate::contact::{self, Contact, ContactId, Origin};
 use crate::context::Context;
@@ -1962,6 +1963,34 @@ impl Chat {
         }
     }
 
+    /// Returns chat member list timestamp.
+    pub(crate) async fn member_list_timestamp(&self, context: &Context) -> Result<i64> {
+        if let Some(member_list_timestamp) = self.param.get_i64(Param::MemberListTimestamp) {
+            Ok(member_list_timestamp)
+        } else {
+            let creation_timestamp: i64 = context
+                .sql
+                .query_get_value("SELECT created_timestamp FROM chats WHERE id=?", (self.id,))
+                .await
+                .context("SQL error querying created_timestamp")?
+                .context("Chat not found")?;
+            Ok(creation_timestamp)
+        }
+    }
+
+    /// Returns true if member list is stale,
+    /// i.e. has not been updated for 60 days.
+    ///
+    /// This is used primarily to detect the case
+    /// where the user just restored an old backup.
+    pub(crate) async fn member_list_is_stale(&self, context: &Context) -> Result<bool> {
+        let now = time();
+        let member_list_ts = self.member_list_timestamp(context).await?;
+        let is_stale = now.saturating_add(TIMESTAMP_SENT_TOLERANCE)
+            >= member_list_ts.saturating_add(60 * 24 * 3600);
+        Ok(is_stale)
+    }
+
     /// Adds missing values to the msg object,
     /// writes the record to the database and returns its msg_id.
     ///
@@ -3468,16 +3497,19 @@ pub async fn get_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec
 
 /// Returns a vector of contact IDs for given chat ID that are no longer part of the group.
 pub async fn get_past_chat_contacts(context: &Context, chat_id: ChatId) -> Result<Vec<ContactId>> {
+    let now = time();
     let list = context
         .sql
         .query_map(
             "SELECT cc.contact_id
              FROM chats_contacts cc
              LEFT JOIN contacts c
                   ON c.id=cc.contact_id
-             WHERE cc.chat_id=? AND cc.add_timestamp < cc.remove_timestamp
+             WHERE cc.chat_id=?
+             AND cc.add_timestamp < cc.remove_timestamp
+             AND ? < cc.remove_timestamp
              ORDER BY c.id=1, c.last_seen DESC, c.id DESC",
-            (chat_id,),
+            (chat_id, now.saturating_sub(60 * 24 * 3600)),
             |row| row.get::<_, ContactId>(0),
             |ids| ids.collect::<Result<Vec<_>, _>>().map_err(Into::into),
         )

src/chat/chat_tests.rs

@@ -2,6 +2,7 @@ use super::*;
 use crate::chatlist::get_archived_cnt;
 use crate::constants::{DC_GCL_ARCHIVED_ONLY, DC_GCL_NO_SPECIALS};
 use crate::headerdef::HeaderDef;
+use crate::imex::{has_backup, imex, ImexMode};
 use crate::message::{delete_msgs, MessengerMessage};
 use crate::receive_imf::receive_imf;
 use crate::test_utils::{sync, TestContext, TestContextManager, TimeShiftFalsePositiveNote};
@@ -3365,3 +3366,160 @@ async fn unpromoted_group_no_tombstones() -> Result<()> {
 
     Ok(())
 }
+
+/// Test that past members expire after 60 days.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_expire_past_members_after_60_days() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+
+    let alice = &tcm.alice().await;
+    let fiona_addr = "[email protected]";
+    let alice_fiona_contact_id = Contact::create(alice, "Fiona", fiona_addr).await?;
+
+    let alice_chat_id =
+        create_group_chat(alice, ProtectionStatus::Unprotected, "Group chat").await?;
+    add_contact_to_chat(alice, alice_chat_id, alice_fiona_contact_id).await?;
+    alice
+        .send_text(alice_chat_id, "Hi! I created a group.")
+        .await;
+    remove_contact_from_chat(alice, alice_chat_id, alice_fiona_contact_id).await?;
+    assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 1);
+
+    SystemTime::shift(Duration::from_secs(60 * 24 * 60 * 60 + 1));
+    assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0);
+
+    let bob = &tcm.bob().await;
+    let bob_addr = bob.get_config(Config::Addr).await?.unwrap();
+    let alice_bob_contact_id = Contact::create(alice, "Bob", &bob_addr).await?;
+    add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?;
+
+    let add_message = alice.pop_sent_msg().await;
+    assert_eq!(add_message.payload.contains(fiona_addr), false);
+    let bob_add_message = bob.recv_msg(&add_message).await;
+    let bob_chat_id = bob_add_message.chat_id;
+    assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 2);
+    assert_eq!(get_past_chat_contacts(bob, bob_chat_id).await?.len(), 0);
+
+    Ok(())
+}
+
+/// Test the case when Alice restores a backup older than 60 days
+/// with outdated member list.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_restore_backup_after_60_days() -> Result<()> {
+    let backup_dir = tempfile::tempdir()?;
+
+    let mut tcm = TestContextManager::new();
+
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+    let fiona = &tcm.fiona().await;
+
+    let bob_addr = bob.get_config(Config::Addr).await?.unwrap();
+    let alice_bob_contact_id = Contact::create(alice, "Bob", &bob_addr).await?;
+
+    let charlie_addr = "[email protected]";
+    let alice_charlie_contact_id = Contact::create(alice, "Charlie", charlie_addr).await?;
+
+    let alice_chat_id =
+        create_group_chat(alice, ProtectionStatus::Unprotected, "Group chat").await?;
+    add_contact_to_chat(alice, alice_chat_id, alice_bob_contact_id).await?;
+    add_contact_to_chat(alice, alice_chat_id, alice_charlie_contact_id).await?;
+
+    let alice_sent_promote = alice
+        .send_text(alice_chat_id, "Hi! I created a group.")
+        .await;
+    let bob_rcvd_promote = bob.recv_msg(&alice_sent_promote).await;
+    let bob_chat_id = bob_rcvd_promote.chat_id;
+    bob_chat_id.accept(bob).await?;
+
+    // Alice exports a backup.
+    imex(alice, ImexMode::ExportBackup, backup_dir.path(), None).await?;
+
+    remove_contact_from_chat(alice, alice_chat_id, alice_charlie_contact_id).await?;
+    assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 2);
+    assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 1);
+
+    let remove_message = alice.pop_sent_msg().await;
+    assert_eq!(remove_message.payload.contains(charlie_addr), true);
+    bob.recv_msg(&remove_message).await;
+
+    // 60 days pass.
+    SystemTime::shift(Duration::from_secs(60 * 24 * 60 * 60 + 1));
+
+    assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0);
+
+    // Bob adds Fiona to the chat.
+    let fiona_addr = fiona.get_config(Config::Addr).await?.unwrap();
+    let bob_fiona_contact_id = Contact::create(bob, "Fiona", &fiona_addr).await?;
+    add_contact_to_chat(bob, bob_chat_id, bob_fiona_contact_id).await?;
+
+    let add_message = bob.pop_sent_msg().await;
+    alice.recv_msg(&add_message).await;
+    let fiona_add_message = fiona.recv_msg(&add_message).await;
+    let fiona_chat_id = fiona_add_message.chat_id;
+    fiona_chat_id.accept(fiona).await?;
+
+    // Fiona does not learn about Charlie,
+    // even from `Chat-Group-Past-Members`, because tombstone has expired.
+    assert_eq!(get_chat_contacts(fiona, fiona_chat_id).await?.len(), 3);
+    assert_eq!(get_past_chat_contacts(fiona, fiona_chat_id).await?.len(), 0);
+
+    // Fiona sends a message
+    // so chat is not stale for Bob again.
+    // Alice also receives the message,
+    // but will import a backup immediately afterwards,
+    // so it does not matter.
+    let fiona_sent_message = fiona.send_text(fiona_chat_id, "Hi!").await;
+    alice.recv_msg(&fiona_sent_message).await;
+    bob.recv_msg(&fiona_sent_message).await;
+
+    tcm.section("Alice imports old backup");
+    let alice = &tcm.unconfigured().await;
+    let backup = has_backup(alice, backup_dir.path()).await?;
+    imex(alice, ImexMode::ImportBackup, backup.as_ref(), None).await?;
+
+    // Alice thinks Charlie is in the chat, but does not know about Fiona.
+    assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 3);
+    assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0);
+
+    assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 3);
+    assert_eq!(get_past_chat_contacts(bob, bob_chat_id).await?.len(), 0);
+
+    assert_eq!(get_chat_contacts(fiona, fiona_chat_id).await?.len(), 3);
+    assert_eq!(get_past_chat_contacts(fiona, fiona_chat_id).await?.len(), 0);
+
+    // Bob sends a text message to the chat, without a tombstone for Charlie.
+    // Alice learns about Fiona.
+    let bob_sent_text = bob.send_text(bob_chat_id, "Message.").await;
+
+    tcm.section("Alice sends a message to stale chat");
+    let alice_sent_text = alice
+        .send_text(alice_chat_id, "Hi! I just restored a backup.")
+        .await;
+
+    tcm.section("Alice sent a message to stale chat");
+    alice.recv_msg(&bob_sent_text).await;
+    fiona.recv_msg(&bob_sent_text).await;
+
+    bob.recv_msg(&alice_sent_text).await;
+    fiona.recv_msg(&alice_sent_text).await;
+
+    // Alice should have learned about Charlie not being part of the group
+    // by receiving Bob's message.
+    assert_eq!(get_chat_contacts(alice, alice_chat_id).await?.len(), 3);
+    assert!(!is_contact_in_chat(alice, alice_chat_id, alice_charlie_contact_id).await?);
+    assert_eq!(get_past_chat_contacts(alice, alice_chat_id).await?.len(), 0);
+
+    // This should not add or restore Charlie for Bob and Fiona,
+    // Charlie is not part of the chat.
+    assert_eq!(get_chat_contacts(bob, bob_chat_id).await?.len(), 3);
+    assert_eq!(get_past_chat_contacts(bob, bob_chat_id).await?.len(), 0);
+    let bob_charlie_contact_id = Contact::create(bob, "Charlie", charlie_addr).await?;
+    assert!(!is_contact_in_chat(bob, bob_chat_id, bob_charlie_contact_id).await?);
+
+    assert_eq!(get_chat_contacts(fiona, fiona_chat_id).await?.len(), 3);
+    assert_eq!(get_past_chat_contacts(fiona, fiona_chat_id).await?.len(), 0);
+
+    Ok(())
+}

src/mimefactory.rs

@@ -155,6 +155,7 @@ fn new_address_with_name(name: &str, address: String) -> Address {
 
 impl MimeFactory {
     pub async fn from_msg(context: &Context, msg: Message) -> Result<MimeFactory> {
+        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 undisclosed_recipients = chat.typ == Chattype::Broadcast;
@@ -240,7 +241,7 @@ impl MimeFactory {
                                     }
                                 }
                                 recipient_ids.insert(id);
-                            } else {
+                            } else if remove_timestamp.saturating_add(60 * 24 * 3600) > now {
                                 // Row is a tombstone,
                                 // member is not actually part of the group.
                                 if !recipients_contain_addr(&past_members, &addr) {
@@ -621,7 +622,13 @@ impl MimeFactory {
             );
         }
 
-        if !self.member_timestamps.is_empty() {
+        let chat_memberlist_is_stale = if let Loaded::Message { chat, .. } = &self.loaded {
+            chat.member_list_is_stale(context).await?
+        } else {
+            false
+        };
+
+        if !self.member_timestamps.is_empty() && !chat_memberlist_is_stale {
             headers.push(
                 Header::new_with_value(
                     "Chat-Group-Member-Timestamps".into(),

src/param.rs

@@ -183,8 +183,6 @@ pub enum Param {
     GroupNameTimestamp = b'g',
 
     /// For Chats: timestamp of member list update.
-    ///
-    /// Deprecated 2025-01-07.
     MemberListTimestamp = b'k',
 
     /// For Webxdc Message Instances: Current document name

src/receive_imf.rs

@@ -2328,7 +2328,40 @@ async fn apply_group_changes(
     }
 
     if is_from_in_chat {
-        if let Some(ref chat_group_member_timestamps) = mime_parser.chat_group_member_timestamps() {
+        if chat.member_list_is_stale(context).await? {
+            info!(context, "Member list is stale.");
+            let mut new_members: HashSet<ContactId> = HashSet::from_iter(to_ids.iter().copied());
+            new_members.insert(ContactId::SELF);
+            if !from_id.is_special() {
+                new_members.insert(from_id);
+            }
+
+            context
+                .sql
+                .transaction(|transaction| {
+                    // Remove all contacts and tombstones.
+                    transaction.execute(
+                        "DELETE FROM chats_contacts
+                         WHERE chat_id=?",
+                        (chat_id,),
+                    )?;
+
+                    // Insert contacts with default timestamps of 0.
+                    let mut statement = transaction.prepare(
+                        "INSERT INTO chats_contacts (chat_id, contact_id)
+                         VALUES                     (?,       ?)",
+                    )?;
+                    for contact_id in &new_members {
+                        statement.execute((chat_id, contact_id))?;
+                    }
+
+                    Ok(())
+                })
+                .await?;
+            send_event_chat_modified = true;
+        } else if let Some(ref chat_group_member_timestamps) =
+            mime_parser.chat_group_member_timestamps()
+        {
             send_event_chat_modified |= update_chats_contacts_timestamps(
                 context,
                 chat_id,
@@ -2378,6 +2411,14 @@ async fn apply_group_changes(
                 send_event_chat_modified = true;
             }
         }
+
+        chat_id
+            .update_timestamp(
+                context,
+                Param::MemberListTimestamp,
+                mime_parser.timestamp_sent,
+            )
+            .await?;
     }
 
     let new_chat_contacts = HashSet::<ContactId>::from_iter(