forward html-messages properly (#2151)

* add failing tests for forwarding html-mails

* let MsgId.get_html() return an option

* write html-part to local database on forwarding

* add html-part to forwarded non-dc messages

* read HTML-parts from encrypted messages

* avoid clone()

* Received:-header is no longer needed since #2152

* Update src/html.rs

Co-authored-by: Floris Bruynooghe <flub@devork.be>

* Update src/html.rs

Co-authored-by: Floris Bruynooghe <flub@devork.be>

* Update src/mimeparser.rs

Co-authored-by: Floris Bruynooghe <flub@devork.be>

* prefer 'orig' over 'org' as abbreviation for 'original'

* improve comment on tests

* prefer 'try_into()' over 'as u32' to avoid panics on bad data

* simplify ffi

Co-authored-by: Floris Bruynooghe <flub@devork.be>

examples/repl/cmdline.rs

@@ -951,7 +951,7 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu
             let file = dirs::home_dir()
                 .unwrap_or_default()
                 .join(format!("msg-{}.html", id.to_u32()));
-            let html = id.get_html(&context).await;
+            let html = id.get_html(&context).await.unwrap_or_default();
             fs::write(&file, html)?;
             println!("HTML written to: {:#?}", file);
         }

src/chat.rs

@@ -1060,6 +1060,12 @@ impl Chat {
             EphemeralTimer::Enabled { duration } => time() + i64::from(duration),
         };
 
+        let new_mime_headers = if msg.param.exists(Param::Forwarded) && msg.mime_modified {
+            msg.get_id().get_html_as_rawmime(context).await
+        } else {
+            None
+        };
+
         // add message to the database
 
         if context
@@ -1078,10 +1084,12 @@ impl Chat {
                         hidden,
                         mime_in_reply_to,
                         mime_references,
+                        mime_modified,
+                        mime_headers,
                         location_id,
                         ephemeral_timer,
                         ephemeral_timestamp)
