From 80cd64e84296f1166e133c237fa0afc046b01ce2 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 4 Feb 2022 08:59:17 -0500 Subject: [PATCH 1/2] Re-enable util_datadir check disabled in #20744 This should also fix an assert error if a -datadir with a trailing slash is used on windows. This appears to be a real error and regression introduced with #20744. On windows (or at least wine), fs calls that actuallly access the filesystem like fs::equivalent or fs::exists seem to treat directory paths with trailing slashes as not existing, so it's necessary to normalize these paths before using them. This fix adds a path::lexically_normal() call to the failing assert so it passes. --- src/test/util_tests.cpp | 3 --- src/util/system.cpp | 2 +- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index b6479efe7d1..52b24327eca 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -71,12 +71,9 @@ BOOST_AUTO_TEST_CASE(util_datadir) args.ClearPathCache(); BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); -#ifndef WIN32 - // Windows does not consider "datadir/.//" to be a valid directory path. args.ForceSetArg("-datadir", fs::PathToString(dd_norm) + "/.//"); args.ClearPathCache(); BOOST_CHECK_EQUAL(dd_norm, args.GetDataDirBase()); -#endif } BOOST_AUTO_TEST_CASE(util_check) diff --git a/src/util/system.cpp b/src/util/system.cpp index 0ee63f6381d..8d0cec249d8 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -253,7 +253,7 @@ fs::path StripRedundantLastElementsOfPath(const fs::path& path) result = result.parent_path(); } - assert(fs::equivalent(result, path)); + assert(fs::equivalent(result, path.lexically_normal())); return result; } } // namespace From d216bc8d76d7f4e9dce58b0bb732a2d4deaf23b6 Mon Sep 17 00:00:00 2001 From: Ryan Ofsky Date: Fri, 4 Feb 2022 08:59:17 -0500 Subject: [PATCH 2/2] Re-enable walletinit_verify_walletdir_no_trailing2 test disabled in #20744 This should also fix an init error if a -walletdir with a trailing slash is used on windows. This appears to be a real error and regression introduced with #20744. On windows (or at least wine), fs calls that actuallly access the filesystem like fs::equivalent or fs::exists seem to treat directory paths with trailing slashes as not existing, so it's necessary to normalize these paths before using them. This change passes canonical paths to fs calls validating the -walletdir path to fix this. --- src/wallet/load.cpp | 6 ++++-- src/wallet/test/init_tests.cpp | 3 --- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp index 4949ed7dc90..6a74f2eb846 100644 --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -31,11 +31,13 @@ bool VerifyWallets(WalletContext& context) fs::path wallet_dir = fs::PathFromString(args.GetArg("-walletdir", "")); std::error_code error; // The canonical path cleans the path, preventing >1 Berkeley environment instances for the same directory + // It also lets the fs::exists and fs::is_directory checks below pass on windows, since they return false + // if a path has trailing slashes, and it strips trailing slashes. fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error); - if (error || !fs::exists(wallet_dir)) { + if (error || !fs::exists(canonical_wallet_dir)) { chain.initError(strprintf(_("Specified -walletdir \"%s\" does not exist"), fs::PathToString(wallet_dir))); return false; - } else if (!fs::is_directory(wallet_dir)) { + } else if (!fs::is_directory(canonical_wallet_dir)) { chain.initError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), fs::PathToString(wallet_dir))); return false; // The canonical path transforms relative paths into absolute ones, so we check the non-canonical version diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp index d1df48a3125..c1cae5c5f65 100644 --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -73,8 +73,6 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing) BOOST_CHECK_EQUAL(walletdir, expected_path); } -#ifndef WIN32 -// Windows does not consider "datadir/wallets//" to be a valid directory path. BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) { SetWalletDir(m_walletdir_path_cases["trailing2"]); @@ -84,7 +82,6 @@ BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2) fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); BOOST_CHECK_EQUAL(walletdir, expected_path); } -#endif BOOST_AUTO_TEST_SUITE_END() } // namespace wallet