Improve recipient handling
authorAndre Heinecke <aheinecke@intevation.de>
Tue, 10 Jul 2018 08:50:33 +0000 (10:50 +0200)
committerAndre Heinecke <aheinecke@intevation.de>
Tue, 10 Jul 2018 08:50:33 +0000 (10:50 +0200)
* src/cryptcontroller.cpp (CryptController::CryptController),
src/keycache.cpp (KeyCache::isMailResolvable),
src/wks-helper.cpp (WKSHelper::handle_confirmation_read):
Update call to mail->getRecipients.
* src/mail.h (Mail::getRecipients): Renamed to
Mail::getCachedRecipients.
* src/mail.cpp (Mail::getRecipients_o): Adapt to vector.
Add error handling.
(Mail::updateOOMData_o): Use vector directly.
* src/oomhelp.cpp, src/oomhelp.h (get_oom_recipients):
Use vector and provide an error.

--
Downstream we already used the recipients mostly as a
vector anyway to avoid the memory mangement hassle.
This gives us more flexibility.

Additionally we now report an error for the case
from:
GnuPG-Bug-Id: T4065

src/cryptcontroller.cpp
src/keycache.cpp
src/mail.cpp
src/mail.h
src/oomhelp.cpp
src/oomhelp.h
src/wks-helper.cpp

index 025a47d..9a9d4d6 100644 (file)
@@ -70,7 +70,7 @@ CryptController::CryptController (Mail *mail, bool encrypt, bool sign,
 {
   log_debug ("%s:%s: CryptController ctor for %p encrypt %i sign %i inline %i.",
              SRCNAME, __func__, mail, encrypt, sign, mail->getDoPGPInline ());
-  m_recipient_addrs = vector_to_cArray (mail->getRecipients ());
+  m_recipient_addrs = vector_to_cArray (mail->getCachedRecipients ());
 }
 
 CryptController::~CryptController()
index 0e53c2d..72eea28 100644 (file)
@@ -626,7 +626,7 @@ KeyCache::isMailResolvable(Mail *mail)
 {
   /* Get the data from the mail. */
   const auto sender = mail->getSender ();
-  auto recps = mail->getRecipients ();
+  auto recps = mail->getCachedRecipients ();
 
   if (sender.empty() || recps.empty())
     {
index 6850c69..aaa15c5 100644 (file)
@@ -1527,9 +1527,7 @@ Mail::updateOOMData_o ()
       xfree (m_cached_plain_body);
       m_cached_plain_body = get_oom_string (m_mailitem, "Body");
 
-      char **recipients = getRecipients_o ();
-      m_cached_recipients = cArray_to_vector ((const char **)recipients);
-      xfree (recipients);
+      m_cached_recipients = getRecipients_o ();
     }
   /* For some reason outlook may store the recipient address
      in the send using account field. If we have SMTP we prefer
@@ -1779,17 +1777,34 @@ Mail::getHTMLBody_o () const
   return get_string_o (m_mailitem, "HTMLBody");
 }
 
-char **
+std::vector<std::string>
 Mail::getRecipients_o () const
 {
   LPDISPATCH recipients = get_oom_object (m_mailitem, "Recipients");
   if (!recipients)
     {
       TRACEPOINT;
-      return nullptr;
+      std::vector<std::string>();
     }
-  auto ret = get_oom_recipients (recipients);
+  bool err = false;
+  auto ret = get_oom_recipients (recipients, &err);
   gpgol_release (recipients);
+
+  if (err)
+    {
+      /* Should not happen. But we add it for better bug reports. */
+      const char *bugmsg = utf8_gettext ("Operation failed.\n\n"
+              "This is usually caused by a bug in GpgOL or an error in your setup.\n"
+              "Please see https://www.gpg4win.org/reporting-bugs.html "
+              "or ask your Administrator for support.");
+      char *buf;
+      gpgrt_asprintf (&buf, "Failed to resolve recipients.\n\n%s\n", bugmsg);
+      gpgol_message_box (get_active_hwnd (),
+                         buf,
+                         _("GpgOL"), MB_OK);
+      xfree(buf);
+    }
+
   return ret;
 }
 
@@ -2704,7 +2719,7 @@ Mail::locateKeys_o ()
   updateOOMData_o ();
   KeyCache::instance()->startLocateSecret (getSender_o ().c_str (), this);
   KeyCache::instance()->startLocate (getSender_o ().c_str (), this);
-  KeyCache::instance()->startLocate (getRecipients (), this);
+  KeyCache::instance()->startLocate (getCachedRecipients (), this);
   autoresolveCheck ();
 
   locate_in_progress = false;
@@ -2751,7 +2766,7 @@ Mail::getNeedsEncrypt () const
 }
 
 std::vector<std::string>
-Mail::getRecipients ()
+Mail::getCachedRecipients ()
 {
   return m_cached_recipients;
 }
index c6ecd2c..ef04c9f 100644 (file)
@@ -394,10 +394,8 @@ public:
   /** Get the html of the mail */
   std::string getHTMLBody_o () const;
 
