From 5ce44ade173662e64b390ac39dcc77558b13ba5e Mon Sep 17 00:00:00 2001
From: iequidoo <dgreshilov@gmail.com>
Date: Tue, 6 Aug 2024 18:35:10 -0300
Subject: [PATCH] feat: Disable MDNs for bots by default

- To avoid receiving undecryptable MDNs by bots and replying to them if the bot's key changes.
- MDNs from bots don't look useful in general, usually the user expects some reply from the bot, not
  just that the message is read.
---
 src/config.rs      | 19 +++++++++++++++++--
 src/message.rs     | 22 ++++++++++------------
 src/mimefactory.rs |  2 +-
 src/smtp.rs        |  4 +---
 4 files changed, 29 insertions(+), 18 deletions(-)

diff --git a/src/config.rs b/src/config.rs
index e55b933be..978becda3 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -129,7 +129,6 @@ pub enum Config {
 
     /// True if Message Delivery Notifications (read receipts) should
     /// be sent and requested.
-    #[strum(props(default = "1"))]
     MdnsEnabled,
 
     /// True if "Sent" folder should be watched for changes.
@@ -515,6 +514,14 @@ impl Context {
             && !self.get_config_bool(Config::Bot).await?)
     }
 
+    /// Returns whether MDNs should be requested and sent.
+    pub(crate) async fn mdns_enabled(&self) -> Result<bool> {
+        match self.get_config_bool_opt(Config::MdnsEnabled).await? {
+            Some(val) => Ok(val),
+            None => Ok(!self.get_config_bool(Config::Bot).await?),
+        }
+    }
+
     /// Gets configured "delete_server_after" value.
     ///
     /// `None` means never delete the message, `Some(0)` means delete
@@ -964,6 +971,15 @@ mod tests {
         Ok(())
     }
 
+    #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+    async fn test_mdns_enabled() -> Result<()> {
+        let t = &TestContext::new_alice().await;
+        assert!(t.mdns_enabled().await?);
+        t.set_config_bool(Config::Bot, true).await?;
+        assert!(!t.mdns_enabled().await?);
+        Ok(())
+    }
+
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
     async fn test_sync() -> Result<()> {
         let alice0 = TestContext::new_alice().await;
@@ -990,7 +1006,6 @@ mod tests {
         // Reset to default. Test that it's not synced because defaults may differ across client
         // versions.
         alice0.set_config(Config::MdnsEnabled, None).await?;
-        assert_eq!(alice0.get_config_bool(Config::MdnsEnabled).await?, true);
         alice0.set_config_bool(Config::MdnsEnabled, false).await?;
         sync(&alice0, &alice1).await;
         assert_eq!(alice1.get_config_bool(Config::MdnsEnabled).await?, false);
diff --git a/src/message.rs b/src/message.rs
index b984bb0df..6637a1129 100644
--- a/src/message.rs
+++ b/src/message.rs
@@ -1769,19 +1769,17 @@ pub async fn markseen_msgs(context: &Context, msg_ids: Vec<MsgId>) -> Result<()>
             if curr_blocked == Blocked::Not
                 && curr_param.get_bool(Param::WantsMdn).unwrap_or_default()
                 && curr_param.get_cmd() == SystemMessage::Unknown
+                && context.mdns_enabled().await?
             {
-                let mdns_enabled = context.get_config_bool(Config::MdnsEnabled).await?;
-                if mdns_enabled {
-                    context
-                        .sql
-                        .execute(
-                            "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)",
-                            (id, curr_from_id, curr_rfc724_mid),
-                        )
-                        .await
-                        .context("failed to insert into smtp_mdns")?;
-                    context.scheduler.interrupt_smtp().await;
-                }
+                context
+                    .sql
+                    .execute(
+                        "INSERT INTO smtp_mdns (msg_id, from_id, rfc724_mid) VALUES(?, ?, ?)",
+                        (id, curr_from_id, curr_rfc724_mid),
+                    )
+                    .await
+                    .context("failed to insert into smtp_mdns")?;
+                context.scheduler.interrupt_smtp().await;
             }
             updated_chat_ids.insert(curr_chat_id);
         }
diff --git a/src/mimefactory.rs b/src/mimefactory.rs
index 4daf73e95..2796e51ee 100644
--- a/src/mimefactory.rs
+++ b/src/mimefactory.rs
@@ -186,7 +186,7 @@ impl MimeFactory {
 
             if !msg.is_system_message()
                 && msg.param.get_int(Param::Reaction).unwrap_or_default() == 0
-                && context.get_config_bool(Config::MdnsEnabled).await?
+                && context.mdns_enabled().await?
             {
                 req_mdn = true;
             }
diff --git a/src/smtp.rs b/src/smtp.rs
index 546fce38f..7bbe73264 100644
--- a/src/smtp.rs
+++ b/src/smtp.rs
@@ -631,9 +631,7 @@ async fn send_mdn_rfc724_mid(
 
 /// Tries to send a single MDN. Returns true if more MDNs should be sent.
 async fn send_mdn(context: &Context, smtp: &mut Smtp) -> Result<bool> {
-    let mdns_enabled = context.get_config_bool(Config::MdnsEnabled).await?;
-    if !mdns_enabled {
-        // User has disabled MDNs.
+    if !context.mdns_enabled().await? {
         context.sql.execute("DELETE FROM smtp_mdns", []).await?;
         return Ok(false);
     }