From 0d4f6e054d0b2cf66cd34aa4feb737ddeae66f85 Mon Sep 17 00:00:00 2001 From: Geert Janssens Date: Fri, 2 Feb 2018 10:24:04 +0100 Subject: [PATCH] Improve gnc_data_home verification and creation - Don't attempt to create a subdirectory of a non-existing home directory (use tmpdir as base directory in that case) - Make sure all tests run in an environment with GNC_BUILDDIR and GNC_UNINSTALLED set. Otherwise the one-shot old .gnucash to new GNC_DATA_HOME migration may already have run at build time, preventing us from informing the user a run time. - Re-enable the userdata-dir with invalid home test (linux only). --- common/cmake_modules/GncAddTest.cmake | 6 +- libgnucash/core-utils/gnc-filepath-utils.cpp | 91 ++++++++++++++----- libgnucash/core-utils/test/CMakeLists.txt | 6 +- .../test/test-userdata-dir-invalid-home.c | 4 +- .../core-utils/test/test-userdata-dir.c | 5 +- 5 files changed, 79 insertions(+), 33 deletions(-) diff --git a/common/cmake_modules/GncAddTest.cmake b/common/cmake_modules/GncAddTest.cmake index 6435f225e2..9c7863cd81 100644 --- a/common/cmake_modules/GncAddTest.cmake +++ b/common/cmake_modules/GncAddTest.cmake @@ -76,15 +76,15 @@ FUNCTION(GNC_ADD_TEST _TARGET _SOURCE_FILES TEST_INCLUDE_VAR_NAME TEST_LIBS_VAR_ IF (${HAVE_ENV_VARS}) SET(CMAKE_COMMAND_TMP "") IF (${CMAKE_VERSION} VERSION_GREATER 3.1) - SET(CMAKE_COMMAND_TMP ${CMAKE_COMMAND} -E env "GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}") + SET(CMAKE_COMMAND_TMP ${CMAKE_COMMAND} -E env "GNC_UNINSTALLED=YES;GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}") ENDIF() ADD_TEST(${_TARGET} ${CMAKE_COMMAND_TMP} ${CMAKE_BINARY_DIR}/bin/${_TARGET} ) - SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}") + SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_UNINSTALLED=YES;GNC_BUILDDIR=${CMAKE_BINARY_DIR};${ARGN}") ELSE() ADD_TEST(NAME ${_TARGET} COMMAND ${_TARGET}) - SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_BUILDDIR=${CMAKE_BINARY_DIR}") + SET_TESTS_PROPERTIES(${_TARGET} PROPERTIES ENVIRONMENT "GNC_UNINSTALLED=YES;GNC_BUILDDIR=${CMAKE_BINARY_DIR}") ENDIF() ADD_DEPENDENCIES(check ${_TARGET}) ENDFUNCTION() diff --git a/libgnucash/core-utils/gnc-filepath-utils.cpp b/libgnucash/core-utils/gnc-filepath-utils.cpp index fc57483018..8b45780b8a 100644 --- a/libgnucash/core-utils/gnc-filepath-utils.cpp +++ b/libgnucash/core-utils/gnc-filepath-utils.cpp @@ -318,6 +318,27 @@ gnc_path_find_localized_html_file (const gchar *file_name) } /* ====================================================================== */ +static auto gnc_userdata_home = bfs::path(); +static auto build_dir = bfs::path(); + +static bool dir_is_descendant (const bfs::path& path, const bfs::path& base) +{ + auto test_path = path; + if (bfs::exists (path)) + test_path = bfs::canonical (path); + auto test_base = base; + if (bfs::exists (base)) + test_base = bfs::canonical (base); + + auto is_descendant = (test_path.string() == test_base.string()); + while (!test_path.empty() && !is_descendant) + { + test_path = test_path.parent_path(); + is_descendant = (test_path.string() == test_base.string()); + } + return is_descendant; +} + /** @brief Check that the supplied directory path exists, is a directory, and * that the user has adequate permissions to use it. * @@ -329,13 +350,39 @@ gnc_validate_directory (const bfs::path &dirname) if (dirname.empty()) return false; - /* Create directories if they don't exist yet + auto create_dirs = true; + if (build_dir.empty() || !dir_is_descendant (dirname, build_dir)) + { + /* Gnucash won't create a home directory + * if it doesn't exist yet. So if the directory to create + * is a descendant of the homedir, we can't create it either. + * This check is conditioned on do_homedir_check because + * we need to overrule it during build (when guile interferes) + * and testing. + */ + auto home_dir = bfs::path (g_get_home_dir ()); + auto homedir_exists = bfs::exists(home_dir); + auto is_descendant = dir_is_descendant (dirname, home_dir); + if (!homedir_exists && is_descendant) + create_dirs = false; + } + + /* Create directories if they don't exist yet and we can + * * Note this will do nothing if the directory and its * parents already exist, but will fail if the path * points to a file or a softlink. So it serves as a test * for that as well. */ - bfs::create_directories(dirname); + if (create_dirs) + bfs::create_directories(dirname); + else + throw (bfs::filesystem_error ( + std::string (dirname.string() + + " is a descendant of a non-existing home directory. As " + + PACKAGE_NAME + + " will never create a home directory this path can't be used"), + dirname, bst::error_code(bst::errc::permission_denied, bst::generic_category()))); auto d = bfs::directory_entry (dirname); auto perms = d.status().permissions(); @@ -357,8 +404,6 @@ gnc_validate_directory (const bfs::path &dirname) return true; } -static auto gnc_userdata_home = bfs::path(); - /* Will attempt to copy all files and directories from src to dest * Returns true if successful or false if not */ static bool @@ -508,16 +553,15 @@ gnc_filepath_init (void) * in the base of the build directory. This is to deal with all kinds of * issues when the build environment is not a complete environment (like * it could be missing a valid home directory). */ - auto builddir = g_getenv ("GNC_BUILDDIR"); + auto env_build_dir = g_getenv ("GNC_BUILDDIR"); + build_dir = bfs::path(env_build_dir ? env_build_dir : ""); auto running_uninstalled = (g_getenv ("GNC_UNINSTALLED") != NULL); - if (running_uninstalled && builddir) + if (running_uninstalled && !build_dir.empty()) { - auto build_home = g_build_filename (builddir, "gnc_data_home", NULL); - gnc_userdata_home = bfs::path(build_home); - g_free (build_home); + gnc_userdata_home = build_dir / "gnc_data_home"; try { - gnc_validate_directory(gnc_userdata_home); // May throw + gnc_validate_directory (gnc_userdata_home); // May throw have_valid_userdata_home = true; gnc_userdata_home_exists = true; // To prevent possible migration further down } @@ -530,19 +574,18 @@ gnc_filepath_init (void) } } - if (!have_valid_userdata_home) { /* If environment variable GNC_DATA_HOME is set, try whether * it points at a valid directory. */ - auto gnc_userdata_home_env = g_getenv("GNC_DATA_HOME"); + auto gnc_userdata_home_env = g_getenv ("GNC_DATA_HOME"); if (gnc_userdata_home_env) { - gnc_userdata_home = bfs::path(gnc_userdata_home_env); + gnc_userdata_home = bfs::path (gnc_userdata_home_env); try { - gnc_userdata_home_exists = bfs::exists(gnc_userdata_home); - gnc_validate_directory(gnc_userdata_home); // May throw + gnc_userdata_home_exists = bfs::exists (gnc_userdata_home); + gnc_validate_directory (gnc_userdata_home); // May throw have_valid_userdata_home = true; } catch (const bfs::filesystem_error& ex) @@ -563,31 +606,31 @@ gnc_filepath_init (void) gnc_userdata_home = userdata_home / PACKAGE; try { - gnc_userdata_home_exists = bfs::exists(gnc_userdata_home); - gnc_validate_directory(gnc_userdata_home); + gnc_userdata_home_exists = bfs::exists (gnc_userdata_home); + gnc_validate_directory (gnc_userdata_home); } catch (const bfs::filesystem_error& ex) { - g_warning("User data directory doesn't exist, yet could not be created. Proceed with caution.\n" + g_warning ("User data directory doesn't exist, yet could not be created. Proceed with caution.\n" "(Error: %s)", ex.what()); } } auto migrated = FALSE; if (!gnc_userdata_home_exists) - migrated = copy_recursive(bfs::path (g_get_home_dir()) / ".gnucash", - gnc_userdata_home); + migrated = copy_recursive (bfs::path (g_get_home_dir()) / ".gnucash", + gnc_userdata_home); /* Try to create the standard subdirectories for gnucash' user data */ try { - gnc_validate_directory(gnc_userdata_home / "books"); - gnc_validate_directory(gnc_userdata_home / "checks"); - gnc_validate_directory(gnc_userdata_home / "translog"); + gnc_validate_directory (gnc_userdata_home / "books"); + gnc_validate_directory (gnc_userdata_home / "checks"); + gnc_validate_directory (gnc_userdata_home / "translog"); } catch (const bfs::filesystem_error& ex) { - g_warning("Default user data subdirectories don't exist, yet could not be created. Proceed with caution.\n" + g_warning ("Default user data subdirectories don't exist, yet could not be created. Proceed with caution.\n" "(Error: %s)", ex.what()); } diff --git a/libgnucash/core-utils/test/CMakeLists.txt b/libgnucash/core-utils/test/CMakeLists.txt index 1baddd0506..10d53529be 100644 --- a/libgnucash/core-utils/test/CMakeLists.txt +++ b/libgnucash/core-utils/test/CMakeLists.txt @@ -19,9 +19,9 @@ ENDMACRO() ADD_CORE_UTILS_TEST(test-gnc-glib-utils test-gnc-glib-utils.c) ADD_CORE_UTILS_TEST(test-resolve-file-path test-resolve-file-path.c) ADD_CORE_UTILS_TEST(test-userdata-dir test-userdata-dir.c) -#IF (NOT MAC_INTEGRATION AND NOT WIN32) -# ADD_CORE_UTILS_TEST(test-userdata-dir-invalid-home test-userdata-dir-invalid-home.c) -#ENDIF() +IF (NOT MAC_INTEGRATION AND NOT WIN32) + ADD_CORE_UTILS_TEST(test-userdata-dir-invalid-home test-userdata-dir-invalid-home.c) +ENDIF() IF (MAC_INTEGRATION) TARGET_COMPILE_OPTIONS(test-userdata-dir PRIVATE ${OSX_EXTRA_COMPILE_FLAGS}) TARGET_COMPILE_DEFINITIONS(test-userdata-dir PRIVATE ${GTK_MAC_CFLAGS_OTHER}) diff --git a/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c b/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c index 959514789e..b28f3f3349 100644 --- a/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c +++ b/libgnucash/core-utils/test/test-userdata-dir-invalid-home.c @@ -70,6 +70,8 @@ main(G_GNUC_UNUSED int argc, G_GNUC_UNUSED char **argv) #ifndef G_OS_WIN32 int i; const char *tmp_dir = g_get_tmp_dir(); + const char *builddir = g_getenv ("GNC_BUILDDIR"); + char *homedir = g_build_filename (builddir, "notexist", NULL); /* Assume we're not in a build environment to test * the function's actual behaviour in a real world use case, using @@ -80,7 +82,7 @@ main(G_GNUC_UNUSED int argc, G_GNUC_UNUSED char **argv) /* Run usr conf dir tests with an invalid homedir * The code should fall back to using the temporary * directory in that case. */ - g_setenv("HOME", "/notexist", TRUE); + g_setenv("HOME", homedir, TRUE); for (i = 0; strs2[i].funcname != NULL; i++) { char *daout; diff --git a/libgnucash/core-utils/test/test-userdata-dir.c b/libgnucash/core-utils/test/test-userdata-dir.c index d72d9b8685..5a8cd5c502 100644 --- a/libgnucash/core-utils/test/test-userdata-dir.c +++ b/libgnucash/core-utils/test/test-userdata-dir.c @@ -160,13 +160,14 @@ main(int argc, char **argv) g_unsetenv("GNC_UNINSTALLED"); - /* Second run, with existing userdata_dir, but without the GnuCash subdir + /* Second run, with XDG_DATA_HOME set and with existing home_dir, but + * without the XDG_DATA_HOME subdirectories. This test can not be run on OS X or Windows, as our code is not using XDG_DATA_HOME on these platforms */ #ifndef MAC_INTEGRATION #ifndef G_OS_WIN32 + g_mkdir_with_parents(home_dir, 0750); userdata_dir = g_build_filename(home_dir, ".local", "share", (gchar *)NULL); - g_mkdir_with_parents(userdata_dir, 0750); g_setenv("XDG_DATA_HOME", userdata_dir, TRUE); gnc_filepath_init(); for (i = 0; strs2[i].funcname != NULL; i++)