feat: ignore Chat-Disposition-Notification-To value

Main change is the removal of the comparison of Chat-Disposition-Notification-To
to the From header for incoming messages.

Removed code that was settting WantsMdn for outgoing messages
is a leftover not cleaned up in ade39fe026
We do not actually use WantsMdn for outgoing messages.

src/mimeparser.rs

@@ -10,7 +10,7 @@ use anyhow::{Context as _, Result, bail, ensure};
 use deltachat_contact_tools::{addr_cmp, addr_normalize, sanitize_bidi_characters};
 use deltachat_derive::{FromSql, ToSql};
 use format_flowed::unformat_flowed;
-use mailparse::{DispositionType, MailHeader, MailHeaderMap, SingleInfo, addrparse_header};
+use mailparse::{DispositionType, MailHeader, MailHeaderMap, SingleInfo};
 use mime::Mime;
 
 use crate::aheader::Aheader;
@@ -82,7 +82,9 @@ pub(crate) struct MimeMessage {
     /// The List-Post address is only set for mailing lists. Users can send
     /// messages to this address to post them to the list.
     pub list_post: Option<String>,
-    pub chat_disposition_notification_to: Option<SingleInfo>,
+
+    /// True if the message requests a read receipt (MDN).
+    pub wants_mdn: bool,
 
     /// Decryption error if decryption of the message has failed.
     pub decryption_error: Option<String>,
@@ -290,18 +292,17 @@ impl MimeMessage {
         let mut past_members = Default::default();
         let mut from = Default::default();
         let mut list_post = Default::default();
-        let mut chat_disposition_notification_to = None;
+        let mut wants_mdn = false;
 
         // Parse IMF headers.
         MimeMessage::merge_headers(
-            context,
             &mut headers,
             &mut headers_removed,
             &mut recipients,
             &mut past_members,
             &mut from,
             &mut list_post,
-            &mut chat_disposition_notification_to,
+            &mut wants_mdn,
             &mail,
         );
         headers_removed.extend(
@@ -538,14 +539,13 @@ impl MimeMessage {
             let mut inner_from = None;
 
             MimeMessage::merge_headers(
-                context,
                 &mut headers,
                 &mut headers_removed,
                 &mut recipients,
                 &mut past_members,
                 &mut inner_from,
                 &mut list_post,
-                &mut chat_disposition_notification_to,
+                &mut wants_mdn,
                 mail,
             );
 
@@ -649,7 +649,7 @@ impl MimeMessage {
             list_post,
             from,
             incoming,
-            chat_disposition_notification_to,
+            wants_mdn,
             decryption_error: mail.err().map(|err| format!("{err:#}")),
 
             // only non-empty if it was a valid autocrypt message
@@ -944,26 +944,13 @@ impl MimeMessage {
         self.parse_attachments();
 
         // See if an MDN is requested from the other side
-        let mut wants_mdn = false;
         if self.decryption_error.is_none()
             && (!self.parts.is_empty() || matches!(&self.pre_message, PreMessageMode::Pre { .. }))
-            && let Some(ref dn_to) = self.chat_disposition_notification_to
+            && self.wants_mdn
+            && self.incoming
+            && let Some(part) = self.parts.last_mut()
         {
-            // Check that the message is not outgoing.
-            let from = &self.from.addr;
-            if !context.is_self_addr(from).await? {
-                if from.to_lowercase() == dn_to.addr.to_lowercase() {
-                    wants_mdn = true;
-                    if let Some(part) = self.parts.last_mut() {
-                        part.param.set_int(Param::WantsMdn, 1);
-                    }
-                } else {
-                    warn!(
-                        context,
-                        "{} requested a read receipt to {}, ignoring", from, dn_to.addr
-                    );
-                }
-            }
+            part.param.set_int(Param::WantsMdn, 1);
         }
 
         // If there were no parts, especially a non-DC mail user may
@@ -975,7 +962,7 @@ impl MimeMessage {
                 typ: Viewtype::Text,
                 ..Default::default()
             };
-            if wants_mdn {
+            if self.wants_mdn && self.incoming {
                 part.param.set_int(Param::WantsMdn, 1);
             }
             if let Some(ref subject) = self.get_subject()
@@ -1759,14 +1746,13 @@ impl MimeMessage {
     /// outer parts.
     #[allow(clippy::too_many_arguments)]
     fn merge_headers(
-        context: &Context,
         headers: &mut HashMap<String, String>,
         headers_removed: &mut HashSet<String>,
         recipients: &mut Vec<SingleInfo>,
         past_members: &mut Vec<SingleInfo>,
         from: &mut Option<SingleInfo>,
         list_post: &mut Option<String>,
-        chat_disposition_notification_to: &mut Option<SingleInfo>,
+        wants_mdn: &mut bool,
         part: &mailparse::ParsedMail,
     ) {
         let fields = &part.headers;
@@ -1780,18 +1766,13 @@ impl MimeMessage {
         );
 
         if has_header_protection {
-            *chat_disposition_notification_to = None;
+            *wants_mdn = false;
         }
         for field in fields {
             // lowercasing all headers is technically not correct, but makes things work better
             let key = field.get_key().to_lowercase();
             if key == HeaderDef::ChatDispositionNotificationTo.get_headername() {
-                match addrparse_header(field) {
-                    Ok(addrlist) => {
-                        *chat_disposition_notification_to = addrlist.extract_single_info();
-                    }
-                    Err(e) => warn!(context, "Could not read {} address: {}", key, e),
-                }
+                *wants_mdn = true;
             } else {
                 let value = field.get_value();
                 headers.insert(key.to_string(), value);

src/mimeparser/mimeparser_tests.rs

@@ -331,11 +331,11 @@ async fn test_mailparse_0_16_0_panic() {
     );
 }
 
+/// Test that From with multiple addresses is not allowed.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-async fn test_parse_first_addr() {
+async fn test_multiple_from_addresses() {
     let context = TestContext::new().await;
     let raw = b"From: hello@one.org, world@two.org\n\
-                    Chat-Disposition-Notification-To: wrong\n\
                     Content-Type: text/plain\n\
                     Chat-Version: 1.0\n\
                     \n\
@@ -343,11 +343,27 @@ async fn test_parse_first_addr() {
                     ";
 
     let mimeparser = MimeMessage::from_bytes(&context.ctx, &raw[..]).await;
-
     assert!(mimeparser.is_err());
-    context
-        .assert_warn("Invalid address found: must contain a '@' symbol")
-        .await;
+}
+
+/// Tests that Chat-Disposition-Notification-To value does not matter.
+///
+/// Even if it does not look like an address, it is still an MDN request.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_chat_disposition_notification_any_value() {
+    let context = TestContext::new().await;
+    let raw = b"From: alice@example.org\n\
+                    Chat-Disposition-Notification-To: wrong\n\
+                    Content-Type: text/plain\n\
+                    Chat-Version: 1.0\n\
+                    \n\
+                    test1\n\
+                    ";
+
+    let mimeparser = MimeMessage::from_bytes(&context.ctx, &raw[..])
+        .await
+        .unwrap();
+    assert!(mimeparser.wants_mdn);
 }
 
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -1491,7 +1507,7 @@ Some reply
     Ok(())
 }
 
-// Test that WantsMdn parameter is not set on outgoing messages.
+/// Test that WantsMdn parameter is not set on outgoing messages.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_outgoing_wants_mdn() -> Result<()> {
     let mut tcm = TestContextManager::new();
@@ -1515,6 +1531,46 @@ async fn test_outgoing_wants_mdn() -> Result<()> {
     Ok(())
 }
 
+/// Tests that message does not want an MDN if the sender did not request it.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_sender_mdns_disabled() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    alice.set_config_bool(Config::MdnsEnabled, false).await?;
+    let bob = &tcm.bob().await;
+    assert!(!alice.should_request_mdns().await?);
+
+    let chat_id = alice.create_chat(bob).await.id;
+    let sent = alice.send_text(chat_id, "Message.").await;
+
+    let bob_msg = bob.recv_msg(&sent).await;
+    assert!(bob_msg.param.get_bool(Param::WantsMdn).is_none());
+    Ok(())
+}
+
+/// Tests that message may want an MDN if receiver disabled them.
+///
+/// MDN still should not be sent, but may be sent
+/// if receiver re-enables MDNs after receiving the message
+/// and before reading it.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn test_receiver_mdns_disabled() -> Result<()> {
+    let mut tcm = TestContextManager::new();
+    let alice = &tcm.alice().await;
+    let bob = &tcm.bob().await;
+    bob.set_config_bool(Config::MdnsEnabled, false).await?;
+    assert!(alice.should_request_mdns().await?);
+
+    let chat_id = alice.create_chat(bob).await.id;
+    let sent = alice.send_text(chat_id, "Message.").await;
+
+    // Message wants an MDN, but Bob should not send it.
+    let bob_msg = bob.recv_msg(&sent).await;
+    assert!(bob_msg.param.get_bool(Param::WantsMdn).unwrap());
+    assert!(!bob.should_send_mdns().await?);
+    Ok(())
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn test_ignore_read_receipt_to_self() -> Result<()> {
     let mut tcm = TestContextManager::new();

src/param.rs

@@ -68,6 +68,10 @@ pub enum Param {
     DeprecatedSkipAutocrypt = b'o',
 
     /// For Messages
+    ///
+    /// Set if the message is incoming and requests an MDN.
+    /// Should not be set on outgoing messages,
+    /// we do not want to send MDNs to our own messages.
     WantsMdn = b'r',
 
     /// For Messages: Render message as a RFC 9078 reaction.