[git] GpgOL - branch, master, updated. gpgol-2.2.0-58-g514a0d5

by Andre Heinecke cvs at cvs.gnupg.org
Tue Jul 17 14:54:24 CEST 2018


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GnuPG extension for MS Outlook".

The branch, master has been updated
       via  514a0d5d0a1fba979fb2f774aa3b1d241c64cf17 (commit)
       via  34096e5a94a76b3f71270e4c39f7d2f333e0430b (commit)
       via  f847b6e6d244558a1b04ae38a1b267e03a86ce3a (commit)
       via  cf28a7508d65deaeb953074b7ddbd5b064d39177 (commit)
       via  3a8b92c4ee02cc08875e009527f1d8169ff65663 (commit)
       via  22996594617e69285ea4ddef5102fd8ecf7d017b (commit)
       via  6d9156a806c096e197f5efbdaefa84586db48580 (commit)
      from  7e76531304ec9fbd5227ed511cea0f0df68fa9e0 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 514a0d5d0a1fba979fb2f774aa3b1d241c64cf17
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Tue Jul 17 14:53:24 2018 +0200

    Fix possible invalid CloseHandle and DeleteFile
    
    * src/mail.cpp (add_attachments_o): Guard CloseHandle and
    DeleteFile.
    
    --
    CloseHandle may only be called on valid handles.

diff --git a/src/mail.cpp b/src/mail.cpp
index 1843543..84776ea 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -687,8 +687,11 @@ add_attachments_o(LPDISPATCH mail,
                      SRCNAME, __func__, dispName.c_str());
           err = 1;
         }
-      CloseHandle (hFile);
-      if (!DeleteFileW (wchar_file))
+      if (hFile && hFile != INVALID_HANDLE_VALUE)
+        {
+          CloseHandle (hFile);
+        }
+      if (wchar_file && !DeleteFileW (wchar_file))
         {
           log_error ("%s:%s: Failed to delete tmp attachment for: %s",
                      SRCNAME, __func__, dispName.c_str());

commit 34096e5a94a76b3f71270e4c39f7d2f333e0430b
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Tue Jul 17 14:43:12 2018 +0200

    Cleanup shutdown and fix potential crashes
    
    * src/explorer-events.cpp (Close): Unregister sink with addin.
    * src/explorers-events.cpp (NewInstance): Register sink.
    * src/gpgoladdin.cpp (GpgolAddin::~GpgolAddin): Remove shutdown
    logic.
    (GpgolAddin::OnConnection): Mark shutdown state as false.
    (GpgolAddin::OnDisconnection): Call shutdown.
    (install_explorer_sinks): Register created sinks.
    (GpgolAddin::shutdown): New. Handle shutdown.
    (GpgolAddin::registerExplorerSink),
    (GpgolAddin::unregisterExplorerSink): Track explorer sinks.
    * src/gpgoladdin.h: Update accordingly.
    * src/mail.cpp (Mail::closeAllMails_o): Fix folder events
    detach and do it earlier.
    (Mail::installFolderEventHandler_o): Use sink instead of folder
    to track event handler.
    * src/windowmessages.cpp (add_explorer, remove_explorer): removed.
    (gpgol_hook): Interate over actual explorers instead of variables.
    * src/windowmessages.h: Update accordingly.
    
    --
    This fixes Explorer events release / deletion on unload / shutdown.
    Previously this was only done on close, which came after GpgOL
    was unloaded. Maybe this resulted in a jump to an unmapped location?
    
    It also fixes that the gpgol_hook was only enabled for all explorers
    at start time. Maybe we will see a regression now that it works
    as intended on all explorers.
    
    Explorers Events and Application Events were never deleted / released
    because they were not detached before release. (But the release
    then didn't delete them).
    
    Definitely fixed is a race condition on shutdown which would be:
    -> Shutdown detected
    -> Close all mails
    -> The currently visible mail is closed
    -> Outlook loads it again because it is currently visible
    -> But we are in shutdown mode.
    
    Now we would start to parse while actually shutting down. This
    could lead to hangs / crashes. The reason for this was that
    we closed the mails before we detached the Application event
    handler and thus captured the item load again.

diff --git a/src/explorer-events.cpp b/src/explorer-events.cpp
index 03969b1..b9706db 100644
--- a/src/explorer-events.cpp
+++ b/src/explorer-events.cpp
@@ -92,7 +92,7 @@ EVENT_SINK_INVOKE(ExplorerEvents)
           log_oom_extra ("%s:%s: Deleting event handler: %p",
                          SRCNAME, __func__, this);
 
-          remove_explorer (m_object);
+          GpgolAddin::get_instance ()->unregisterExplorerSink (this);
           delete this;
           return S_OK;
         }
diff --git a/src/explorers-events.cpp b/src/explorers-events.cpp
index d56b9a8..74dccf1 100644
--- a/src/explorers-events.cpp
+++ b/src/explorers-events.cpp
@@ -60,13 +60,14 @@ EVENT_SINK_INVOKE(ExplorersEvents)
                          SRCNAME, __func__);
               break;
             }
