diff --git a/python/tests/test_account.py b/python/tests/test_account.py index 141b02bcc..f582368e4 100644 --- a/python/tests/test_account.py +++ b/python/tests/test_account.py @@ -687,6 +687,39 @@ class TestOnlineAccount: except queue.Empty: pass # mark_seen_messages() has generated events before it returns + def test_mdn_asymetric(self, acfactory, lp): + ac1, ac2 = acfactory.get_two_online_accounts() + + lp.sec("ac1: create chat with ac2") + chat = self.get_chat(ac1, ac2, both_created=True) + + # make sure mdns are enabled (usually enabled by default already) + ac1.set_config("mdns_enabled", "1") + ac2.set_config("mdns_enabled", "1") + + lp.sec("sending text message from ac1 to ac2") + msg_out = chat.send_text("message1") + + assert len(chat.get_messages()) == 1 + + lp.sec("disable ac1 MDNs") + ac1.set_config("mdns_enabled", "0") + + lp.sec("wait for ac2 to receive message") + msg = ac2.wait_next_incoming_message() + + assert len(msg.chat.get_messages()) == 1 + + lp.sec("ac2: mark incoming message as seen") + ac2.mark_seen_messages([msg]) + + lp.sec("ac1: waiting for incoming activity") + # wait for MOVED event because even ignored read-receipts should be moved + ac1._evlogger.get_matching("DC_EVENT_IMAP_MESSAGE_MOVED") + + assert len(chat.get_messages()) == 1 + assert not msg_out.is_out_mdn_received() + def test_send_and_receive_will_encrypt_decrypt(self, acfactory, lp): ac1, ac2 = acfactory.get_two_online_accounts() diff --git a/src/contact.rs b/src/contact.rs index fe7bbb285..dd4acd129 100644 --- a/src/contact.rs +++ b/src/contact.rs @@ -971,7 +971,7 @@ fn set_block_contact(context: &Context, contact_id: u32, new_blocking: bool) { pub fn set_profile_image( context: &Context, contact_id: u32, - profile_image: AvatarAction, + profile_image: &AvatarAction, ) -> Result<()> { // the given profile image is expected to be already in the blob directory // as profile images can be set only by receiving messages, this should be always the case, however. diff --git a/src/dc_receive_imf.rs b/src/dc_receive_imf.rs index dbcdfb4d0..fad00b47f 100644 --- a/src/dc_receive_imf.rs +++ b/src/dc_receive_imf.rs @@ -73,15 +73,13 @@ pub fn dc_receive_imf( let mut sent_timestamp = 0; let mut created_db_entries = Vec::new(); let mut create_event_to_send = Some(CreateEvent::MsgsChanged); - let mut rr_event_to_send = Vec::new(); let mut to_ids = ContactIds::new(); // helper method to handle early exit and memory cleanup let cleanup = |context: &Context, create_event_to_send: &Option, - created_db_entries: &Vec<(usize, MsgId)>, - rr_event_to_send: &Vec<(u32, MsgId)>| { + created_db_entries: &Vec<(usize, MsgId)>| { if let Some(create_event_to_send) = create_event_to_send { for (chat_id, msg_id) in created_db_entries { let event = match create_event_to_send { @@ -97,12 +95,6 @@ pub fn dc_receive_imf( context.call_cb(event); } } - for (chat_id, msg_id) in rr_event_to_send { - context.call_cb(Event::MsgRead { - chat_id: *chat_id, - msg_id: *msg_id, - }); - } }; if let Some(value) = mime_parser.get(HeaderDef::Date) { @@ -202,12 +194,7 @@ pub fn dc_receive_imf( &mut created_db_entries, &mut create_event_to_send, ) { - cleanup( - context, - &create_event_to_send, - &created_db_entries, - &rr_event_to_send, - ); + cleanup(context, &create_event_to_send, &created_db_entries); bail!("add_parts error: {:?}", err); } } else { @@ -218,14 +205,6 @@ pub fn dc_receive_imf( } } - mime_parser.handle_reports( - from_id, - sent_timestamp, - &mut rr_event_to_send, - &server_folder, - server_uid, - ); - if mime_parser.location_kml.is_some() || mime_parser.message_kml.is_some() { save_locations( context, @@ -238,7 +217,7 @@ pub fn dc_receive_imf( } if mime_parser.user_avatar != AvatarAction::None { - match contact::set_profile_image(&context, from_id, mime_parser.user_avatar) { + match contact::set_profile_image(&context, from_id, &mime_parser.user_avatar) { Ok(()) => { context.call_cb(Event::ChatModified(chat_id)); } @@ -266,12 +245,9 @@ pub fn dc_receive_imf( "received message {} has Message-Id: {}", server_uid, rfc724_mid ); - cleanup( - context, - &create_event_to_send, - &created_db_entries, - &rr_event_to_send, - ); + cleanup(context, &create_event_to_send, &created_db_entries); + + mime_parser.handle_reports(from_id, sent_timestamp, &server_folder, server_uid); Ok(()) } diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 3bb7b4819..526f1194d 100644 --- a/src/mimeparser.rs +++ b/src/mimeparser.rs @@ -14,11 +14,11 @@ use crate::dc_simplify::*; use crate::dc_tools::*; use crate::e2ee; use crate::error::Result; +use crate::events::Event; use crate::headerdef::HeaderDef; use crate::job::{job_add, Action}; use crate::location; use crate::message; -use crate::message::MsgId; use crate::param::*; use crate::peerstate::Peerstate; use crate::securejoin::handle_degrade_event; @@ -40,7 +40,6 @@ pub struct MimeParser<'a> { pub user_avatar: AvatarAction, pub group_avatar: AvatarAction, reports: Vec, - mdns_enabled: bool, parsed_protected_headers: bool, } @@ -86,7 +85,6 @@ const MIME_AC_SETUP_FILE: &str = "application/autocrypt-setup"; impl<'a> MimeParser<'a> { pub fn from_bytes(context: &'a Context, body: &[u8]) -> Result { let mail = mailparse::parse_mail(body)?; - let mdns_enabled = context.get_config_bool(Config::MdnsEnabled); let mut parser = MimeParser { parts: Vec::new(), @@ -104,7 +102,6 @@ impl<'a> MimeParser<'a> { message_kml: None, user_avatar: AvatarAction::None, group_avatar: AvatarAction::None, - mdns_enabled, parsed_protected_headers: false, }; @@ -313,7 +310,10 @@ impl<'a> MimeParser<'a> { } } - // Cleanup - and try to create at least an empty part if there are no parts yet + // If there were no parts, especially a non-DC mail user may + // just have send a message in the subject with an empty body. + // Besides, we want to show something in case our incoming-processing + // failed to properly handle an incoming message. if self.get_last_nonmeta().is_none() && self.reports.is_empty() { let mut part = Part::default(); part.typ = Viewtype::Text; @@ -726,11 +726,6 @@ impl<'a> MimeParser<'a> { } fn process_report(&self, report: &mailparse::ParsedMail<'_>) -> Result> { - // to get a clear functionality, do not show incoming MDNs if the options is disabled - if !self.mdns_enabled { - return Ok(None); - } - // parse as mailheaders let report_body = report.subparts[1].get_body_raw()?; let (report_fields, _) = mailparse::parse_headers(&report_body)?; @@ -749,33 +744,46 @@ impl<'a> MimeParser<'a> { })); } } + warn!( + self.context, + "ignoring unknown disposition-notification, Message-Id: {:?}", + report_fields.get_first_value("Message-ID").ok() + ); Ok(None) } - // Handle reports (mainly MDNs) + // Handle reports (only MDNs for now) pub fn handle_reports( &self, from_id: u32, sent_timestamp: i64, - rr_event_to_send: &mut Vec<(u32, MsgId)>, server_folder: impl AsRef, server_uid: u32, ) { - for report in &self.reports { - let mut mdn_consumed = false; + if self.reports.is_empty() { + return; + } + // If a user disabled MDNs we do not show pending incoming ones anymore + // but we do want them to potentially get moved from the INBOX still. + let mdns_enabled = self.context.get_config_bool(Config::MdnsEnabled); - if let Some((chat_id, msg_id)) = message::mdn_from_ext( - self.context, - from_id, - &report.original_message_id, - sent_timestamp, - ) { - rr_event_to_send.push((chat_id, msg_id)); - mdn_consumed = true; + for report in &self.reports { + let mut mdn_recognized = false; + + if mdns_enabled { + if let Some((chat_id, msg_id)) = message::mdn_from_ext( + self.context, + from_id, + &report.original_message_id, + sent_timestamp, + ) { + self.context.call_cb(Event::MsgRead { chat_id, msg_id }); + mdn_recognized = true; + } } - if self.has_chat_version() || mdn_consumed { + if self.has_chat_version() || mdn_recognized || !mdns_enabled { let mut param = Params::new(); param.set(Param::ServerFolder, server_folder.as_ref()); param.set_int(Param::ServerUid, server_uid as i32);