From 7bac92a2bba6776586f7605f82c55fa17c679f0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 31 Oct 2022 15:55:59 +0100 Subject: [PATCH 1/4] gutils: Split g_find_program_path() to make it more flexible and testable Split g_find_program_path() in g_find_program_for_path() that supports passing path arguments and providing a custom working directory. Adding tests to cover the cases we were not doing before. --- glib/gutils.c | 181 ++++++++++++++++++++++++++++++------------- glib/gutilsprivate.h | 5 ++ glib/tests/utils.c | 142 +++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+), 52 deletions(-) diff --git a/glib/gutils.c b/glib/gutils.c index 78ccd6121..186ef7d07 100644 --- a/glib/gutils.c +++ b/glib/gutils.c @@ -275,7 +275,44 @@ gchar* g_find_program_in_path (const gchar *program) #endif { - const gchar *path, *p; + return g_find_program_for_path (program, NULL, NULL); +} + +/** + * g_find_program_for_path: + * @program: (type filename): a program name in the GLib file name encoding + * @path: (type filename) (nullable): the current dir where to search program + * @working_dir: (type filename) (nullable): the working dir where to search + * program + * + * Locates the first executable named @program in @path, in the + * same way that execvp() would locate it. Returns an allocated string + * with the absolute path name (taking in account the @working_dir), or + * %NULL if the program is not found in @path. If @program is already an + * absolute path, returns a copy of @program if @program exists and is + * executable, and %NULL otherwise. + * + * On Windows, if @path is %NULL, it looks for the file in the same way as + * CreateProcess() would. This means first in the directory where the + * executing program was loaded from, then in the current directory, then in + * the Windows 32-bit system directory, then in the Windows directory, and + * finally in the directories in the `PATH` environment variable. If + * the program is found, the return value contains the full name + * including the type suffix. + * + * Returns: (type filename) (transfer full) (nullable): a newly-allocated + * string with the absolute path, or %NULL + * Since: 2.76 + **/ +char * +g_find_program_for_path (const char *program, + const char *path, + const char *working_dir) +{ + const char *original_path = path; + const char *original_program = program; + char *program_path = NULL; + const gchar *p; gchar *name, *freeme; #ifdef G_OS_WIN32 const gchar *path_copy; @@ -290,20 +327,28 @@ g_find_program_in_path (const gchar *program) g_return_val_if_fail (program != NULL, NULL); + /* Use the working dir as program path if provided */ + if (working_dir && !g_path_is_absolute (program)) + { + program_path = g_build_filename (working_dir, program, NULL); + program = program_path; + } + /* If it is an absolute path, or a relative path including subdirectories, * don't look in PATH. */ if (g_path_is_absolute (program) - || strchr (program, G_DIR_SEPARATOR) != NULL + || strchr (original_program, G_DIR_SEPARATOR) != NULL #ifdef G_OS_WIN32 - || strchr (program, '/') != NULL + || strchr (original_program, '/') != NULL #endif ) { if (g_file_test (program, G_FILE_TEST_IS_EXECUTABLE) && !g_file_test (program, G_FILE_TEST_IS_DIR)) { - gchar *out = NULL, *cwd = NULL; + gchar *out = NULL; + char *cwd; if (g_path_is_absolute (program)) return g_strdup (program); @@ -311,13 +356,26 @@ g_find_program_in_path (const gchar *program) cwd = g_get_current_dir (); out = g_build_filename (cwd, program, NULL); g_free (cwd); + g_free (program_path); + return g_steal_pointer (&out); } else - return NULL; + { + g_clear_pointer (&program_path, g_free); + + if (g_path_is_absolute (original_program)) + return NULL; + } } - - path = g_getenv ("PATH"); + + program = original_program; + + if G_LIKELY (original_path == NULL) + path = g_getenv ("PATH"); + else + path = original_path; + #if defined(G_OS_UNIX) if (path == NULL) { @@ -334,57 +392,65 @@ g_find_program_in_path (const gchar *program) path = "/bin:/usr/bin:."; } #else - n = GetModuleFileNameW (NULL, wfilename, MAXPATHLEN); - if (n > 0 && n < MAXPATHLEN) - filename = g_utf16_to_utf8 (wfilename, -1, NULL, NULL, NULL); - - n = GetSystemDirectoryW (wsysdir, MAXPATHLEN); - if (n > 0 && n < MAXPATHLEN) - sysdir = g_utf16_to_utf8 (wsysdir, -1, NULL, NULL, NULL); - - n = GetWindowsDirectoryW (wwindir, MAXPATHLEN); - if (n > 0 && n < MAXPATHLEN) - windir = g_utf16_to_utf8 (wwindir, -1, NULL, NULL, NULL); - - if (filename) + if G_LIKELY (original_path == NULL) { - appdir = g_path_get_dirname (filename); - g_free (filename); - } - - path = g_strdup (path); + n = GetModuleFileNameW (NULL, wfilename, MAXPATHLEN); + if (n > 0 && n < MAXPATHLEN) + filename = g_utf16_to_utf8 (wfilename, -1, NULL, NULL, NULL); - if (windir) - { - const gchar *tem = path; - path = g_strconcat (windir, ";", path, NULL); - g_free ((gchar *) tem); - g_free (windir); + n = GetSystemDirectoryW (wsysdir, MAXPATHLEN); + if (n > 0 && n < MAXPATHLEN) + sysdir = g_utf16_to_utf8 (wsysdir, -1, NULL, NULL, NULL); + + n = GetWindowsDirectoryW (wwindir, MAXPATHLEN); + if (n > 0 && n < MAXPATHLEN) + windir = g_utf16_to_utf8 (wwindir, -1, NULL, NULL, NULL); + + if (filename) + { + appdir = g_path_get_dirname (filename); + g_free (filename); + } + + path = g_strdup (path); + + if (windir) + { + const gchar *tem = path; + path = g_strconcat (windir, ";", path, NULL); + g_free ((gchar *) tem); + g_free (windir); + } + + if (sysdir) + { + const gchar *tem = path; + path = g_strconcat (sysdir, ";", path, NULL); + g_free ((gchar *) tem); + g_free (sysdir); + } + + { + const gchar *tem = path; + path = g_strconcat (".;", path, NULL); + g_free ((gchar *) tem); + } + + if (appdir) + { + const gchar *tem = path; + path = g_strconcat (appdir, ";", path, NULL); + g_free ((gchar *) tem); + g_free (appdir); + } + + path_copy = path; } - - if (sysdir) + else { - const gchar *tem = path; - path = g_strconcat (sysdir, ";", path, NULL); - g_free ((gchar *) tem); - g_free (sysdir); - } - - { - const gchar *tem = path; - path = g_strconcat (".;", path, NULL); - g_free ((gchar *) tem); - } - - if (appdir) - { - const gchar *tem = path; - path = g_strconcat (appdir, ";", path, NULL); - g_free ((gchar *) tem); - g_free (appdir); + path_copy = g_strdup (path); } - path_copy = path; #endif len = strlen (program) + 1; @@ -401,6 +467,7 @@ g_find_program_in_path (const gchar *program) do { char *startp; + char *startp_path = NULL; path = p; p = my_strchrnul (path, G_SEARCHPATH_SEPARATOR); @@ -413,6 +480,13 @@ g_find_program_in_path (const gchar *program) else startp = memcpy (name - (p - path), path, p - path); + /* Use the working dir as program path if provided */ + if (working_dir && !g_path_is_absolute (startp)) + { + startp_path = g_build_filename (working_dir, startp, NULL); + startp = startp_path; + } + if (g_file_test (startp, G_FILE_TEST_IS_EXECUTABLE) && !g_file_test (startp, G_FILE_TEST_IS_DIR)) { @@ -425,12 +499,15 @@ g_find_program_in_path (const gchar *program) ret = g_build_filename (cwd, startp, NULL); g_free (cwd); } + g_free (startp_path); g_free (freeme); #ifdef G_OS_WIN32 g_free ((gchar *) path_copy); #endif return ret; } + + g_free (startp_path); } while (*p++ != '\0'); diff --git a/glib/gutilsprivate.h b/glib/gutilsprivate.h index 77bed4e87..f7d435d61 100644 --- a/glib/gutilsprivate.h +++ b/glib/gutilsprivate.h @@ -32,6 +32,11 @@ GLIB_AVAILABLE_IN_2_60 void g_set_user_dirs (const gchar *first_dir_type, ...) G_GNUC_NULL_TERMINATED; +GLIB_AVAILABLE_IN_2_76 +char * g_find_program_for_path (const char *program, + const char *path, + const char *working_dir); + /* Returns the smallest power of 2 greater than or equal to n, * or 0 if such power does not fit in a gsize */ diff --git a/glib/tests/utils.c b/glib/tests/utils.c index 5d73bbc9e..d02701c8a 100644 --- a/glib/tests/utils.c +++ b/glib/tests/utils.c @@ -29,6 +29,8 @@ #include "glib.h" #include "glib-private.h" +#include "gutilsprivate.h" +#include "glib/gstdio.h" #include #include @@ -485,6 +487,145 @@ test_find_program (void) g_assert (res == NULL); } +static void +test_find_program_for_path (void) +{ + GError *error = NULL; + /* Using .cmd extension to make windows to consider it an executable */ + const char *command_to_find = "just-an-exe-file.cmd"; + char *path; + char *exe_path; + char *found_path; + char *old_cwd; + char *tmp; + + tmp = g_dir_make_tmp ("find_program_for_path_XXXXXXX", &error); + g_assert_no_error (error); + + path = g_build_filename (tmp, "sub-path", NULL); + g_mkdir (path, 0700); + g_assert_true (g_file_test (path, G_FILE_TEST_IS_DIR)); + + exe_path = g_build_filename (path, command_to_find, NULL); + g_file_set_contents (exe_path, "", -1, &error); + g_assert_no_error (error); + g_assert_true (g_file_test (exe_path, G_FILE_TEST_EXISTS)); + +#ifdef G_OS_UNIX + g_assert_no_errno (g_chmod (exe_path, 0500)); +#endif + g_assert_true (g_file_test (exe_path, G_FILE_TEST_IS_EXECUTABLE)); + + g_assert_null (g_find_program_in_path (command_to_find)); + g_assert_null (g_find_program_for_path (command_to_find, NULL, NULL)); + + found_path = g_find_program_for_path (command_to_find, path, NULL); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + found_path = g_find_program_for_path (command_to_find, path, path); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + found_path = g_find_program_for_path (command_to_find, NULL, path); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + found_path = g_find_program_for_path (command_to_find, "::", path); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + old_cwd = g_get_current_dir (); + g_chdir (path); + found_path = + g_find_program_for_path (command_to_find, + G_SEARCHPATH_SEPARATOR_S G_SEARCHPATH_SEPARATOR_S, NULL); + g_chdir (old_cwd); + g_clear_pointer (&old_cwd, g_free); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + old_cwd = g_get_current_dir (); + g_chdir (tmp); + found_path = + g_find_program_for_path (command_to_find, + G_SEARCHPATH_SEPARATOR_S G_SEARCHPATH_SEPARATOR_S, "sub-path"); + g_chdir (old_cwd); + g_clear_pointer (&old_cwd, g_free); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + g_assert_null ( + g_find_program_for_path (command_to_find, + G_SEARCHPATH_SEPARATOR_S G_SEARCHPATH_SEPARATOR_S, "other-sub-path")); + + found_path = g_find_program_for_path (command_to_find, + G_SEARCHPATH_SEPARATOR_S "sub-path" G_SEARCHPATH_SEPARATOR_S, tmp); +#ifdef __APPLE__ + g_assert_nonnull (found_path); + g_assert_true (g_str_has_suffix (found_path, exe_path)); +#else + g_assert_cmpstr (exe_path, ==, found_path); +#endif + g_clear_pointer (&found_path, g_free); + + g_assert_null (g_find_program_for_path (command_to_find, + G_SEARCHPATH_SEPARATOR_S "other-sub-path" G_SEARCHPATH_SEPARATOR_S, tmp)); + +#ifdef G_OS_UNIX + found_path = g_find_program_for_path ("sh", NULL, tmp); + g_assert_nonnull (found_path); + g_clear_pointer (&found_path, g_free); + + old_cwd = g_get_current_dir (); + g_chdir ("/"); + found_path = g_find_program_for_path ("sh", "sbin:bin:usr/bin:usr/sbin", NULL); + g_chdir (old_cwd); + g_clear_pointer (&old_cwd, g_free); + g_assert_nonnull (found_path); + g_clear_pointer (&found_path, g_free); + + found_path = g_find_program_for_path ("sh", "sbin:bin:usr/bin:usr/sbin", "/"); + g_assert_nonnull (found_path); + g_clear_pointer (&found_path, g_free); +#endif /* G_OS_UNIX */ + + g_clear_pointer (&exe_path, g_free); + g_clear_pointer (&path, g_free); + g_clear_pointer (&tmp, g_free); + g_clear_error (&error); +} + static void test_debug (void) { @@ -1149,6 +1290,7 @@ main (int argc, g_test_add_func ("/utils/bits", test_bits); g_test_add_func ("/utils/swap", test_swap); g_test_add_func ("/utils/find-program", test_find_program); + g_test_add_func ("/utils/find-program-for-path", test_find_program_for_path); g_test_add_func ("/utils/debug", test_debug); g_test_add_func ("/utils/codeset", test_codeset); g_test_add_func ("/utils/codeset2", test_codeset2); From e41e3dc60115e81e0e78a6150dabbef1638e72ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Mon, 31 Oct 2022 16:56:23 +0100 Subject: [PATCH 2/4] gdesktopappinfo: Take in account the desktop Path to find executables Desktop files can provide the executable working path and that can be used to pick the file to launch. So take it in account. --- gio/gdesktopappinfo.c | 15 +++++-- gio/tests/appinfo-test-path.desktop.in | 22 ++++++++++ gio/tests/desktop-app-info.c | 60 ++++++++++++++++++++++++++ gio/tests/meson.build | 1 + 4 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 gio/tests/appinfo-test-path.desktop.in diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index 990b2b736..697c7b9d1 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -56,6 +56,7 @@ #include "gappinfo.h" #include "gappinfoprivate.h" #include "glocalfilemonitor.h" +#include "gutilsprivate.h" #ifdef G_OS_UNIX #include "gdocumentportal.h" @@ -1830,6 +1831,7 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, char *type; char *try_exec; char *exec; + char *path; gboolean bus_activatable; start_group = g_key_file_get_start_group (key_file); @@ -1851,6 +1853,10 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, } g_free (type); + path = g_key_file_get_string (key_file, + G_KEY_FILE_DESKTOP_GROUP, + G_KEY_FILE_DESKTOP_KEY_PATH, NULL); + try_exec = g_key_file_get_string (key_file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_TRY_EXEC, @@ -1858,9 +1864,10 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, if (try_exec && try_exec[0] != '\0') { char *t; - t = g_find_program_in_path (try_exec); + t = g_find_program_for_path (try_exec, NULL, path); if (t == NULL) { + g_free (path); g_free (try_exec); return FALSE; } @@ -1877,6 +1884,7 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, char **argv; if (!g_shell_parse_argv (exec, &argc, &argv, NULL)) { + g_free (path); g_free (exec); g_free (try_exec); return FALSE; @@ -1888,11 +1896,12 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, /* Since @exec is not an empty string, there must be at least one * argument, so dereferencing argv[0] should return non-NULL. */ g_assert (argc > 0); - t = g_find_program_in_path (argv[0]); + t = g_find_program_for_path (argv[0], NULL, path); g_strfreev (argv); if (t == NULL) { + g_free (path); g_free (exec); g_free (try_exec); return FALSE; @@ -1912,7 +1921,7 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, info->not_show_in = g_key_file_get_string_list (key_file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_NOT_SHOW_IN, NULL, NULL); info->try_exec = try_exec; info->exec = exec; - info->path = g_key_file_get_string (key_file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_PATH, NULL); + info->path = g_steal_pointer (&path); info->terminal = g_key_file_get_boolean (key_file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_TERMINAL, NULL) != FALSE; info->startup_notify = g_key_file_get_boolean (key_file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_STARTUP_NOTIFY, NULL) != FALSE; info->no_fuse = g_key_file_get_boolean (key_file, G_KEY_FILE_DESKTOP_GROUP, "X-GIO-NoFuse", NULL) != FALSE; diff --git a/gio/tests/appinfo-test-path.desktop.in b/gio/tests/appinfo-test-path.desktop.in new file mode 100644 index 000000000..39ab12fed --- /dev/null +++ b/gio/tests/appinfo-test-path.desktop.in @@ -0,0 +1,22 @@ +[Desktop Entry] +Type=Application +GenericName=generic-appinfo-test-path +Name=appinfo-test-path +Name[de]=appinfo-test-de +X-GNOME-FullName=example +X-GNOME-FullName[de]=Beispiel +Comment=GAppInfo example +Comment[de]=GAppInfo Beispiel +Path=@installed_tests_dir@ +TryExec=apps +Exec=appinfo-test --option %U %i --name %c --filename %k %m %% +Icon=testicon.svg +Terminal=false +StartupNotify=true +StartupWMClass=appinfo-path-class +MimeType=image/png;image/jpeg; +Keywords=keyword1;test keyword; +Categories=GNOME;GTK; +X-JunkFood=Burger +X-JunkFood[de]=Bratwurst +X-JunkFood[it]= diff --git a/gio/tests/desktop-app-info.c b/gio/tests/desktop-app-info.c index 33eb3985b..0839eb0a2 100644 --- a/gio/tests/desktop-app-info.c +++ b/gio/tests/desktop-app-info.c @@ -1578,6 +1578,64 @@ test_launch_uris_with_invalid_terminal (void) g_free (old_path); } +static void +test_app_path (void) +{ + GDesktopAppInfo *appinfo; + const char *desktop_path; + + desktop_path = g_test_get_filename (G_TEST_BUILT, "appinfo-test-path.desktop", NULL); + appinfo = g_desktop_app_info_new_from_filename (desktop_path); + + g_assert_true (G_IS_DESKTOP_APP_INFO (appinfo)); + + g_clear_object (&appinfo); +} + +static void +test_app_path_wrong (void) +{ + GKeyFile *key_file; + GDesktopAppInfo *appinfo; + const gchar bad_try_exec_file_contents[] = + "[Desktop Entry]\n" + "Type=Application\n" + "Name=appinfo-test\n" + "TryExec=appinfo-test\n" + "Path=this-must-not-exist‼\n" + "Exec=true\n"; + const gchar bad_exec_file_contents[] = + "[Desktop Entry]\n" + "Type=Application\n" + "Name=appinfo-test\n" + "TryExec=true\n" + "Path=this-must-not-exist‼\n" + "Exec=appinfo-test\n"; + + g_assert_true ( + g_file_test (g_test_get_filename (G_TEST_BUILT, "appinfo-test", NULL), + G_FILE_TEST_IS_REGULAR | G_FILE_TEST_IS_EXECUTABLE)); + + key_file = g_key_file_new (); + + g_assert_true ( + g_key_file_load_from_data (key_file, bad_try_exec_file_contents, -1, + G_KEY_FILE_NONE, NULL)); + + appinfo = g_desktop_app_info_new_from_keyfile (key_file); + g_assert_false (G_IS_DESKTOP_APP_INFO (appinfo)); + + g_assert_true ( + g_key_file_load_from_data (key_file, bad_exec_file_contents, -1, + G_KEY_FILE_NONE, NULL)); + + appinfo = g_desktop_app_info_new_from_keyfile (key_file); + g_assert_false (G_IS_DESKTOP_APP_INFO (appinfo)); + + g_clear_pointer (&key_file, g_key_file_unref); + g_clear_object (&appinfo); +} + int main (int argc, char *argv[]) @@ -1615,6 +1673,8 @@ main (int argc, g_test_add_func ("/desktop-app-info/search", test_search); g_test_add_func ("/desktop-app-info/implements", test_implements); g_test_add_func ("/desktop-app-info/show-in", test_show_in); + g_test_add_func ("/desktop-app-info/app-path", test_app_path); + g_test_add_func ("/desktop-app-info/app-path/wrong", test_app_path_wrong); g_test_add_func ("/desktop-app-info/launch-as-manager", test_launch_as_manager); g_test_add_func ("/desktop-app-info/launch-as-manager/fail", test_launch_as_manager_fail); g_test_add_func ("/desktop-app-info/launch-default-uri-handler", test_default_uri_handler); diff --git a/gio/tests/meson.build b/gio/tests/meson.build index 11acad460..ea685da0f 100644 --- a/gio/tests/meson.build +++ b/gio/tests/meson.build @@ -628,6 +628,7 @@ endif appinfo_test_desktop_files = [ 'appinfo-test-gnome', 'appinfo-test-notgnome', + 'appinfo-test-path', 'appinfo-test', 'appinfo-test2', ] From da8aa0b66d478761906e4ca63091a5f8a3800960 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 1 Nov 2022 00:31:58 +0100 Subject: [PATCH 3/4] desktop-app-info: Use launch context PATH and desktop Path to find terminals We used to launch applications with terminals using the normal program finder logic that did not consider the context path nor the desktop file working dir. Switch to g_find_program_for_path() to find terminals so we can ensure that both conditions are true. Update tests to consider this case too. --- gio/gdesktopappinfo.c | 15 +++-- gio/tests/desktop-app-info.c | 117 +++++++++++++++++++++++++++++------ 2 files changed, 109 insertions(+), 23 deletions(-) diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index 697c7b9d1..aca7e9c37 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -1864,6 +1864,7 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, if (try_exec && try_exec[0] != '\0') { char *t; + /* Use the desktop file path (if any) as working dir to search program */ t = g_find_program_for_path (try_exec, NULL, path); if (t == NULL) { @@ -1896,6 +1897,7 @@ g_desktop_app_info_load_from_keyfile (GDesktopAppInfo *info, /* Since @exec is not an empty string, there must be at least one * argument, so dereferencing argv[0] should return non-NULL. */ g_assert (argc > 0); + /* Use the desktop file path (if any) as working dir to search program */ t = g_find_program_for_path (argv[0], NULL, path); g_strfreev (argv); @@ -2631,8 +2633,10 @@ expand_application_parameters (GDesktopAppInfo *info, } static gboolean -prepend_terminal_to_vector (int *argc, - char ***argv) +prepend_terminal_to_vector (int *argc, + char ***argv, + const char *path, + const char *working_dir) { #ifndef G_OS_WIN32 char **real_argv; @@ -2678,7 +2682,8 @@ prepend_terminal_to_vector (int *argc, for (i = 0, found_terminal = NULL; i < G_N_ELEMENTS (known_terminals); i++) { - found_terminal = g_find_program_in_path (known_terminals[i].exec); + found_terminal = g_find_program_for_path (known_terminals[i].exec, + path, working_dir); if (found_terminal != NULL) { term_arg = known_terminals[i].exec_arg; @@ -2880,7 +2885,9 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info, launched_uris = g_list_prepend (launched_uris, iter->data); launched_uris = g_list_reverse (launched_uris); - if (info->terminal && !prepend_terminal_to_vector (&argc, &argv)) + if (info->terminal && !prepend_terminal_to_vector (&argc, &argv, + g_environ_getenv (envp, "PATH"), + info->path)) { g_set_error_literal (error, G_IO_ERROR, G_IO_ERROR_FAILED, _("Unable to find terminal required for application")); diff --git a/gio/tests/desktop-app-info.c b/gio/tests/desktop-app-info.c index 0839eb0a2..c5e4f2f85 100644 --- a/gio/tests/desktop-app-info.c +++ b/gio/tests/desktop-app-info.c @@ -1369,6 +1369,17 @@ get_terminal_divider (const char *terminal_name) g_return_val_if_reached (NULL); } +typedef enum { + TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_PATH_OVERRIDE, + TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_CONTEXT, + TERMINAL_LAUNCH_TYPE_KEY_FILE_WITH_PATH, +} TerminalLaunchType; + +typedef struct { + const char *exec; + TerminalLaunchType type; +} TerminalLaunchData; + static void test_launch_uris_with_terminal (gconstpointer data) { @@ -1376,8 +1387,9 @@ test_launch_uris_with_terminal (gconstpointer data) int ret; int flags; int terminal_divider_arg_length; - const char *terminal_exec = data; - char *old_path; + const TerminalLaunchData *launch_data = data; + const char *terminal_exec = launch_data->exec; + char *old_path = NULL; char *command_line; char *bin_path; char *terminal_path; @@ -1392,6 +1404,7 @@ test_launch_uris_with_terminal (gconstpointer data) GError *error = NULL; GInputStream *input_stream; GDataInputStream *data_input_stream; + GAppLaunchContext *launch_context; sh = g_find_program_in_path ("sh"); g_assert_nonnull (sh); @@ -1399,26 +1412,40 @@ test_launch_uris_with_terminal (gconstpointer data) bin_path = g_dir_make_tmp ("bin-path-XXXXXX", &error); g_assert_no_error (error); - old_path = g_strdup (g_getenv ("PATH")); - g_assert_true (g_setenv ("PATH", bin_path, TRUE)); + launch_context = g_object_new (test_launch_context_get_type (), NULL); + + switch (launch_data->type) + { + case TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_PATH_OVERRIDE: + old_path = g_strdup (g_getenv ("PATH")); + g_assert_true (g_setenv ("PATH", bin_path, TRUE)); + break; + + case TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_CONTEXT: + g_app_launch_context_setenv (launch_context, "PATH", bin_path); + break; + + case TERMINAL_LAUNCH_TYPE_KEY_FILE_WITH_PATH: + g_app_launch_context_setenv (launch_context, "PATH", "/not/valid"); + break; + + default: + g_assert_not_reached (); + } terminal_path = g_build_filename (bin_path, terminal_exec, NULL); output_fd_path = g_build_filename (bin_path, "fifo", NULL); ret = mkfifo (output_fd_path, 0600); - g_assert_cmpint (ret, ==, 0); fd = g_open (output_fd_path, O_RDONLY | O_CLOEXEC | O_NONBLOCK, 0); - g_assert_cmpint (fd, >=, 0); flags = fcntl (fd, F_GETFL); - g_assert_cmpint (flags, >=, 0); ret = fcntl (fd, F_SETFL, flags & ~O_NONBLOCK); - g_assert_cmpint (ret, ==, 0); input_stream = g_unix_input_stream_new (fd, TRUE); @@ -1435,12 +1462,43 @@ test_launch_uris_with_terminal (gconstpointer data) g_test_message ("Fake '%s' terminal created as: %s", terminal_exec, terminal_path); command_line = g_strdup_printf ("true %s-argument", terminal_exec); - app_info = g_app_info_create_from_commandline (command_line, - "Test App on Terminal", - G_APP_INFO_CREATE_NEEDS_TERMINAL | - G_APP_INFO_CREATE_SUPPORTS_URIS, - &error); - g_assert_no_error (error); + + if (launch_data->type == TERMINAL_LAUNCH_TYPE_KEY_FILE_WITH_PATH) + { + GKeyFile *key_file; + char *key_file_contents; + const char base_file[] = + "[Desktop Entry]\n" + "Type=Application\n" + "Name=terminal launched app\n" + "Terminal=true\n" + "Path=%s\n" + "Exec=%s\n"; + + key_file = g_key_file_new (); + key_file_contents = g_strdup_printf (base_file, bin_path, command_line); + + g_assert_true ( + g_key_file_load_from_data (key_file, key_file_contents, -1, + G_KEY_FILE_NONE, NULL)); + + app_info = (GAppInfo*) g_desktop_app_info_new_from_keyfile (key_file); + g_assert_true (G_IS_DESKTOP_APP_INFO (app_info)); + g_assert_true ( + g_desktop_app_info_get_boolean (G_DESKTOP_APP_INFO (app_info), "Terminal")); + + g_key_file_unref (key_file); + g_free (key_file_contents); + } + else + { + app_info = g_app_info_create_from_commandline (command_line, + "Test App on Terminal", + G_APP_INFO_CREATE_NEEDS_TERMINAL | + G_APP_INFO_CREATE_SUPPORTS_URIS, + &error); + g_assert_no_error (error); + } paths = g_list_prepend (NULL, bin_path); uris = g_list_prepend (NULL, g_filename_to_uri (bin_path, NULL, &error)); @@ -1451,7 +1509,7 @@ test_launch_uris_with_terminal (gconstpointer data) g_assert_no_error (error); g_assert_cmpint (g_list_length (paths), ==, 2); - g_app_info_launch_uris (app_info, uris, NULL, &error); + g_app_info_launch_uris (app_info, uris, launch_context, &error); g_assert_no_error (error); while (output_contents == NULL) @@ -1523,7 +1581,9 @@ test_launch_uris_with_terminal (gconstpointer data) g_clear_pointer (&output_args, g_strfreev); g_assert_null (paths); - g_assert_true (g_setenv ("PATH", old_path, TRUE)); + + if (launch_data->type == TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_PATH_OVERRIDE) + g_assert_true (g_setenv ("PATH", old_path, TRUE)); g_close (fd, &error); g_assert_no_error (error); @@ -1540,6 +1600,7 @@ test_launch_uris_with_terminal (gconstpointer data) g_clear_object (&data_input_stream); g_clear_object (&input_stream); g_clear_object (&app_info); + g_clear_object (&launch_context); g_clear_error (&error); g_clear_list (&paths, NULL); g_clear_list (&uris, g_free); @@ -1685,12 +1746,30 @@ main (int argc, { char *path; - path = g_strdup_printf ("/desktop-app-info/launch-uris-with-terminal/%s", + path = g_strdup_printf ("/desktop-app-info/launch-uris-with-terminal/with-path/%s", supported_terminals[i]); - g_test_add_data_func (path, supported_terminals[i], - test_launch_uris_with_terminal); + g_test_add_data_func (path, &(TerminalLaunchData) { + .exec = supported_terminals[i], + .type = TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_PATH_OVERRIDE, + }, test_launch_uris_with_terminal); g_free (path); + + path = g_strdup_printf ("/desktop-app-info/launch-uris-with-terminal/with-context/%s", + supported_terminals[i]); + g_test_add_data_func (path, &(TerminalLaunchData) { + .exec = supported_terminals[i], + .type = TERMINAL_LAUNCH_TYPE_COMMAND_LINE_WITH_CONTEXT, + }, test_launch_uris_with_terminal); + g_clear_pointer (&path, g_free); + + path = g_strdup_printf ("/desktop-app-info/launch-uris-with-terminal/with-desktop-path/%s", + supported_terminals[i]); + g_test_add_data_func (path, &(TerminalLaunchData) { + .exec = supported_terminals[i], + .type = TERMINAL_LAUNCH_TYPE_KEY_FILE_WITH_PATH, + }, test_launch_uris_with_terminal); + g_clear_pointer (&path, g_free); } g_test_add_func ("/desktop-app-info/launch-uris-with-terminal/invalid-glib-terminal", From 511d1cad02c6ff427eecef4314121ef22cfb3700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Tue, 1 Nov 2022 00:55:15 +0100 Subject: [PATCH 4/4] gdesktopappinfo: Fail early if trying to launch an invalid executable GDesktopAppInfo never failed in the most simple of the cases: when a desktop file or a command line app info was pointing to an invalid executable (for the context). The reason for this is that we're launching all the programs using gio-launch-desktop which will always exist in a sane GLib installation, and thus our call to execvp won't ever fail on failure. This was partially mitigated by not allowing to create a desktop app icon using a non-existent executable (even if not fully correctly) but still did not work in case a custom PATH was provided in the launch context. To avoid this, use g_find_program_for_path() to find early if a program that we're about to launch is available, and if it's not the case return the same error that g_spawn_async_with_fds() would throw in such cases. While this is slowing a bit our preparation phase, would avoid to leave to the exec function the job to find where our program is. Add tests simulating this behavior. --- gio/gdesktopappinfo.c | 40 +++++++++++++++ gio/tests/desktop-app-info.c | 99 ++++++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+) diff --git a/gio/gdesktopappinfo.c b/gio/gdesktopappinfo.c index aca7e9c37..6441156b5 100644 --- a/gio/gdesktopappinfo.c +++ b/gio/gdesktopappinfo.c @@ -2919,6 +2919,46 @@ g_desktop_app_info_launch_uris_with_spawn (GDesktopAppInfo *info, emit_launch_started (launch_context, info, sn_id); } + g_assert (argc > 0); + + if (!g_path_is_absolute (argv[0]) || + !g_file_test (argv[0], G_FILE_TEST_IS_EXECUTABLE) || + g_file_test (argv[0], G_FILE_TEST_IS_DIR)) + { + char *program = g_steal_pointer (&argv[0]); + char *program_path = NULL; + + if (!g_path_is_absolute (program)) + { + const char *env_path = g_environ_getenv (envp, "PATH"); + + program_path = g_find_program_for_path (program, + env_path, + info->path); + } + + if (program_path) + { + argv[0] = g_steal_pointer (&program_path); + } + else + { + if (sn_id) + g_app_launch_context_launch_failed (launch_context, sn_id); + + g_set_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT, + _("Program ‘%s’ not found in $PATH"), + program); + + g_free (program); + g_clear_pointer (&sn_id, g_free); + g_clear_list (&launched_uris, NULL); + goto out; + } + + g_free (program); + } + if (g_once_init_enter (&gio_launch_desktop_path)) { const gchar *tmp = NULL; diff --git a/gio/tests/desktop-app-info.c b/gio/tests/desktop-app-info.c index c5e4f2f85..6749f6061 100644 --- a/gio/tests/desktop-app-info.c +++ b/gio/tests/desktop-app-info.c @@ -1697,6 +1697,102 @@ test_app_path_wrong (void) g_clear_object (&appinfo); } +static void +test_launch_startup_notify_fail (void) +{ + GAppInfo *app_info; + GAppLaunchContext *context; + GError *error = NULL; + gboolean launch_started; + gboolean launch_failed; + gboolean launched; + GList *uris; + + app_info = g_app_info_create_from_commandline ("this-must-not-exist‼", + "failing app", + G_APP_INFO_CREATE_NONE | + G_APP_INFO_CREATE_SUPPORTS_STARTUP_NOTIFICATION, + &error); + g_assert_no_error (error); + + context = g_object_new (test_launch_context_get_type (), NULL); + g_signal_connect (context, "launch-started", + G_CALLBACK (on_launch_started), + &launch_started); + g_signal_connect (context, "launched", + G_CALLBACK (on_launch_started), + &launched); + g_signal_connect (context, "launch-failed", + G_CALLBACK (on_launch_failed), + &launch_failed); + + launch_started = FALSE; + launch_failed = FALSE; + launched = FALSE; + uris = g_list_prepend (NULL, g_file_new_for_uri ("foo://bar")); + uris = g_list_prepend (uris, g_file_new_for_uri ("bar://foo")); + g_assert_false (g_app_info_launch (app_info, uris, context, &error)); + g_assert_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT); + g_assert_true (launch_started); + g_assert_true (launch_failed); + g_assert_false (launched); + + g_clear_error (&error); + g_clear_object (&app_info); + g_clear_object (&context); + g_clear_list (&uris, g_object_unref); +} + +static void +test_launch_fail (void) +{ + GAppInfo *app_info; + GError *error = NULL; + + app_info = g_app_info_create_from_commandline ("this-must-not-exist‼", + "failing app", + G_APP_INFO_CREATE_NONE, + &error); + g_assert_no_error (error); + + g_assert_false (g_app_info_launch (app_info, NULL, NULL, &error)); + g_assert_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT); + + g_clear_error (&error); + g_clear_object (&app_info); +} + +static void +test_launch_fail_absolute_path (void) +{ + GAppInfo *app_info; + GError *error = NULL; + + app_info = g_app_info_create_from_commandline ("/nothing/of/this-must-exist‼", + NULL, + G_APP_INFO_CREATE_NONE, + &error); + g_assert_no_error (error); + + g_assert_false (g_app_info_launch (app_info, NULL, NULL, &error)); + g_assert_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT); + + g_clear_error (&error); + g_clear_object (&app_info); + + app_info = g_app_info_create_from_commandline ("/", + NULL, + G_APP_INFO_CREATE_NONE, + &error); + g_assert_no_error (error); + + g_assert_false (g_app_info_launch (app_info, NULL, NULL, &error)); + g_assert_error (error, G_SPAWN_ERROR, G_SPAWN_ERROR_NOENT); + + g_clear_error (&error); + g_clear_object (&app_info); +} + int main (int argc, char *argv[]) @@ -1736,6 +1832,9 @@ main (int argc, g_test_add_func ("/desktop-app-info/show-in", test_show_in); g_test_add_func ("/desktop-app-info/app-path", test_app_path); g_test_add_func ("/desktop-app-info/app-path/wrong", test_app_path_wrong); + g_test_add_func ("/desktop-app-info/launch/fail", test_launch_fail); + g_test_add_func ("/desktop-app-info/launch/fail-absolute-path", test_launch_fail_absolute_path); + g_test_add_func ("/desktop-app-info/launch/fail-startup-notify", test_launch_startup_notify_fail); g_test_add_func ("/desktop-app-info/launch-as-manager", test_launch_as_manager); g_test_add_func ("/desktop-app-info/launch-as-manager/fail", test_launch_as_manager_fail); g_test_add_func ("/desktop-app-info/launch-default-uri-handler", test_default_uri_handler);