-          if (!install_ExplorerEvents_sink (parms->rgvarg[0].pdispVal))
+          LPDISPATCH expSink = install_ExplorerEvents_sink (parms->rgvarg[0].pdispVal);
+          if (!expSink)
             {
               log_error ("%s:%s: Failed to install Explorer event sink.",
                          SRCNAME, __func__);
               break;
-
             }
+          GpgolAddin::get_instance()->registerExplorerSink (expSink);
         }
       default:
         break;
diff --git a/src/gpgoladdin.cpp b/src/gpgoladdin.cpp
index 15fb475..7d85267 100644
--- a/src/gpgoladdin.cpp
+++ b/src/gpgoladdin.cpp
@@ -170,6 +170,7 @@ GpgolAddin::GpgolAddin (void) : m_lRef(0),
   m_applicationEventSink(nullptr),
   m_explorersEventSink(nullptr),
   m_disabled(false),
+  m_shutdown(false),
   m_hook(nullptr)
 {
   read_options ();
@@ -185,15 +186,6 @@ GpgolAddin::~GpgolAddin (void)
     {
       return;
     }
-  log_debug ("%s:%s: Releasing Application Event Sink;",
-             SRCNAME, __func__);
-  gpgol_release (m_explorersEventSink);
-  gpgol_release (m_applicationEventSink);
-
-  write_options ();
-
-  UnhookWindowsHookEx (m_hook);
-
   addin_instance = NULL;
 
   log_debug ("%s:%s: Object deleted\n", SRCNAME, __func__);
@@ -316,6 +308,7 @@ GpgolAddin::OnConnection (LPDISPATCH Application, ext_ConnectMode ConnectMode,
   log_debug ("%s:%s: this is GpgOL %s\n",
              SRCNAME, __func__, PACKAGE_VERSION);
 
+  m_shutdown = false;
   can_unload = false;
   m_application = Application;
   m_application->AddRef();
@@ -365,18 +358,7 @@ GpgolAddin::OnDisconnection (ext_DisconnectMode RemoveMode,
      does not allow us any OOM calls then and only returns
      "Unexpected error" in that case. Weird. */
 
-  if (Mail::closeAllMails_o ())
-    {
-      MessageBox (NULL,
-                  "Failed to remove plaintext from at least one message.\n\n"
-                  "Until GpgOL is activated again it is possible that the "
-                  "plaintext of messages decrypted in this Session is saved "
-                  "or transfered back to your mailserver.",
-                  _("GpgOL"),
-                  MB_ICONINFORMATION|MB_OK);
-    }
-
-  write_options();
+  shutdown ();
   can_unload = true;
   return S_OK;
 }
@@ -469,9 +451,8 @@ install_explorer_sinks (LPDISPATCH application)
         {
           log_oom_extra ("%s:%s: created sink %p for explorer %i",
                          SRCNAME, __func__, sink, i);
+          GpgolAddin::get_instance ()->registerExplorerSink (sink);
         }
-      add_explorer (explorer);
-      gpgol_release (explorer);
     }
   /* Now install the event sink to handle new explorers */
   return install_ExplorersEvents_sink (explorers);
@@ -1111,3 +1092,70 @@ GpgolAddin::get_instance ()
     }
   return addin_instance;
 }
