diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -2151,7 +2151,7 @@ // Warn about relative -datadir path. if (args.IsArgSet("-datadir") && - !fs::PathFromString(args.GetArg("-datadir", "")).is_absolute()) { + !args.GetPathArg("-datadir").is_absolute()) { LogPrintf("Warning: relative datadir option '%s' specified, which will " "be interpreted relative to the current working directory " "'%s'. This is fragile, because if bitcoin is started in the " diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -158,6 +158,97 @@ BOOST_CHECK_EQUAL(m_local_args.GetIntArg("-bar", 11), 0); } +BOOST_AUTO_TEST_CASE(patharg) { + const auto dir = std::make_pair("-dir", ArgsManager::ALLOW_ANY); + SetupArgs({dir}); + ResetArgs(""); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), fs::path{}); + + const fs::path root_path{"/"}; + ResetArgs("-dir=/"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), root_path); + + ResetArgs("-dir=/."); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), root_path); + + ResetArgs("-dir=/./"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), root_path); + + ResetArgs("-dir=/.//"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), root_path); + +#ifdef WIN32 + const fs::path win_root_path{"C:\\"}; + ResetArgs("-dir=C:\\"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), win_root_path); + + ResetArgs("-dir=C:/"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), win_root_path); + + ResetArgs("-dir=C:\\\\"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), win_root_path); + + ResetArgs("-dir=C:\\."); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), win_root_path); + + ResetArgs("-dir=C:\\.\\"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), win_root_path); + + ResetArgs("-dir=C:\\.\\\\"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), win_root_path); +#endif + + const fs::path absolute_path{"/home/user/.bitcoin"}; + ResetArgs("-dir=/home/user/.bitcoin"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/root/../home/user/.bitcoin"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/home/./user/.bitcoin"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/home/user/.bitcoin/"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/home/user/.bitcoin//"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/home/user/.bitcoin/."); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/home/user/.bitcoin/./"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + ResetArgs("-dir=/home/user/.bitcoin/.//"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), absolute_path); + + const fs::path relative_path{"user/.bitcoin"}; + ResetArgs("-dir=user/.bitcoin"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=somewhere/../user/.bitcoin"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=user/./.bitcoin"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=user/.bitcoin/"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=user/.bitcoin//"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=user/.bitcoin/."); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=user/.bitcoin/./"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); + + ResetArgs("-dir=user/.bitcoin/.//"); + BOOST_CHECK_EQUAL(m_local_args.GetPathArg("-dir"), relative_path); +} + BOOST_AUTO_TEST_CASE(doubledash) { const auto foo = std::make_pair("-foo", ArgsManager::ALLOW_ANY); const auto bar = std::make_pair("-bar", ArgsManager::ALLOW_ANY); diff --git a/src/util/system.h b/src/util/system.h --- a/src/util/system.h +++ b/src/util/system.h @@ -233,6 +233,17 @@ */ const std::list GetUnrecognizedSections() const; + /** + * Get a normalized path from a specified pathlike argument + * + * It is guaranteed that the returned path has no trailing slashes. + * + * @param pathlike_arg Pathlike argument to get a path from (e.g., + * "-datadir", "-blocksdir" or "-walletdir") + * @return Normalized path which is get from a specified pathlike argument + */ + fs::path GetPathArg(std::string pathlike_arg) const; + /** * Get blocks directory path * diff --git a/src/util/system.cpp b/src/util/system.cpp --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -244,19 +244,6 @@ return true; } -namespace { -fs::path StripRedundantLastElementsOfPath(const fs::path &path) { - auto result = path; - while (result.filename().empty() || - fs::PathToString(result.filename()) == ".") { - result = result.parent_path(); - } - - assert(fs::equivalent(result, path)); - return result; -} -} // namespace - // Define default constructor and destructor that are not inline, so code // instantiating this class doesn't need to #include class definitions for all // members. For example, m_settings has an internal dependency on univalue. @@ -405,6 +392,13 @@ return std::nullopt; } +fs::path ArgsManager::GetPathArg(std::string pathlike_arg) const { + auto result = + fs::PathFromString(GetArg(pathlike_arg, "")).lexically_normal(); + // Remove trailing slash, if present. + return result.has_filename() ? result : result.parent_path(); +} + const fs::path &ArgsManager::GetBlocksDirPath() const { LOCK(cs_args); fs::path &path = m_cached_blocks_path; @@ -416,7 +410,7 @@ } if (IsArgSet("-blocksdir")) { - path = fs::absolute(fs::PathFromString(GetArg("-blocksdir", ""))); + path = fs::absolute(GetPathArg("-blocksdir")); if (!fs::is_directory(path)) { path = ""; return path; @@ -441,10 +435,10 @@ if (!path.empty()) { return path; } - std::string datadir = GetArg("-datadir", ""); + + const fs::path datadir{GetPathArg("-datadir")}; if (!datadir.empty()) { - path = fs::absolute( - StripRedundantLastElementsOfPath(fs::PathFromString(datadir))); + path = fs::absolute(datadir); if (!fs::is_directory(path)) { path = ""; return path; @@ -461,7 +455,6 @@ fs::create_directories(path / "wallets"); } - path = StripRedundantLastElementsOfPath(path); return path; } @@ -839,9 +832,8 @@ } bool CheckDataDirOption() { - std::string datadir = gArgs.GetArg("-datadir", ""); - return datadir.empty() || - fs::is_directory(fs::absolute(fs::PathFromString(datadir))); + const fs::path datadir{gArgs.GetPathArg("-datadir")}; + return datadir.empty() || fs::is_directory(fs::absolute(datadir)); } fs::path GetConfigFile(const std::string &confPath) { diff --git a/src/wallet/load.cpp b/src/wallet/load.cpp --- a/src/wallet/load.cpp +++ b/src/wallet/load.cpp @@ -21,8 +21,7 @@ bool VerifyWallets(interfaces::Chain &chain) { if (gArgs.IsArgSet("-walletdir")) { - fs::path wallet_dir = - fs::PathFromString(gArgs.GetArg("-walletdir", "")); + const fs::path wallet_dir{gArgs.GetPathArg("-walletdir")}; std::error_code error; // The canonical path cleans the path, preventing >1 Berkeley // environment instances for the same directory diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp --- a/src/wallet/test/init_tests.cpp +++ b/src/wallet/test/init_tests.cpp @@ -17,7 +17,7 @@ SetWalletDir(m_walletdir_path_cases["default"]); bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); - fs::path walletdir = fs::PathFromString(gArgs.GetArg("-walletdir", "")); + fs::path walletdir = gArgs.GetPathArg("-walletdir"); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); BOOST_CHECK(walletdir == expected_path); } @@ -26,7 +26,7 @@ SetWalletDir(m_walletdir_path_cases["custom"]); bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); - fs::path walletdir = fs::PathFromString(gArgs.GetArg("-walletdir", "")); + fs::path walletdir = gArgs.GetPathArg("-walletdir"); fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]); BOOST_CHECK(walletdir == expected_path); } @@ -62,7 +62,7 @@ SetWalletDir(m_walletdir_path_cases["trailing"]); bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); - fs::path walletdir = fs::PathFromString(gArgs.GetArg("-walletdir", "")); + fs::path walletdir = gArgs.GetPathArg("-walletdir"); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); BOOST_CHECK(walletdir == expected_path); } @@ -73,7 +73,7 @@ SetWalletDir(m_walletdir_path_cases["trailing2"]); bool result = m_wallet_client->verify(); BOOST_CHECK(result == true); - fs::path walletdir = fs::PathFromString(gArgs.GetArg("-walletdir", "")); + fs::path walletdir = gArgs.GetPathArg("-walletdir"); fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]); BOOST_CHECK(walletdir == expected_path); } diff --git a/src/wallet/walletutil.cpp b/src/wallet/walletutil.cpp --- a/src/wallet/walletutil.cpp +++ b/src/wallet/walletutil.cpp @@ -14,7 +14,7 @@ fs::path path; if (gArgs.IsArgSet("-walletdir")) { - path = fs::PathFromString(gArgs.GetArg("-walletdir", "")); + path = gArgs.GetPathArg("-walletdir"); if (!fs::is_directory(path)) { // If the path specified doesn't exist, we return the deliberately // invalid empty string.