From 2969a81b5a15a6b4d4dfdc3c861d6cdb563e44d8 Mon Sep 17 00:00:00 2001 From: link2xt Date: Thu, 10 Sep 2026 22:12:57 +0000 Subject: [PATCH] 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 ade39fe026a3539d5279a6902f23fe6b825c17a7 We do not actually use WantsMdn for outgoing messages. --- src/mimeparser.rs | 51 +++++++--------------- src/mimeparser/mimeparser_tests.rs | 70 +++++++++++++++++++++++++++--- src/param.rs | 4 ++ 3 files changed, 83 insertions(+), 42 deletions(-) diff --git a/src/mimeparser.rs b/src/mimeparser.rs index 2c7847d7cc..3d0f2940a9 100644 --- a/src/mimeparser.rs +++ b/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, - pub chat_disposition_notification_to: Option, + + /// 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, @@ -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, headers_removed: &mut HashSet, recipients: &mut Vec, past_members: &mut Vec, from: &mut Option, list_post: &mut Option, - chat_disposition_notification_to: &mut Option, + 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); diff --git a/src/mimeparser/mimeparser_tests.rs b/src/mimeparser/mimeparser_tests.rs index 96fd58b7be..fbe5b58a86 100644 --- a/src/mimeparser/mimeparser_tests.rs +++ b/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(); diff --git a/src/param.rs b/src/param.rs index 2f1d66894e..68bde7a71a 100644 --- a/src/param.rs +++ b/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.