+
+void
+GpgolAddin::shutdown ()
+{
+  if (m_shutdown)
+    {
+      log_debug ("%s:%s: Already shutdown",
+                 SRCNAME, __func__);
+      return;
+    }
+
+  /* Disabling message hook */
+  UnhookWindowsHookEx (m_hook);
+
+  log_debug ("%s:%s: Releasing Application Event Sink;",
+             SRCNAME, __func__);
+  detach_ApplicationEvents_sink (m_applicationEventSink);
+  gpgol_release (m_applicationEventSink);
+
+  log_debug ("%s:%s: Releasing Explorers Event Sink;",
+             SRCNAME, __func__);
+  detach_ExplorersEvents_sink (m_explorersEventSink);
+  gpgol_release (m_explorersEventSink);
+
+  log_debug ("%s:%s: Releasing Explorer Event Sinks;",
+             SRCNAME, __func__);
+  for (auto sink: m_explorerEventSinks)
+    {
+      detach_ExplorerEvents_sink (sink);
+      gpgol_release (sink);
+    }
+
+  write_options ();
+
+  if (Mail::closeAllMails_o ())
+    {
+      MessageBox (NULL,
+                  "Failed to remove plaintext from at least one message.\n\n"
+                  "Until GpgOL is activated again it is possible that the "
+                  "plaintext of messages decrypted in this Session is saved "
+                  "or transfered back to your mailserver.",
+                  _("GpgOL"),
+                  MB_ICONINFORMATION|MB_OK);
+    }
+  m_shutdown = true;
+}
+
+void
+GpgolAddin::registerExplorerSink (LPDISPATCH sink)
+{
+  m_explorerEventSinks.push_back (sink);
+}
+
+void
+GpgolAddin::unregisterExplorerSink (LPDISPATCH sink)
+{
+  for (int i = 0; i < m_explorerEventSinks.size(); ++i)
+    {
+      if (m_explorerEventSinks[i] == sink)
+        {
+          m_explorerEventSinks.erase(m_explorerEventSinks.begin() + i);
+          return;
+        }
+    }
+  log_error ("%s:%s: Unregister %p which was not registered.",
+             SRCNAME, __func__, sink);
+}
diff --git a/src/gpgoladdin.h b/src/gpgoladdin.h
index ae7ab22..05c23c0 100644
--- a/src/gpgoladdin.h
+++ b/src/gpgoladdin.h
@@ -26,6 +26,8 @@
 
 #include "mymapi.h"
 
+#include <vector>
+
 class GpgolAddinRibbonExt;
 class ApplicationEventListener;
 
@@ -203,6 +205,12 @@ public:
 
 public:
   static GpgolAddin * get_instance ();
+
+  void registerExplorerSink (LPDISPATCH sink);
+  void unregisterExplorerSink (LPDISPATCH sink);
+  /* Start the shutdown. Unregisters everything and closes all
+     crypto mails. */
+  void shutdown ();
   LPDISPATCH get_application () { return m_application; }
 
 private:
@@ -215,7 +223,9 @@ private:
   LPDISPATCH m_explorersEventSink;
   LPDISPATCH m_ribbon_control;
   bool m_disabled;
+  bool m_shutdown;
   HHOOK m_hook;
+  std::vector<LPDISPATCH> m_explorerEventSinks;
 };
 
 class GpgolAddinFactory: public IClassFactory
