gpg: Improve the photo image viewer selection.

* g10/exec.c (w32_system): Add "!ShellExecute" special.
* g10/photoid.c (get_default_photo_command): Use the new ShellExecute
under Windows and fallbac to 'display' and 'xdg-open' in the Unix
case.
(show_photos): Flush stdout so that the output is shown before the
image pops up.
--

For Unix this basically syncs the code with what we have in gpg 1.4.
Note that xdg-open may not be used when running as root which we
support here.

For Windows we now use ShellExecute as this seems to be preferred over
"cmd /c start"; however this does not solve the actual problem we had
in the bug report.  To solve that problem we resort to a wait
parameter which defaults to 400ms.  This works on my Windows-10
virtualized test box.  If we can figure out which simple viewers are
commonly installed on Windows we should enhance this patch to test for
them.

GnuPG-bug-id: 4334
Signed-off-by: Werner Koch <[email protected]>

3 files changed, 113 insertions, 36 deletions

diff --git a/doc/gpg.texi b/doc/gpg.texi
index 7858baf82..8aaf8db0f 100644
--- a/doc/gpg.texi
+++ b/doc/gpg.texi
@@ -1426,19 +1426,24 @@ viewed (e.g. "f"), "%V" for the calculated validity as a string (e.g.
 and "%%" for an actual percent sign. If neither %i or %I are present,
 then the photo will be supplied to the viewer on standard input.
 
-The default viewer is "xloadimage -fork -quiet -title 'KeyID 0x%k'
-STDIN". Note that if your image viewer program is not secure, then
-executing it from GnuPG does not make it secure.
+On Unix the default viewer is
+@code{xloadimage -fork -quiet -title 'KeyID 0x%k' STDIN}
+with a fallback to
+@code{display -title 'KeyID 0x%k' %i}
+and finally to
+@code{xdg-open %i}.
+On Windows
+@code{!ShellExecute 400 %i} is used; here the command is a meta
+command to use that API call followed by a wait time in milliseconds
+which is used to give the viewer time to read the temporary image file
+before gpg deletes it again.  Note that if your image viewer program
+is not secure, then executing it from gpg does not make it secure.
 
 @item --exec-path @var{string}
 @opindex exec-path
 @efindex PATH
-Sets a list of directories to search for photo viewers and keyserver
-helpers. If not provided, keyserver helpers use the compiled-in
-default directory, and photo viewers use the @code{PATH} environment
-variable.
-Note, that on W32 system this value is ignored when searching for
-keyserver helpers.
+Sets a list of directories to search for photo viewers If not provided
+photo viewers use the @code{PATH} environment variable.
 
 @item --keyring @var{file}
 @opindex keyring
diff --git a/g10/exec.c b/g10/exec.c
index 74a83970e..3e5dc278b 100644
--- a/g10/exec.c
+++ b/g10/exec.c
@@ -77,37 +77,99 @@ set_exec_path(const char *path) { return GPG_ERR_GENERAL; }
 static int
 w32_system(const char *command)
 {
-#ifdef HAVE_W32CE_SYSTEM
-#warning Change this code to use common/exechelp.c
-#else
-  PROCESS_INFORMATION pi;
-  STARTUPINFO si;
-  char *string;
+  if (!strncmp (command, "!ShellExecute ", 14))
+    {
+      SHELLEXECUTEINFOW see;
+      wchar_t *wname;
+      int waitms;
+
+      command = command + 14;
+      while (spacep (command))
+        command++;
+      waitms = atoi (command);
+      if (waitms < 0)
+        waitms = 0;
+      else if (waitms > 60*1000)
+        waitms = 60000;
+      while (*command && !spacep (command))
+        command++;
+      while (spacep (command))
+        command++;
+
+      wname = utf8_to_wchar (command);
+      if (!wname)
+        return -1;
+
+      memset (&see, 0, sizeof see);
+      see.cbSize = sizeof see;
+      see.fMask = (SEE_MASK_NOCLOSEPROCESS
+                   | SEE_MASK_NOASYNC
+                   | SEE_MASK_FLAG_NO_UI
+                   | SEE_MASK_NO_CONSOLE);
+      see.lpVerb = L"open";
+      see.lpFile = (LPCWSTR)wname;
+      see.nShow = SW_SHOW;
+
+      if (DBG_EXTPROG)
+        log_debug ("running ShellExecuteEx(open,'%s')\n", command);
+      if (!ShellExecuteExW (&see))
+        {
+          if (DBG_EXTPROG)
+            log_debug ("ShellExecuteEx failed: rc=%d\n", (int)GetLastError ());
+          xfree (wname);
+          return -1;
+        }
+      if (DBG_EXTPROG)
+        log_debug ("ShellExecuteEx succeeded (hProcess=%p,hInstApp=%d)\n",
+                   see.hProcess, (int)see.hInstApp);
+
+      if (!see.hProcess)
+        {
+          gnupg_usleep (waitms*1000);
+          if (DBG_EXTPROG)
+            log_debug ("ShellExecuteEx ready (wait=%dms)\n", waitms);
+        }
+      else
+        {
+          WaitForSingleObject (see.hProcess, INFINITE);
+          if (DBG_EXTPROG)
+            log_debug ("ShellExecuteEx ready\n");
+        }
+      CloseHandle (see.hProcess);
+
+      xfree (wname);
+    }
+  else
+    {
+      char *string;
+      PROCESS_INFORMATION pi;
+      STARTUPINFO si;
 
-  /* We must use a copy of the command as CreateProcess modifies this
-     argument. */
-  string=xstrdup(command);
+      /* We must use a copy of the command as CreateProcess modifies
+       * this argument. */
+      string = xstrdup (command);
 
-  memset(&pi,0,sizeof(pi));
-  memset(&si,0,sizeof(si));
-  si.cb=sizeof(si);
+      memset (&pi, 0, sizeof(pi));
+      memset (&si, 0, sizeof(si));
+      si.cb = sizeof (si);
 
-  if(!CreateProcess(NULL,string,NULL,NULL,FALSE,
-                    DETACHED_PROCESS,
-                    NULL,NULL,&si,&pi))
-    return -1;
+      if (!CreateProcess (NULL, string, NULL, NULL, FALSE,
+                          DETACHED_PROCESS,
+                          NULL, NULL, &si, &pi))
+        return -1;
 
-  /* Wait for the child to exit */
-  WaitForSingleObject(pi.hProcess,INFINITE);
+      /* Wait for the child to exit */
+      WaitForSingleObject (pi.hProcess, INFINITE);
 
-  CloseHandle(pi.hProcess);
-  CloseHandle(pi.hThread);
-  xfree(string);
+      CloseHandle (pi.hProcess);
+      CloseHandle (pi.hThread);
+      xfree (string);
+    }
 
   return 0;
-#endif
 }
-#endif
+#endif /*_W32*/
+
 
 /* Replaces current $PATH */
 int
@@ -508,7 +570,7 @@ exec_read(struct exec_info *info)
   if(info->flags.use_temp_files)
     {
       if(DBG_EXTPROG)
-	log_debug("system() command is %s\n",info->command);
+	log_debug ("running command: %s\n",info->command);
 
 #if defined (_WIN32)
       info->progreturn=w32_system(info->command);
diff --git a/g10/photoid.c b/g10/photoid.c
index bcea64fbf..f9720d329 100644
--- a/g10/photoid.c
+++ b/g10/photoid.c
@@ -262,7 +262,8 @@ char *image_type_to_string(byte type,int style)
 }
 
 #if !defined(FIXED_PHOTO_VIEWER) && !defined(DISABLE_PHOTO_VIEWER)
-static const char *get_default_photo_command(void)
+static const char *
+get_default_photo_command(void)
 {
 #if defined(_WIN32)
   OSVERSIONINFO osvi;
@@ -274,14 +275,21 @@ static const char *get_default_photo_command(void)
   if(osvi.dwPlatformId==VER_PLATFORM_WIN32_WINDOWS)
     return "start /w %i";
   else
-    return "cmd /c start /w %i";
+    return "!ShellExecute 400 %i";
 #elif defined(__APPLE__)
   /* OS X.  This really needs more than just __APPLE__. */
   return "open %I";
 #elif defined(__riscos__)
   return "Filer_Run %I";
 #else
-  return "xloadimage -fork -quiet -title 'KeyID 0x%k' stdin";
+  if (!path_access ("xloadimage", X_OK))
+    return "xloadimage -fork -quiet -title 'KeyID 0x%k' stdin";
+  else if (!path_access ("display",X_OK))
+    return "display -title 'KeyID 0x%k' %i";
+  else if (getuid () && !path_access ("xdg-open", X_OK))
+    return "xdg-open %i";
+  else
+    return "/bin/true";
 #endif
 }
 #endif
@@ -312,6 +320,8 @@ show_photos (ctrl_t ctrl, const struct user_attribute *attrs, int count,
   if (pk)
     keyid_from_pk (pk, kid);
 
+  es_fflush (es_stdout);
+
   for(i=0;i<count;i++)
     if(attrs[i].type==ATTRIB_IMAGE &&
        parse_image_header(&attrs[i],&args.imagetype,&len))