fix: Cross-profile forwarding of messages with has_html() returning true (#7791)

This includes forwarding of long messages. Also this fixes sending, but more likely resending of
forwarded messages for which the original message was deleted, because now we save HTML to the db
immediately when creating a forwarded message, not when actually sending it.

Author: iequidoo

src/chat.rs

@@ -7,6 +7,7 @@ use std::io::Cursor;
 use std::marker::Sync;
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
+use std::sync::Arc;
 use std::time::Duration;
 
 use anyhow::{Context as _, Result, anyhow, bail, ensure};
@@ -1886,11 +1887,7 @@ impl Chat {
 
         let (msg_text, was_truncated) = truncate_msg_text(context, msg.text.clone()).await?;
         let new_mime_headers = if msg.has_html() {
-            if msg.param.exists(Param::Forwarded) {
-                msg.get_id().get_html(context).await?
-            } else {
-                msg.param.get(Param::SendHtml).map(|s| s.to_string())
-            }
+            msg.param.get(Param::SendHtml).map(|s| s.to_string())
         } else {
             None
         };
@@ -4367,8 +4364,12 @@ pub async fn forward_msgs_2ctx(
         msg.param = Params::new();
 
         if msg.get_viewtype() != Viewtype::Sticker {
+            let forwarded_msg_id = match Arc::ptr_eq(&ctx_src.inner, &ctx_dst.inner) {
+                true => src_msg_id,
+                false => MsgId::new_unset(),
+            };
             msg.param
-                .set_int(Param::Forwarded, src_msg_id.to_u32() as i32);
+                .set_int(Param::Forwarded, forwarded_msg_id.to_u32() as i32);
         }
 
         if msg.get_viewtype() == Viewtype::Call {
@@ -4395,6 +4396,9 @@ pub async fn forward_msgs_2ctx(
         msg.param.steal(param, Param::ProtectQuote);
         msg.param.steal(param, Param::Quote);
         msg.param.steal(param, Param::Summary1);
+        if msg.has_html() {
+            msg.set_html(src_msg_id.get_html(ctx_src).await?);
+        }
         msg.in_reply_to = None;
 
         // do not leak data as group names; a default subject is generated by mimefactory

src/html.rs

@@ -254,11 +254,14 @@ fn mimepart_to_data_url(mail: &mailparse::ParsedMail<'_>) -> Result<String> {
 
 impl MsgId {
     /// Get HTML by database message id.
-    /// This requires `mime_headers` field to be set for the message;
-    /// this is the case at least when `Message.has_html()` returns true
-    /// (we do not save raw mime unconditionally in the database to save space).
+    /// Returns `Some` at least when `Message.has_html()` is true.
+    /// NB: we do not save raw mime unconditionally in the database to save space.
     /// The corresponding ffi-function is `dc_get_msg_html()`.
     pub async fn get_html(self, context: &Context) -> Result<Option<String>> {
+        let param = self.get_param(context).await?;
+        if let Some(html) = param.get(SendHtml) {
+            return Ok(Some(html.to_string()));
+        }
         let rawmime = message::get_mime_headers(context, self).await?;
 
         if !rawmime.is_empty() {
@@ -279,9 +282,9 @@ impl MsgId {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::chat;
-    use crate::chat::{forward_msgs, save_msgs};
+    use crate::chat::{self, Chat, forward_msgs, save_msgs};
     use crate::config::Config;
+    use crate::constants;
     use crate::contact::ContactId;
     use crate::message::{MessengerMessage, Viewtype};
     use crate::receive_imf::receive_imf;
@@ -440,7 +443,7 @@ test some special html-characters as &lt; &gt; and &amp; but also &quot; and &#x
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
-    async fn test_html_forwarding() {
+    async fn test_html_forwarding() -> Result<()> {
         // alice receives a non-delta html-message
         let mut tcm = TestContextManager::new();
         let alice = &tcm.alice().await;
@@ -459,31 +462,57 @@ test some special html-characters as &lt; &gt; and &amp; but also &quot; and &#x
         assert!(html.contains("this is <b>html</b>"));
 
         // alice: create chat with bob and forward received html-message there
-        let chat = alice.create_chat_with_contact("", "bob@example.net").await;
-        forward_msgs(alice, &[msg.get_id()], chat.get_id())
+        let chat_alice = alice.create_chat_with_contact("", "bob@example.net").await;
+        forward_msgs(alice, &[msg.get_id()], chat_alice.get_id())
             .await
             .unwrap();
-        let msg = alice.get_last_msg_in(chat.get_id()).await;
-        assert_eq!(msg.get_from_id(), ContactId::SELF);
-        assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
-        assert!(msg.is_forwarded());
-        assert!(msg.get_text().contains("this is plain"));
-        assert!(msg.has_html());
-        let html = msg.get_id().get_html(alice).await.unwrap().unwrap();
-        assert!(html.contains("this is <b>html</b>"));
+        async fn check_sender(ctx: &TestContext, chat: &Chat) {
+            let msg = ctx.get_last_msg_in(chat.get_id()).await;
+            assert_eq!(msg.get_from_id(), ContactId::SELF);
+            assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
+            assert!(msg.is_forwarded());
+            assert!(msg.get_text().contains("this is plain"));
+            assert!(msg.has_html());
+            let html = msg.get_id().get_html(ctx).await.unwrap().unwrap();
+            assert!(html.contains("this is <b>html</b>"));
+        }
+        check_sender(alice, &chat_alice).await;
 
         // bob: check that bob also got the html-part of the forwarded message
         let bob = &tcm.bob().await;
-        let chat = bob.create_chat_with_contact("", "alice@example.org").await;
-        let msg = bob.recv_msg(&alice.pop_sent_msg().await).await;
-        assert_eq!(chat.id, msg.chat_id);
-        assert_ne!(msg.get_from_id(), ContactId::SELF);
-        assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
-        assert!(msg.is_forwarded());
-        assert!(msg.get_text().contains("this is plain"));
+        let chat_bob = bob.create_chat_with_contact("", "alice@example.org").await;
+        async fn check_receiver(ctx: &TestContext, chat: &Chat, sender: &TestContext) {
+            let msg = ctx.recv_msg(&sender.pop_sent_msg().await).await;
+            assert_eq!(chat.id, msg.chat_id);
+            assert_ne!(msg.get_from_id(), ContactId::SELF);
+            assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
+            assert!(msg.is_forwarded());
+            assert!(msg.get_text().contains("this is plain"));
+            assert!(msg.has_html());
+            let html = msg.get_id().get_html(ctx).await.unwrap().unwrap();
+            assert!(html.contains("this is <b>html</b>"));
+        }
+        check_receiver(bob, &chat_bob, alice).await;
+
+        // Finally it appears that alice is bob, so alice can forward the message to herself via
+        // "bob" profile!
+        chat::forward_msgs_2ctx(alice, &[msg.get_id()], bob, chat_bob.get_id()).await?;
+        check_sender(bob, &chat_bob).await;
+        check_receiver(alice, &chat_alice, bob).await;
+
+        // Check cross-profile forwarding of long outgoing messages.
+        let line = "this text with 42 chars is just repeated.\n";
+        let long_txt = line.repeat(constants::DC_DESIRED_TEXT_LEN / line.len() + 2);
+        let mut msg = Message::new_text(long_txt);
+        alice.send_msg(chat_alice.id, &mut msg).await;
+        let msg = alice.get_last_msg_in(chat_alice.id).await;
         assert!(msg.has_html());
-        let html = msg.get_id().get_html(bob).await.unwrap().unwrap();
-        assert!(html.contains("this is <b>html</b>"));
+        let html = msg.id.get_html(alice).await?.unwrap();
+        chat::forward_msgs_2ctx(alice, &[msg.get_id()], bob, chat_bob.get_id()).await?;
+        let msg = bob.get_last_msg_in(chat_bob.id).await;
+        assert!(msg.has_html());
+        assert_eq!(msg.id.get_html(bob).await?.unwrap(), html);
+        Ok(())
     }
 
     #[tokio::test(flavor = "multi_thread", worker_threads = 2)]

src/message.rs

@@ -980,7 +980,7 @@ impl Message {
 
     /// Returns true if the message is a forwarded message.
     pub fn is_forwarded(&self) -> bool {
-        0 != self.param.get_int(Param::Forwarded).unwrap_or_default()
+        self.param.get_int(Param::Forwarded).is_some()
     }
 
     /// Returns true if the message is edited.

src/mimefactory.rs

@@ -1934,15 +1934,19 @@ impl MimeFactory {
 
         let mut parts = Vec::new();
 
-        // add HTML-part, this is needed only if a HTML-message from a non-delta-client is forwarded;
-        // for simplificity and to avoid conversion errors, we're generating the HTML-part from the original message.
         if msg.has_html() {
-            let html = if let Some(orig_msg_id) = msg.param.get_int(Param::Forwarded) {
+            let html = if let Some(html) = msg.param.get(Param::SendHtml) {
+                Some(html.to_string())
+            } else if let Some(orig_msg_id) = msg.param.get_int(Param::Forwarded)
+                && orig_msg_id != 0
+            {
+                // Legacy forwarded messages may not have `Param::SendHtml` set. Let's hope the
+                // original message exists.
                 MsgId::new(orig_msg_id.try_into()?)
                     .get_html(context)
                     .await?
             } else {
-                msg.param.get(Param::SendHtml).map(|s| s.to_string())
+                None
             };
             if let Some(html) = html {
                 main_part = MimePart::new(