diff --git a/src/mail.cpp b/src/mail.cpp
index 3a25323..1843543 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -1614,6 +1614,16 @@ int
 Mail::closeAllMails_o ()
 {
   int err = 0;
+
+  /* Detach Folder sinks */
+  for (auto fit = s_folder_events_map.begin(); fit != s_folder_events_map.end(); ++fit)
+    {
+      detach_FolderEvents_sink (fit->second);
+      gpgol_release (fit->second);
+    }
+  s_folder_events_map.clear();
+
+
   std::map<LPDISPATCH, Mail *>::iterator it;
   TRACEPOINT;
   gpgrt_lock_lock (&mail_map_lock);
@@ -1645,11 +1655,6 @@ Mail::closeAllMails_o ()
             }
         }
     }
-  for (auto fit = s_folder_events_map.begin(); fit != s_folder_events_map.end(); ++fit)
-    {
-      detach_FolderEvents_sink (fit->second);
-    }
-  s_folder_events_map.clear();
   return err;
 }
 int
@@ -3407,8 +3412,8 @@ Mail::installFolderEventHandler_o()
     {
       log_error ("%s:%s: Install folder events watcher for %s.",
                  SRCNAME, __func__, strPath.c_str());
-      install_FolderEvents_sink (folder);
-      s_folder_events_map.insert (std::make_pair (strPath, folder));
+      const auto sink = install_FolderEvents_sink (folder);
+      s_folder_events_map.insert (std::make_pair (strPath, sink));
     }
 
   /* Folder already registered */
diff --git a/src/windowmessages.cpp b/src/windowmessages.cpp
index 0364717..76f29b8 100644
--- a/src/windowmessages.cpp
+++ b/src/windowmessages.cpp
@@ -313,22 +313,6 @@ do_in_ui_thread_async (gpgol_wmsg_type type, void *data, int delay)
   CloseHandle (CreateThread (NULL, 0, do_async, (LPVOID) ctx, 0, NULL));
 }
 
