From cd5f040a5389944dd8a05bc9c938f888581dfc8a Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 17 May 2019 12:31:11 +0200 Subject: [PATCH] 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 --- doc/gpg.texi | 23 ++++++----- g10/exec.c | 110 +++++++++++++++++++++++++++++++++++++++----------- g10/photoid.c | 16 ++++++-- 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; - /* We must use a copy of the command as CreateProcess modifies this - argument. */ - string=xstrdup(command); + 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++; - memset(&pi,0,sizeof(pi)); - memset(&si,0,sizeof(si)); - si.cb=sizeof(si); + wname = utf8_to_wchar (command); + if (!wname) + return -1; - if(!CreateProcess(NULL,string,NULL,NULL,FALSE, - DETACHED_PROCESS, - NULL,NULL,&si,&pi)) - 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; - /* Wait for the child to exit */ - WaitForSingleObject(pi.hProcess,INFINITE); + 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); - CloseHandle(pi.hProcess); - CloseHandle(pi.hThread); - xfree(string); + 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); + + 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; + + /* Wait for the child to exit */ + WaitForSingleObject (pi.hProcess, INFINITE); + + 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