fix #1037 and simplify mdn-report related code and state

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()
 

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.

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<CreateEvent>,
-                   created_db_entries: &Vec<(usize, MsgId)>,
+                   created_db_entries: &Vec<(usize, MsgId)>| {
-                   rr_event_to_send: &Vec<(u32, 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(
+            cleanup(context, &create_event_to_send, &created_db_entries);
-                context,
-                &create_event_to_send,
-                &created_db_entries,
-                &rr_event_to_send,
-            );
             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(
+    cleanup(context, &create_event_to_send, &created_db_entries);
-        context,
+
-        &create_event_to_send,
+    mime_parser.handle_reports(from_id, sent_timestamp, &server_folder, server_uid);
-        &created_db_entries,
-        &rr_event_to_send,
-    );
 
     Ok(())
 }

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<Report>,
-    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<Self> {
         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<Option<Report>> {
-        // 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<str>,
         server_uid: u32,
     ) {
-        for report in &self.reports {
+        if self.reports.is_empty() {
-            let mut mdn_consumed = false;
+            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);
 
+        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,
                 ) {
-                rr_event_to_send.push((chat_id, msg_id));
+                    self.context.call_cb(Event::MsgRead { chat_id, msg_id });
-                mdn_consumed = true;
+                    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);