-                        VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);",
+                        VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?,?);",
                 paramsv![
                     new_rfc724_mid,
                     self.id,
@@ -1095,6 +1103,8 @@ impl Chat {
                     msg.hidden,
                     msg.in_reply_to.as_deref().unwrap_or_default(),
                     new_references,
+                    new_mime_headers.is_some(),
+                    new_mime_headers,
                     location_id as i32,
                     ephemeral_timer,
                     ephemeral_timestamp
@@ -2738,7 +2748,8 @@ pub async fn forward_msgs(
             // we tested a sort of broadcast
             // by not marking own forwarded messages as such,
             // however, this turned out to be to confusing and unclear.
-            msg.param.set_int(Param::Forwarded, 1);
+            msg.param
+                .set_int(Param::Forwarded, src_msg_id.to_u32() as i32);
 
             msg.param.remove(Param::GuaranteeE2ee);
             msg.param.remove(Param::ForcePlaintext);

src/dc_receive_imf.rs

@@ -822,16 +822,16 @@ async fn add_parts(
     // if indicated by the parser,
     // we save the full mime-message and add a flag
     // that the ui should show button to display the full message.
-    //
-    // (currently, we skip saving mime-messages for encrypted messages
-    // as there is probably no huge intersection between html-messages and encrypted messages,
-    // however, that should be doable, we need the decrypted mime-structure in this case)
 
     // a flag used to avoid adding "show full message" button to multiple parts of the message.
-    let mut save_mime_modified = mime_parser.is_mime_modified && !mime_parser.was_encrypted();
+    let mut save_mime_modified = mime_parser.is_mime_modified;
 
     let mime_headers = if save_mime_headers || save_mime_modified {
-        Some(String::from_utf8_lossy(imf_raw).to_string())
+        if mime_parser.was_encrypted() {
+            Some(String::from_utf8_lossy(&mime_parser.decoded_data).to_string())
+        } else {
+            Some(String::from_utf8_lossy(imf_raw).to_string())
+        }
     } else {
         None
     };

src/html.rs

@@ -18,6 +18,7 @@ use crate::headerdef::{HeaderDef, HeaderDefMap};
 use crate::message::{Message, MsgId};
 use crate::mimeparser::parse_message_id;
 use crate::plaintext::PlainText;
+use lettre_email::PartBuilder;
 use mailparse::ParsedContentType;
 
 impl Message {
@@ -221,7 +222,7 @@ impl MsgId {
     /// 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).
     /// The corresponding ffi-function is `dc_get_msg_html()`.
-    pub async fn get_html(self, context: &Context) -> String {
+    pub async fn get_html(self, context: &Context) -> Option<String> {
         let rawmime: Option<String> = context
             .sql
             .query_get_value(
@@ -232,19 +233,52 @@ impl MsgId {
             .await;
 
         if let Some(rawmime) = rawmime {
-            match HtmlMsgParser::from_bytes(context, rawmime.as_bytes()).await {
-                Err(err) => format!("parser error: {}", err),
-                Ok(parser) => parser.html,
+            if !rawmime.is_empty() {
+                match HtmlMsgParser::from_bytes(context, rawmime.as_bytes()).await {
+                    Err(err) => {
+                        warn!(context, "get_html: parser error: {}", err);
+                        None
+                    }
+                    Ok(parser) => Some(parser.html),
+                }
+            } else {
+                warn!(context, "get_html: empty mime for {}", self);
+                None
             }
         } else {
-            format!("parser error: no mime for {}", self)
+            warn!(context, "get_html: no mime for {}", self);
+            None
         }
     }
+
+    /// Wraps HTML generated by [`MsgId::get_html`] into a text/html mimepart structure.
+    ///
+    /// Used on forwarding messages to avoid leaking the original mime structure
+    /// and also to avoid sending too much, maybe large data.
+    pub async fn get_html_as_mimepart(self, context: &Context) -> Option<PartBuilder> {
+        self.get_html(context).await.map(|s| {
+            PartBuilder::new()
+                .content_type(&"text/html; charset=utf-8".parse::<mime::Mime>().unwrap())
+                .body(s)
+        })
+    }
+
+    // As [`MsgId::get_html_as_mimepart`] but wraps [`MsgId::get_html`] into text/html mime raw string.
+    pub async fn get_html_as_rawmime(self, context: &Context) -> Option<String> {
+        self.get_html_as_mimepart(context)
+            .await
+            .map(|p| p.build().as_string())
+    }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::chat::forward_msgs;
+    use crate::config::Config;
+    use crate::constants::DC_CONTACT_ID_SELF;
+    use crate::dc_receive_imf::dc_receive_imf;
+    use crate::message::MessengerMessage;
     use crate::test_utils::TestContext;
 
     #[async_std::test]
@@ -393,4 +427,96 @@ test some special html-characters as &lt; &gt; and &amp; but also &quot; and &#x
             .is_some());
         assert!(parser.html.find("cid:").is_none());
     }
+
+    #[async_std::test]
+    async fn test_get_html_empty() {
+        let t = TestContext::new().await;
+        let msg_id = MsgId::new_unset();
+        assert!(msg_id.get_html(&t).await.is_none())
+    }
+
+    #[async_std::test]
+    async fn test_html_forwarding() {
+        // alice receives a non-delta html-message
+        let alice = TestContext::new_alice().await;
+        alice.set_config(Config::ShowEmails, Some("2")).await.ok();
+        let chat = alice.chat_with_contact("", "sender@testrun.org").await;
+        let raw = include_bytes!("../test-data/message/text_alt_plain_html.eml");
+        dc_receive_imf(&alice, raw, "INBOX", 1, false)
+            .await
+            .unwrap();
+        let msg = alice.get_last_msg_in(chat.get_id()).await;
+        assert_ne!(msg.get_from_id(), DC_CONTACT_ID_SELF);
+        assert_eq!(msg.is_dc_message, MessengerMessage::No);
+        assert!(!msg.is_forwarded());
+        assert!(msg.get_text().unwrap().find("this is plain").is_some());
+        assert!(msg.has_html());
+        let html = msg.get_id().get_html(&alice).await.unwrap();
+        assert!(html.find("this is <b>html</b>").is_some());
+
+        // alice: create chat with bob and forward received html-message there
+        let chat = alice.chat_with_contact("", "bob@example.net").await;
+        forward_msgs(&alice, &[msg.get_id()], chat.get_id())
+            .await
+            .unwrap();
+        let msg = alice.get_last_msg_in(chat.get_id()).await;
+        assert_eq!(msg.get_from_id(), DC_CONTACT_ID_SELF);
+        assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
+        assert!(msg.is_forwarded());
+        assert!(msg.get_text().unwrap().find("this is plain").is_some());
+        assert!(msg.has_html());
+        let html = msg.get_id().get_html(&alice).await.unwrap();
+        assert!(html.find("this is <b>html</b>").is_some());
+
+        // bob: check that bob also got the html-part of the forwarded message
+        let bob = TestContext::new_bob().await;
+        let chat = bob.chat_with_contact("", "alice@example.com").await;
+        bob.recv_msg(&alice.pop_sent_msg().await).await;
+        let msg = bob.get_last_msg_in(chat.get_id()).await;
+        assert_ne!(msg.get_from_id(), DC_CONTACT_ID_SELF);
+        assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
+        assert!(msg.is_forwarded());
+        assert!(msg.get_text().unwrap().find("this is plain").is_some());
+        assert!(msg.has_html());
+        let html = msg.get_id().get_html(&bob).await.unwrap();
+        assert!(html.find("this is <b>html</b>").is_some());
+    }
+
+    #[async_std::test]
+    async fn test_html_forwarding_encrypted() {
+        // Alice receives a non-delta html-message
+        // (`ShowEmails=1` lets Alice actually receive non-delta messages for known contacts,
+        // the contact is marked as known by creating a chat using `chat_with_contact()`)
+        let alice = TestContext::new_alice().await;
+        alice.set_config(Config::ShowEmails, Some("1")).await.ok();
+        let chat = alice.chat_with_contact("", "sender@testrun.org").await;
+        let raw = include_bytes!("../test-data/message/text_alt_plain_html.eml");
+        dc_receive_imf(&alice, raw, "INBOX", 1, false)
+            .await
+            .unwrap();
+        let msg = alice.get_last_msg_in(chat.get_id()).await;
+
+        // forward the message to saved-messages,
+        // this will encrypt the message as new_alice() has set up keys
+        let chat = alice.get_self_chat().await;
+        forward_msgs(&alice, &[msg.get_id()], chat.get_id())
+            .await
+            .unwrap();
+        let msg = alice.pop_sent_msg().await;
+
+        // receive the message on another device
+        let alice = TestContext::new_alice().await;
+        assert_eq!(alice.get_config_int(Config::ShowEmails).await, 0); // set to "1" above, make sure it is another db
+        alice.recv_msg(&msg).await;
+        let chat = alice.get_self_chat().await;
+        let msg = alice.get_last_msg_in(chat.get_id()).await;
+        assert_eq!(msg.get_from_id(), DC_CONTACT_ID_SELF);
+        assert_eq!(msg.is_dc_message, MessengerMessage::Yes);
+        assert!(msg.get_showpadlock());
+        assert!(msg.is_forwarded());
+        assert!(msg.get_text().unwrap().find("this is plain").is_some());
+        assert!(msg.has_html());
+        let html = msg.get_id().get_html(&alice).await.unwrap();
+        assert!(html.find("this is <b>html</b>").is_some());
+    }
 }

src/mimefactory.rs

@@ -15,12 +15,13 @@ use crate::ephemeral::Timer as EphemeralTimer;
 use crate::error::{bail, ensure, format_err, Error};
 use crate::format_flowed::{format_flowed, format_flowed_quote};
 use crate::location;
-use crate::message::{self, Message};
+use crate::message::{self, Message, MsgId};
 use crate::mimeparser::SystemMessage;
 use crate::param::Param;
 use crate::peerstate::{Peerstate, PeerstateVerifiedStatus};
 use crate::simplify::escape_message_footer_marks;
 use crate::stock::StockMessage;
+use std::convert::TryInto;
 
 // attachments of 25 mb brutto should work on the majority of providers
 // (brutto examples: web.de=50, 1&1=40, t-online.de=32, gmail=25, posteo=50, yahoo=25, all-inkl=100).
@@ -954,7 +955,7 @@ impl<'a, 'b> MimeFactory<'a, 'b> {
         );
 
         // Message is sent as text/plain, with charset = utf-8
-        let main_part = PartBuilder::new()
+        let mut main_part = PartBuilder::new()
             .header((
                 "Content-Type".to_string(),
                 "text/plain; charset=utf-8; format=flowed; delsp=no".to_string(),
@@ -962,6 +963,20 @@ impl<'a, 'b> MimeFactory<'a, 'b> {
             .body(message_text);
         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 self.msg.has_html() {
+            if let Some(orig_msg_id) = self.msg.param.get_int(Param::Forwarded) {
+                let orig_msg_id = MsgId::new(orig_msg_id.try_into()?);
+                if let Some(html_part) = orig_msg_id.get_html_as_mimepart(context).await {
+                    main_part = PartBuilder::new()
+                        .message_type(MimeMultipartType::Alternative)
+                        .child(main_part.build())
+                        .child(html_part.build());
+                }
+            }
+        }
+
         // add attachment part
         if chat::msgtype_has_file(self.msg.viewtype) {
             if !is_file_size_okay(context, &self.msg).await {

src/mimeparser.rs

@@ -69,6 +69,12 @@ pub struct MimeMessage {
     // if this flag is set, the parts/text/etc. are just close to the original mime-message;
     // clients should offer a way to view the original message in this case
     pub is_mime_modified: bool,
+
+    /// The decrypted, raw mime structure.
+    ///
+    /// This is non-empty only if the message was actually encrypted.  It is used
+    /// for e.g. late-parsing HTML.
+    pub decoded_data: Vec<u8>,
 }
 
 #[derive(Debug, PartialEq)]
@@ -136,7 +142,7 @@ impl MimeMessage {
         headers.remove("chat-verified");
 
         // Memory location for a possible decrypted message.
-        let mail_raw;
+        let mut mail_raw = Vec::new();
         let mut gossipped_addr = Default::default();
 
         let (mail, signatures, warn_empty_signature) =
@@ -228,6 +234,7 @@ impl MimeMessage {
             group_avatar: None,
             failure_report: None,
             is_mime_modified: false,
+            decoded_data: Vec::new(),
         };
         parser.parse_mime_recursive(context, &mail).await?;
         parser.maybe_remove_bad_parts();
@@ -240,6 +247,10 @@ impl MimeMessage {
             }
         }
 
+        if parser.is_mime_modified {
+            parser.decoded_data = mail_raw;
+        }
+
         Ok(parser)
     }
 

src/param.rs

@@ -50,7 +50,8 @@ pub enum Param {
     /// For Messages
     WantsMdn = b'r',
 
-    /// For Messages
+    /// For Messages: unset or 0=not forwarded,
+    /// 1=forwarded from unknown msg_id, >9 forwarded from msg_id
     Forwarded = b'a',
 
     /// For Messages: quoted text.

test-data/message/text_alt_plain_html.eml

@@ -1,7 +1,7 @@
 Subject: mime-modified test
 Message-ID: 12345@testrun.org
 Date: Sat, 07 Dec 2019 19:00:27 +0000
-To: recp@testrun.org
+To: alice@example.com
 From: sender@testrun.org
 Content-Type: multipart/alternative; boundary="==BREAK=="