diff --git a/python/tests/test_account.py b/python/tests/test_account.py index 244160007..432cdea15 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -475,27 +475,32 @@ class TestOnlineAccount: ac1._evlogger.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED") ac1._evlogger.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED") - def test_forward_messages(self, acfactory): + def test_forward_messages(self, acfactory, lp): ac1, ac2 = acfactory.get_two_online_accounts() chat = self.get_chat(ac1, ac2) + lp.sec("ac1: send message to ac2") msg_out = chat.send_text("message2") - # wait for other account to receive + lp.sec("ac2: wait for receive") ev = ac2._evlogger.get_matching("DC_EVENT_INCOMING_MSG|DC_EVENT_MSGS_CHANGED") assert ev[2] == msg_out.id msg_in = ac2.get_message_by_id(msg_out.id) assert msg_in.text == "message2" - # check the message arrived in contact-requests/deaddrop + lp.sec("ac2: check that the message arrive in deaddrop") chat2 = msg_in.chat assert msg_in in chat2.get_messages() assert not msg_in.is_forwarded() assert chat2.is_deaddrop() assert chat2 == ac2.get_deaddrop_chat() + + lp.sec("ac2: create new chat and forward message to it") chat3 = ac2.create_group_chat("newgroup") assert not chat3.is_promoted() ac2.forward_messages([msg_in], chat3) + + lp.sec("ac2: check new chat has a forwarded message") assert chat3.is_promoted() messages = chat3.get_messages() msg = messages[-1] diff --git a/src/chat.rs b/src/chat.rs index e4f5de0c5..cd0a238c0 100644 --- a/src/chat.rs +++ b/src/chat.rs @@ -579,6 +579,10 @@ pub fn unblock(context: &Context, chat_id: u32) { } pub fn set_blocking(context: &Context, chat_id: u32, new_blocking: Blocked) -> bool { + if chat_id == 0 { + warn!(context, "ignoring setting of Block-status for chat_id=0"); + return false; + } sql::execute( context, &context.sql, diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index 4e004d71a..99f52fb60 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -422,8 +422,7 @@ fn add_parts( *from_id, to_ids, )?; - if new_chat_id != 0 && chat_id_blocked != Blocked::Not && create_blocked == Blocked::Not - { + if chat_id_blocked != Blocked::Not && create_blocked == Blocked::Not { chat::unblock(context, new_chat_id); } *chat_id = new_chat_id; @@ -514,8 +513,9 @@ fn add_parts( *from_id, to_ids, )?; - if new_chat_id != 0 && chat_id_blocked != Blocked::Not { - chat::unblock(context, *chat_id); + // automatically unblock chat when the user sends a message + if chat_id_blocked != Blocked::Not { + chat::unblock(context, new_chat_id); } *chat_id = new_chat_id; } @@ -785,7 +785,7 @@ fn calc_timestamps( /// - is there a group with the same recipients? if so, use this (if there are multiple, use the most recent one) /// - create an ad-hoc group based on the recipient list /// -/// So when the function returns, the caller has the group id matching the current state of the group. +/// on success the function returns the found/created (chat_id, chat_blocked) tuple . #[allow(non_snake_case)] fn create_or_lookup_group( context: &Context, @@ -793,9 +793,8 @@ fn create_or_lookup_group( allow_creation: i32, create_blocked: Blocked, from_id: u32, - to_ids: &mut Vec, + to_ids: &Vec, ) -> Result<(u32, Blocked)> { - let group_explicitly_left: bool; let mut chat_id_blocked = Blocked::Not; let mut grpid = "".to_string(); let mut grpname = None; @@ -808,17 +807,6 @@ fn create_or_lookup_group( let mut X_MrGrpImageChanged = "".to_string(); let mut better_msg: String = From::from(""); - let cleanup = |chat_id: u32, chat_id_blocked: Blocked| { - Ok(( - chat_id, - if 0 != chat_id { - chat_id_blocked - } else { - Blocked::Not - }, - )) - }; - if mime_parser.is_system_message == SystemMessage::LocationStreamingEnabled { better_msg = context.stock_system_msg(StockMessage::MsgLocationEnabled, "", "", from_id as u32) @@ -847,15 +835,18 @@ fn create_or_lookup_group( } if grpid.is_empty() { - let (new_chat_id, new_chat_id_blocked) = create_or_lookup_adhoc_group( + return create_or_lookup_adhoc_group( context, mime_parser, allow_creation, create_blocked, from_id, to_ids, - )?; - return cleanup(new_chat_id, new_chat_id_blocked); + ) + .map_err(|err| { + info!(context, "could not create adhoc-group: {:?}", err); + err + }); } } } @@ -935,26 +926,29 @@ fn create_or_lookup_group( // check, if we have a chat with this group ID let (mut chat_id, chat_id_verified, _blocked) = chat::get_chat_id_by_grpid(context, &grpid); - if chat_id != 0 && chat_id_verified { - if let Err(err) = check_verified_properties(context, mime_parser, from_id as u32, to_ids) { - warn!(context, "verification problem: {}", err); - let s = format!("{}. See 'Info' for more details", err); - mime_parser.repl_msg_by_error(s); + if chat_id != 0 { + if chat_id_verified { + if let Err(err) = + check_verified_properties(context, mime_parser, from_id as u32, to_ids) + { + warn!(context, "verification problem: {}", err); + let s = format!("{}. See 'Info' for more details", err); + mime_parser.repl_msg_by_error(s); + } + } + // check if the sender is a member of the existing group - + // if not, we'll recreate the group list + if !chat::is_contact_in_chat(context, chat_id, from_id as u32) { + recreate_member_list = 1; } } - // check if the sender is a member of the existing group - - // if not, we'll recreate the group list - if chat_id != 0 && !chat::is_contact_in_chat(context, chat_id, from_id as u32) { - recreate_member_list = 1; - } - // check if the group does not exist but should be created - group_explicitly_left = chat::is_group_explicitly_left(context, &grpid).unwrap_or_default(); - + let group_explicitly_left = chat::is_group_explicitly_left(context, &grpid).unwrap_or_default(); let self_addr = context .get_config(Config::ConfiguredAddr) .unwrap_or_default(); + if chat_id == 0 && !mime_parser.is_mailinglist_message() && !grpid.is_empty() @@ -977,9 +971,7 @@ fn create_or_lookup_group( mime_parser.repl_msg_by_error(&s); } } - if 0 == allow_creation { - return cleanup(chat_id, chat_id_blocked); - } + ensure!(allow_creation != 0, "creating group forbidden by caller"); chat_id = create_group_record( context, &grpid, @@ -993,21 +985,22 @@ fn create_or_lookup_group( // again, check chat_id if chat_id <= DC_CHAT_ID_LAST_SPECIAL { - if group_explicitly_left { - chat_id = DC_CHAT_ID_TRASH; + return if group_explicitly_left { + Ok((DC_CHAT_ID_TRASH, chat_id_blocked)) } else { - let (new_chat_id, new_chat_id_blocked) = create_or_lookup_adhoc_group( + create_or_lookup_adhoc_group( context, mime_parser, allow_creation, create_blocked, from_id, to_ids, - )?; - chat_id = new_chat_id; - chat_id_blocked = new_chat_id_blocked; - } - return cleanup(chat_id, chat_id_blocked); + ) + .map_err(|err| { + warn!(context, "failed to create ad-hoc group: {:?}", err); + err + }) + }; } // execute group commands @@ -1112,13 +1105,12 @@ fn create_or_lookup_group( } // check the number of receivers - - // the only critical situation is if the user hits "Reply" instead of "Reply all" in a non-messenger-client */ + // the only critical situation is if the user hits "Reply" instead + // of "Reply all" in a non-messenger-client */ if to_ids_cnt == 1 && !mime_parser.is_send_by_messenger { - let is_contact_cnt = chat::get_chat_contact_cnt(context, chat_id); - if is_contact_cnt > 3 { + if chat::get_chat_contact_cnt(context, chat_id) > 3 { // to_ids_cnt==1 may be "From: A, To: B, SELF" as SELF is not counted in to_ids_cnt. // So everything up to 3 is no error. - chat_id = 0; create_or_lookup_adhoc_group( context, mime_parser, @@ -1126,30 +1118,31 @@ fn create_or_lookup_group( create_blocked, from_id, to_ids, - )?; + ) + .map_err(|err| { + warn!(context, "could not create ad-hoc group: {:?}", err); + err + })?; } } - - return cleanup(chat_id, chat_id_blocked); + return Ok((chat_id, chat_id_blocked)); } -/// Handle groups for received messages +/// Handle groups for received messages, return chat_id/Blocked status on success fn create_or_lookup_adhoc_group( context: &Context, mime_parser: &MimeParser, allow_creation: i32, create_blocked: Blocked, from_id: u32, - to_ids: &mut Vec, + to_ids: &Vec, ) -> Result<(u32, Blocked)> { - // if we're here, no grpid was found, check there is an existing ad-hoc - // group matching the to-list or if we can create one + // if we're here, no grpid was found, check there if is an existing ad-hoc + // group matching the to-list or if we should and can create one - ensure!(!to_ids.is_empty(), "empty To-list"); - ensure!( - !mime_parser.is_mailinglist_message(), - "mailing-list message" - ); + if to_ids.is_empty() || mime_parser.is_mailinglist_message() { + return Ok((0, Blocked::Not)); + } let mut member_ids = to_ids.clone(); if !member_ids.contains(&from_id) { @@ -1158,11 +1151,10 @@ fn create_or_lookup_adhoc_group( if !member_ids.contains(&DC_CONTACT_ID_SELF) { member_ids.push(DC_CONTACT_ID_SELF); } - ensure!( - member_ids.len() >= 3, - "Num-contacts={} too low", - member_ids.len() - ); + + if member_ids.len() < 3 { + return Ok((0, Blocked::Not)); + } let chat_ids = search_chat_ids_by_contact_ids(context, &member_ids)?; if !chat_ids.is_empty() { @@ -1185,10 +1177,10 @@ fn create_or_lookup_adhoc_group( } } - ensure!( - allow_creation != 0, - "creating ad-hoc group prevented from caller" - ); + if allow_creation == 0 { + info!(context, "creating ad-hoc group prevented from caller"); + return Ok((0, Blocked::Not)); + } // we do not check if the message is a reply to another group, this may result in // chats with unclear member list. instead we create a new group in the following lines ...