Take plain body from OOM and not MAPI
authorAndre Heinecke <aheinecke@intevation.de>
Tue, 12 Dec 2017 07:23:04 +0000 (08:23 +0100)
committerAndre Heinecke <aheinecke@intevation.de>
Tue, 12 Dec 2017 07:23:04 +0000 (08:23 +0100)
* src/mail.cpp (Mail::Mail): Initialize cache variables.
(Mail::take_cached_html_body): Optimize as take to avoid
string copies.
(Mail::take_cached_plain_body): New.
(Mail::update_oom_data): Also cache plain body.
* src/mail.h: Update accordingly.
* src/mimemaker.cpp (add_body): Adapt to take html
body logic.
(do_mime_sign, mime_encrypt): Prefer cached body over MAPI.

--
At least for multipart/alternative mails the MAPI property
is not updated correctly if the mail was saved as draft.
As we know that the update_oom_data / caching logic works
nicely for the HTML body we can use a similar logic for
the plain body to avoid any MAPI weirdness.

GnuPG-Bug-Id: T3419

src/mail.cpp
src/mail.h
src/mimemaker.cpp

index be7be31..266a428 100644 (file)
@@ -130,6 +130,8 @@ Mail::Mail (LPDISPATCH mailitem) :
     m_needs_encrypt(false),
     m_moss_position(0),
     m_crypto_flags(0),
+    m_cached_html_body(nullptr),
+    m_cached_plain_body(nullptr),
     m_type(MSGTYPE_UNKNOWN),
     m_do_inline(false),
     m_is_gsuite(false)
@@ -1144,12 +1146,11 @@ Mail::update_oom_data ()
   if (m_is_html_alternative)
     {
       log_debug ("%s:%s: Is html alternative mail.", SRCNAME, __func__);
-      const char * html_body = get_oom_string (m_mailitem, "HTMLBody");
-      if (html_body)
-        {
-          m_html_body = html_body;
-        }
+      xfree (m_cached_html_body);
+      m_cached_html_body = get_oom_string (m_mailitem, "HTMLBody");
     }
+  xfree (m_cached_plain_body);
+  m_cached_plain_body = get_oom_string (m_mailitem, "Body");
   /* For some reason outlook may store the recipient address
      in the send using account field. If we have SMTP we prefer
      the SenderEmailAddress string. */
@@ -2356,10 +2357,20 @@ Mail::is_html_alternative () const
   return m_is_html_alternative;
 }
 
-const std::string &
-Mail::get_cached_html_body () const
+char *
+Mail::take_cached_html_body ()
 {
-  return m_html_body;
+  char *ret = m_cached_html_body;
+  m_cached_html_body = nullptr;
+  return ret;
+}
+
+char *
+Mail::take_cached_plain_body ()
+{
+  char *ret = m_cached_plain_body;
+  m_cached_plain_body = nullptr;
+  return ret;
 }
 
 int
index d26f477..28b4bb5 100644 (file)
@@ -304,8 +304,15 @@ public:
     Only valid if update_oom_data was called before. */
   bool is_html_alternative () const;
 
-  /** Get the html body. It is updated in update_oom_data. */
-  const std::string & get_cached_html_body () const;
+  /** Get the html body. It is updated in update_oom_data.
+      Caller takes ownership of the string and has to free it.
+  */
+  char *take_cached_html_body ();
+
+  /** Get the plain body. It is updated in update_oom_data.
+      Caller takes ownership of the string and has to free it.
+  */
+  char *take_cached_plain_body ();
 
   /** Returns 1 if the mail was encrypted, 2 if signed, 3 if both.
       Only valid after decrypt_verify.
@@ -349,7 +356,8 @@ private:
   int m_moss_position; /* The number of the original message attachment. */
   int m_crypto_flags;
   std::string m_sender;
-  std::string m_html_body; /* Cached html body. */
+  char *m_cached_html_body; /* Cached html body. */
+  char *m_cached_plain_body; /* Cached plain body. */
   msgtype_t m_type; /* Our messagetype as set in mapi */
   std::shared_ptr <ParseController> m_parser;
   GpgME::VerificationResult m_verify_result;
index ba9bddd..fe8d4b5 100644 (file)
@@ -1366,17 +1366,19 @@ add_body (Mail *mail, const char *boundary, sink_t sink,
     }
 
   /* Now the html body. It is somehow not accessible through PR_HTML,
-     OutlookSpy also shows MAPI Unsported (but shows the data) strange.
+     OutlookSpy also shows MAPI Unsupported (but shows the data) strange.
      We just cache it. Memory is cheap :-) */
-  const auto html_body = mail->get_cached_html_body();
-  if (html_body.empty())
+  char *html_body = mail->take_cached_html_body();
+  if (!html_body)
     {
       log_error ("%s:%s: BUG: Body but no html body in alternative mail?",
                  SRCNAME, __func__);
+      return -1;
     }
 
-  rc = write_part (sink, html_body.c_str(), html_body.size(),
+  rc = write_part (sink, html_body, strlen (html_body),
                    alt_boundary, NULL, 2);
+  xfree (html_body);
   if (rc)
     {
       TRACEPOINT;
@@ -1521,8 +1523,18 @@ do_mime_sign (LPMESSAGE message, HWND hwnd, protocol_t protocol,
         return -1;
     }
 
+  /* Take the Body from the mail if possible. This is a fix for
+     GnuPG-Bug-ID: T3614 because the body is not always properly
+     updated in MAPI when sending. */
+  if (mail)
+    {
+      body = mail->take_cached_plain_body ();
+    }
   /* Get the attachment info and the body.  */
-  body = mapi_get_body (message, NULL);
+  if (!body)
+    {
+      body = mapi_get_body (message, NULL);
+    }
   if (body && !*body)
     {
       xfree (body);
@@ -1948,12 +1960,24 @@ mime_encrypt (LPMESSAGE message, HWND hwnd,
     return -1;
 
   /* Get the attachment info and the body.  We need to do this before
-     creating the engine's filter sue problem sending the cancel to
-     the engine with nothing for the engine to process.  This is
-     actually a bug in our engine code but we better avoid triggering
-     this bug because the engine sometimes hangs.  Fixme: Needs a
-     proper fix. */
-  body = mapi_get_body (message, NULL);
+     creating the engine's filter because sending the cancel to
+     the engine with nothing for the engine to process.  Will result
+     in an error. This is actually a bug in our engine code but
+     we better avoid triggering this bug because the engine
+     sometimes hangs.  Fixme: Needs a proper fix. */
+
+
+  /* Take the Body from the mail if possible. This is a fix for
+     GnuPG-Bug-ID: T3614 because the body is not always properly
+     updated in MAPI when sending. */
+  if (mail)
+    {
+      body = mail->take_cached_plain_body ();
+    }
+  if (!body)
+    {
+      body = mapi_get_body (message, NULL);
+    }
   if (body && !*body)
     {
       xfree (body);