-  /** Get the recipients recipients is a null
-      terminated array of strings. Needs to be freed
-      by the caller. */
-  char ** getRecipients_o () const;
+  /** Get the recipients. */
+  std::vector<std::string> getRecipients_o () const;
 
   /** Try to locate the keys for all recipients */
   void locateKeys_o ();
@@ -421,7 +419,7 @@ public:
   char *takeCachedPlainBody ();
 
   /** Get the cached recipients. It is updated in update_oom_data.*/
-  std::vector<std::string> getRecipients ();
+  std::vector<std::string> getCachedRecipients ();
 
   /** Returns 1 if the mail was encrypted, 2 if signed, 3 if both.
       Only valid after decrypt_verify.
index 0826f49..8ed1842 100644 (file)
@@ -1167,23 +1167,20 @@ get_recipient_addr_fallbacks (LPDISPATCH recipient)
   return nullptr;
 }
 
-/* Gets a malloced NULL terminated array of recipent strings from
-   an OOM recipients Object. */
-char **
-get_oom_recipients (LPDISPATCH recipients)
+/* Gets the resolved smtp addresses of the recpients. */
+std::vector<std::string>
+get_oom_recipients (LPDISPATCH recipients, bool *r_err)
 {
   int recipientsCnt = get_oom_int (recipients, "Count");
-  char **recipientAddrs = NULL;
+  std::vector<std::string> ret;
   int i;
 
   if (!recipientsCnt)
     {
-      return NULL;
+      return ret;
     }
 
   /* Get the recipients */
-  recipientAddrs = (char**) xmalloc((recipientsCnt + 1) * sizeof(char*));
-  recipientAddrs[recipientsCnt] = NULL;
   for (i = 1; i <= recipientsCnt; i++)
     {
       char buf[16];
@@ -1193,15 +1190,19 @@ get_oom_recipients (LPDISPATCH recipients)
       if (!recipient)
         {
           /* Should be impossible */
-          recipientAddrs[i-1] = NULL;
           log_error ("%s:%s: could not find Item %i;",
                      SRCNAME, __func__, i);
+          if (r_err)
+            {
+              *r_err = true;
+            }
           break;
         }
       char *resolved = get_pa_string (recipient, PR_SMTP_ADDRESS_DASL);
       if (resolved)
         {
-          recipientAddrs[i-1] = resolved;
+          ret.push_back (resolved);
+          xfree (resolved);
           gpgol_release (recipient);
           continue;
         }
@@ -1209,7 +1210,8 @@ get_oom_recipients (LPDISPATCH recipients)
       resolved = get_recipient_addr_fallbacks (recipient);
       if (resolved)
         {
-          recipientAddrs[i-1] = resolved;
+          ret.push_back (resolved);
+          xfree (resolved);
           gpgol_release (recipient);
           continue;
         }
@@ -1218,9 +1220,18 @@ get_oom_recipients (LPDISPATCH recipients)
       gpgol_release (recipient);
       log_debug ("%s:%s: Failed to look up Address probably EX addr is returned",
                  SRCNAME, __func__);
-      recipientAddrs[i-1] = address;
+      if (address)
+        {
+          ret.push_back (address);
+          xfree (address);
+        }
+      else if (r_err)
+        {
+          *r_err = true;
+        }
     }
-  return recipientAddrs;
+
+  return ret;
 }
 
 /* Add an attachment to the outlook dispatcher disp
index 1136992..2fea2ce 100644 (file)
@@ -25,6 +25,9 @@
 #include <unknwn.h>
 #include "mymapi.h"
 
+#include <vector>
+#include <string>
+
 #define MSOCONTROLBUTTON    1
 #define MSOCONTROLEDIT      2
 #define MSOCONTROLDROPDOWN  3
@@ -172,8 +175,10 @@ void del_oom_button (LPDISPATCH button);
 /* Get the HWND of the active window in the current context */
 HWND get_oom_context_window (LPDISPATCH context);
 
-/* Get the address of the recipients as string list */
-char ** get_oom_recipients (LPDISPATCH recipients);
+/* Get the address of the recipients as string list.
+   If r_err is not null it is set to true in case of an error. */
+std::vector<std::string> get_oom_recipients (LPDISPATCH recipients,
+                                             bool *r_err = nullptr);
 
 /* Add an attachment to a dispatcher */
 int
index 7edb0ea..109ecab 100644 (file)
@@ -797,21 +797,19 @@ WKSHelper::handle_confirmation_read (Mail *mail, LPSTREAM stream) const
     }
 
   /* Get the recipient of the confirmation mail */
-  char **recipients = mail->getRecipients_o ();
+  const auto recipients = mail->getRecipients_o ();
 
   /* We assert that we have one recipient as the mail should have been
      sent by the wks-server. */
-  if (!recipients || !recipients[0] || recipients[1])
+  if (recipients.size() != 1)
     {
       log_error ("%s:%s: invalid recipients",
                  SRCNAME, __func__);
-      release_cArray (recipients);
       gpgol_release (stream);
       return;
     }
 
   std::string mbox = recipients[0];
-  release_cArray (recipients);
 
   /* Prepare stdin for the wks-client process */