-static std::vector <LPDISPATCH> explorers;
-
-void
-add_explorer (LPDISPATCH explorer)
-{
-  explorers.push_back (explorer);
-}
-
-void remove_explorer (LPDISPATCH explorer)
-{
-  explorers.erase(std::remove(explorers.begin(),
-                              explorers.end(),
-                              explorer),
-                  explorers.end());
-}
-
 LRESULT CALLBACK
 gpgol_hook(int code, WPARAM wParam, LPARAM lParam)
 {
@@ -343,12 +327,41 @@ gpgol_hook(int code, WPARAM wParam, LPARAM lParam)
       case WM_CLOSE:
       {
         HWND lastChild = NULL;
-        for (const auto explorer: explorers)
+        log_debug ("%s:%s: Got WM_CLOSE",
+                   SRCNAME, __func__);
+        if (!GpgolAddin::get_instance() || !GpgolAddin::get_instance ()->get_application())
           {
+            TRACEPOINT;
+            break;
+          }
+        LPDISPATCH explorers = get_oom_object (GpgolAddin::get_instance ()->get_application(),
+                                               "Explorers");
+
+        if (!explorers)
+          {
+            log_error ("%s:%s: No explorers object",
+                       SRCNAME, __func__);
+            break;
+          }
+        int count = get_oom_int (explorers, "Count");
+
+        for (int i = 1; i <= count; i++)
+          {
+            std::string item = "Item(";
+            item += std::to_string (i) + ")";
+            LPDISPATCH explorer = get_oom_object (explorers, item.c_str());
+
+            if (!explorer)
+              {
+                TRACEPOINT;
+                break;
+              }
+
             /* Casting to LPOLEWINDOW and calling GetWindow
                succeeded in Outlook 2016 but always returned
                the number 1. So we need this hack. */
             char *caption = get_oom_string (explorer, "Caption");
+            gpgol_release (explorer);
             if (!caption)
               {
                 log_debug ("%s:%s: No caption.",
@@ -363,9 +376,9 @@ gpgol_hook(int code, WPARAM wParam, LPARAM lParam)
             if (hwnd == cwp->hwnd)
               {
                 log_debug ("%s:%s: WM_CLOSE windowmessage for explorer. "
-                           "Closing all mails.",
+                           "Shutting down.",
                            SRCNAME, __func__);
-                Mail::closeAllMails_o ();
+                GpgolAddin::get_instance ()->shutdown();
                 break;
               }
           }
@@ -379,7 +392,7 @@ gpgol_hook(int code, WPARAM wParam, LPARAM lParam)
         {
           log_debug ("%s:%s: SC_CLOSE syscommand. Closing all mails.",
                      SRCNAME, __func__);
-          Mail::closeAllMails_o ();
+          GpgolAddin::get_instance ()->shutdown();
         } */
        break;
      default:
diff --git a/src/windowmessages.h b/src/windowmessages.h
index 6a96a13..2d6da49 100644
--- a/src/windowmessages.h
+++ b/src/windowmessages.h
@@ -98,9 +98,6 @@ delayed_invalidate_ui (LPVOID);
 DWORD WINAPI
 close_mail (LPVOID);
 
-void add_explorer (LPDISPATCH explorer);
-void remove_explorer (LPDISPATCH explorer);
-
 /* The lock to invalide the ui */
 extern gpgrt_lock_t invalidate_lock;
 

commit f847b6e6d244558a1b04ae38a1b267e03a86ce3a
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Wed Jul 11 16:48:21 2018 +0200

    Tune down error to debug
    
    * src/mail.cpp (get_uid_for_sender): It's common for S/MIME
    to have two uids where one does not have an email. So don't
    error but just debug it.

diff --git a/src/mail.cpp b/src/mail.cpp
index fb28256..3a25323 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -1927,7 +1927,8 @@ get_uid_for_sender (const Key &k, const char *sender)
     {
       if (!uid.email() || !*(uid.email()))
         {
-          log_error ("%s:%s: skipping uid without email.",
+          /* This happens for S/MIME a lot */
+          log_debug ("%s:%s: skipping uid without email.",
                      SRCNAME, __func__);
           continue;
         }

commit cf28a7508d65deaeb953074b7ddbd5b064d39177
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Wed Jul 11 16:47:32 2018 +0200

    Check for wchar_t conversion success for attach
    
    * src/mail.cpp (add_attachments_o): Check wchar_name.

diff --git a/src/mail.cpp b/src/mail.cpp
index 6c74633..fb28256 100644
--- a/src/mail.cpp
+++ b/src/mail.cpp
@@ -651,39 +651,47 @@ add_attachments_o(LPDISPATCH mail,
   for (auto att: attachments)
     {
       int err = 0;
-      if (att->get_display_name().empty())
+      const auto dispName = att->get_display_name ();
+      if (dispName.empty())
         {
           log_error ("%s:%s: Ignoring attachment without display name.",
                      SRCNAME, __func__);
           continue;
         }
-      wchar_t* wchar_name = utf8_to_wchar (att->get_display_name().c_str());
+      wchar_t* wchar_name = utf8_to_wchar (dispName.c_str());
+      if (!wchar_name)
+        {
+          log_error ("%s:%s: Failed to convert '%s' to wchar.",
+                     SRCNAME, __func__, dispName.c_str());
+          continue;
+        }
+
       HANDLE hFile;
       wchar_t* wchar_file = get_tmp_outfile (wchar_name,
                                              &hFile);
       if (!wchar_file)
         {
           log_error ("%s:%s: Failed to obtain a tmp filename for: %s",
-                     SRCNAME, __func__, att->get_display_name().c_str());
+                     SRCNAME, __func__, dispName.c_str());
           err = 1;
         }
       if (!err && copy_attachment_to_file (att, hFile))
         {
           log_error ("%s:%s: Failed to copy attachment %s to temp file",
-                     SRCNAME, __func__, att->get_display_name().c_str());
+                     SRCNAME, __func__, dispName.c_str());
           err = 1;
         }
       if (!err && add_oom_attachment (mail, wchar_file, wchar_name))
         {
           log_error ("%s:%s: Failed to add attachment: %s",
-                     SRCNAME, __func__, att->get_display_name().c_str());
+                     SRCNAME, __func__, dispName.c_str());
           err = 1;
         }
       CloseHandle (hFile);
       if (!DeleteFileW (wchar_file))
         {
           log_error ("%s:%s: Failed to delete tmp attachment for: %s",
-                     SRCNAME, __func__, att->get_display_name().c_str());
+                     SRCNAME, __func__, dispName.c_str());
           err = 1;
         }
       xfree (wchar_file);

commit 3a8b92c4ee02cc08875e009527f1d8169ff65663
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Wed Jul 11 16:46:37 2018 +0200

    Log sent windowmessages
    
    * src/windowmessages.cpp (do_in_ui_thread): Log message sending.

diff --git a/src/windowmessages.cpp b/src/windowmessages.cpp
index c371886..0364717 100644
--- a/src/windowmessages.cpp
+++ b/src/windowmessages.cpp
@@ -275,6 +275,10 @@ do_in_ui_thread (gpgol_wmsg_type type, void *data)
   wm_ctx_t ctx = {NULL, UNKNOWN, 0, 0};
   ctx.wmsg_type = type;
   ctx.data = data;
+
+  log_debug ("%s:%s: Sending message of type %i",
+             SRCNAME, __func__, type);
+
   if (send_msg_to_ui_thread (&ctx))
     {
       return -1;

commit 22996594617e69285ea4ddef5102fd8ecf7d017b
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Wed Jul 11 16:46:14 2018 +0200

    Minor improvement to debug output
    
    --

diff --git a/src/common.cpp b/src/common.cpp
index feb68ff..b118dff 100644
--- a/src/common.cpp
+++ b/src/common.cpp
@@ -425,7 +425,7 @@ get_tmp_outfile (wchar_t *name, HANDLE *outHandle)
                                     FILE_ATTRIBUTE_TEMPORARY,
                                     NULL)) == INVALID_HANDLE_VALUE)
     {
-      log_debug_w32 (-1, "%s:%s: Failed to open candidate %S.",
+      log_debug_w32 (-1, "%s:%s: Failed to open candidate '%S'",
                      SRCNAME, __func__, outName);
       wchar_t fnameBuf[MAX_PATH + 1];
       wchar_t origName[MAX_PATH + 1];

commit 6d9156a806c096e197f5efbdaefa84586db48580
Author: Andre Heinecke <aheinecke at intevation.de>
Date:   Wed Jul 11 16:45:22 2018 +0200

    Check if someone tries to set null as att name
    
    * src/attachment.cpp (Attachment::set_display_name): Check for NULL.
    
    --
    Should not happen but better save then sorry

diff --git a/src/attachment.cpp b/src/attachment.cpp
index 783272c..68f8025 100644
--- a/src/attachment.cpp
+++ b/src/attachment.cpp
@@ -43,6 +43,12 @@ Attachment::get_display_name() const
 void
 Attachment::set_display_name(const char *name)
 {
+  if (!name)
+    {
+      log_error ("%s:%s: Display name set to null.",
+                 SRCNAME, __func__);
+      return;
+    }
   m_utf8DisplayName = std::string(name);
 }
 

-----------------------------------------------------------------------

Summary of changes:
 src/attachment.cpp       |  6 ++++
 src/common.cpp           |  2 +-
 src/explorer-events.cpp  |  2 +-
 src/explorers-events.cpp |  5 +--
 src/gpgoladdin.cpp       | 94 ++++++++++++++++++++++++++++++++++++------------
 src/gpgoladdin.h         | 10 ++++++
 src/mail.cpp             | 49 ++++++++++++++++---------
 src/windowmessages.cpp   | 57 ++++++++++++++++++-----------
 src/windowmessages.h     |  3 --
 9 files changed, 162 insertions(+), 66 deletions(-)


hooks/post-receive
-- 
GnuPG extension for MS Outlook
http://git.gnupg.org




More information about the Gnupg-commits mailing list