Mirrors/chatmail-core
1
0
Fork 0
mirror of https://github.com/chatmail/core.git synced 2026-04-01 21:12:13 +03:00
Code Issues Packages Projects Releases Wiki Activity

fix: Switch to original Hocuri's group membership consistency algo (#3782)(#4624)

Browse Source 
- If we don't know the parent (=In-Reply-To) message, then completely recreate the group member list
  (i.e. use the member list of the incoming message) (because we assume that we missed some messages
  & have a wrong group state).
- If the message has a "Chat-Group-Member-Removed: member@example.com" header, then remove this
  member.
- If the message has a "Chat-Group-Member-Added: member@example.com" header, then add this member.

That means:
- Remove checks for the presense of `ContactId::SELF` in the group. Thus all recipients of a message
  take the same decision about group membership changes, no matter if they are in the group
  currently. This fixes a situation when a recipient thinks it's not a member because it missed a
  message about its addition before.
  NOTE: But always recreate membership list if SELF has been added. The older versions of DC don't
  always set "In-Reply-To" to the latest message they sent, but to the latest delivered message (so
  it's a race), so we need this heuristic currently.
- Recreate the group member list if we don't know the parent (=In-Reply-To) message, even if the
  sender isn't in the group as per our view, because we missed some messages and our view may be
  stale.
This commit is contained in:
iequidoo
2023-08-31 16:04:34 -03:00
committed by iequidoo
parent 212fbc125c
commit e12e026bd8
2 changed files with 104 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files

103
src/receive_imf.rs
View File

@@ -1654,40 +1654,27 @@ async fn apply_group_changes(
false
};
// Whether to allow any changes to the member list at all.
let allow_member_list_changes = if chat::is_contact_in_chat(context, chat_id, ContactId::SELF)
.await?
|| self_added
|| !mime_parser.has_chat_version()
{
// Reject old group changes.
chat_id
.update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
.await?
} else {
// Member list changes are not allowed if we're not in the group
// and are not explicitly added.
// This message comes from a Delta Chat that restored an old backup
// or the message is a MUA reply to an old message.
false
};
// Whether to allow any changes to the member list at all. Just reject old group changes.
let allow_member_list_changes = chat_id
.update_timestamp(context, Param::MemberListTimestamp, sent_timestamp)
.await?;
let is_from_in_chat = chat::is_contact_in_chat(context, chat_id, from_id).await?;
// Whether to rebuild the member list from scratch.
let recreate_member_list = if allow_member_list_changes {
let recreate_member_list = allow_member_list_changes && {
// Recreate member list if the message comes from a MUA as these messages do _not_ set add/remove headers.
// Always recreate membership list if self has been added.
if !mime_parser.has_chat_version() || self_added {
true
} else {
match mime_parser.get_header(HeaderDef::InReplyTo) {
(is_from_in_chat && !mime_parser.has_chat_version())
// Always recreate membership list if SELF has been added. The older versions of DC
// don't always set "In-Reply-To" to the latest message they sent, but to the latest
// delivered message (so it's a race), so we have this heuristic here.
|| self_added
|| 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(),
None => false,
}
}
} else {
false
};
if let Some(removed_addr) = mime_parser.get_header(HeaderDef::ChatGroupMemberRemoved) {
@@ -1794,43 +1781,41 @@ async fn apply_group_changes(
// Recreate the member list.
if recreate_member_list {
if !chat::is_contact_in_chat(context, chat_id, from_id).await? {
if !is_from_in_chat {
warn!(
context,
"Contact {from_id} attempts to modify group chat {chat_id} member list without being a member."
"Contact {from_id} modifies group chat {chat_id} member list possibly not being a member."
);
} else {
// Only delete old contacts if the sender is not a classical MUA user:
// Classical MUA users usually don't intend to remove users from an email
// thread, so if they removed a recipient then it was probably by accident.
if mime_parser.has_chat_version() {
context
.sql
.execute("DELETE FROM chats_contacts WHERE chat_id=?;", (chat_id,))
.await?;
}
let mut members_to_add = HashSet::new();
members_to_add.extend(to_ids);
members_to_add.insert(ContactId::SELF);
if !from_id.is_special() {
members_to_add.insert(from_id);
}
if let Some(removed_id) = removed_id {
members_to_add.remove(&removed_id);
}
info!(
context,
"Recreating chat {chat_id} with members {members_to_add:?}."
);
chat::add_to_chat_contacts_table(context, chat_id, &Vec::from_iter(members_to_add))
.await?;
send_event_chat_modified = true;
}
// Only delete old contacts if the sender is not a classical MUA user:
// Classical MUA users usually don't intend to remove users from an email
// thread, so if they removed a recipient then it was probably by accident.
if mime_parser.has_chat_version() {
context
.sql
.execute("DELETE FROM chats_contacts WHERE chat_id=?;", (chat_id,))
.await?;
}
let mut members_to_add = HashSet::new();
members_to_add.extend(to_ids);
members_to_add.insert(ContactId::SELF);
if !from_id.is_special() {
members_to_add.insert(from_id);
}
if let Some(removed_id) = removed_id {
members_to_add.remove(&removed_id);
}
info!(
context,
"Recreating chat {chat_id} with members {members_to_add:?}."
);
chat::add_to_chat_contacts_table(context, chat_id, &Vec::from_iter(members_to_add)).await?;
send_event_chat_modified = true;
}
if let Some(avatar_action) = &mime_parser.group_avatar {

60
src/receive_imf/tests.rs
View File

@@ -8,6 +8,7 @@ use crate::chat::{
};
use crate::chat::{get_chat_msgs, ChatItem, ChatVisibility};
use crate::chatlist::Chatlist;
use crate::config::Config;
use crate::constants::{DC_GCL_FOR_FORWARDING, DC_GCL_NO_SPECIALS};
use crate::imap::prefetch_should_download;
use crate::message::Message;
@@ -3611,3 +3612,62 @@ async fn test_mua_can_readd() -> Result<()> {
assert!(is_contact_in_chat(&alice, alice_chat.id, ContactId::SELF).await?);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_recreate_member_list_on_missing_add_of_self() -> 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.get_config(Config::Addr).await?.unwrap()).await?,
)
.await?;
send_text_msg(&alice, alice_chat_id, "populate".to_string()).await?;
alice.pop_sent_msg().await;
remove_contact_from_chat(&alice, alice_chat_id, ContactId::SELF).await?;
let bob_chat_id = bob.recv_msg(&alice.pop_sent_msg().await).await.chat_id;
// Bob missed the message adding them, but must recreate the member list.
assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 1);
assert!(is_contact_in_chat(&bob, bob_chat_id, ContactId::SELF).await?);
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn test_recreate_member_list_on_missing_add_of_others() -> 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.get_config(Config::Addr).await?.unwrap()).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;
let fiona = TestContext::new_fiona().await;
add_contact_to_chat(
&alice,
alice_chat_id,
Contact::create(
&alice,
"fiona",
&fiona.get_config(Config::Addr).await?.unwrap(),
)
.await?,
)
.await?;
let fiona_chat_id = fiona.recv_msg(&alice.pop_sent_msg().await).await.chat_id;
fiona_chat_id.accept(&fiona).await?;
send_text_msg(&fiona, fiona_chat_id, "hi".to_string()).await?;
bob.recv_msg(&fiona.pop_sent_msg().await).await;
// Bob missed the message adding fiona, but must recreate the member list.
assert_eq!(get_chat_contacts(&bob, bob_chat_id).await?.len(), 3);
Ok(())
}