Merge #14146: wallet: Remove trailing separators from -walletdir arg

2d471636eb9160ab51b08e491e3f003f57adbc36 wallet: Remove trailing separators from -walletdir arg (Pierre Rochard)
ea3009ee942188750480ca6cc273b2b91cf77ded wallet: Add walletdir arg unit tests (Pierre Rochard)

Pull request description:

  If a user passes in a path with a trailing separator as the `walletdir`, multiple BerkeleyEnvironments may be created in the same directory which can lead to data corruption.

  Discovered while reviewing https://github.com/bitcoin/bitcoin/pull/12493#issuecomment-417147646

Tree-SHA512: f2bbf1749d904fd3f326b88f2ead58c8386034355910906d7faea155d518642e9cd4ceb3cae272f2d9d8feb61f126523e1c97502799d24e4315bb53e49fd7c09

3 files changed, 141 insertions, 0 deletions

diff --git a/src/wallet/test/init_test_fixture.cpp b/src/wallet/test/init_test_fixture.cpp
new file mode 100644
index 0000000000..1453029c9c
--- /dev/null
+++ b/src/wallet/test/init_test_fixture.cpp
@@ -0,0 +1,42 @@
+// Copyright (c) 2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <fs.h>
+
+#include <wallet/test/init_test_fixture.h>
+
+InitWalletDirTestingSetup::InitWalletDirTestingSetup(const std::string& chainName): BasicTestingSetup(chainName)
+{
+    std::string sep;
+    sep += fs::path::preferred_separator;
+
+    m_datadir = SetDataDir("tempdir");
+    m_cwd = fs::current_path();
+
+    m_walletdir_path_cases["default"] = m_datadir / "wallets";
+    m_walletdir_path_cases["custom"] = m_datadir / "my_wallets";
+    m_walletdir_path_cases["nonexistent"] = m_datadir / "path_does_not_exist";
+    m_walletdir_path_cases["file"] = m_datadir / "not_a_directory.dat";
+    m_walletdir_path_cases["trailing"] = m_datadir / "wallets" / sep;
+    m_walletdir_path_cases["trailing2"] = m_datadir / "wallets" / sep / sep;
+
+    fs::current_path(m_datadir);
+    m_walletdir_path_cases["relative"] = "wallets";
+
+    fs::create_directories(m_walletdir_path_cases["default"]);
+    fs::create_directories(m_walletdir_path_cases["custom"]);
+    fs::create_directories(m_walletdir_path_cases["relative"]);
+    std::ofstream f(m_walletdir_path_cases["file"].BOOST_FILESYSTEM_C_STR);
+    f.close();
+}
+
+InitWalletDirTestingSetup::~InitWalletDirTestingSetup()
+{
+    fs::current_path(m_cwd);
+}
+
+void InitWalletDirTestingSetup::SetWalletDir(const fs::path& walletdir_path)
+{
+    gArgs.ForceSetArg("-walletdir", walletdir_path.string());
+}
\ No newline at end of file
diff --git a/src/wallet/test/init_test_fixture.h b/src/wallet/test/init_test_fixture.h
new file mode 100644
index 0000000000..5684adbece
--- /dev/null
+++ b/src/wallet/test/init_test_fixture.h
@@ -0,0 +1,21 @@
+// Copyright (c) 2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H
+#define BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H
+
+#include <test/test_bitcoin.h>
+
+
+struct InitWalletDirTestingSetup: public BasicTestingSetup {
+    explicit InitWalletDirTestingSetup(const std::string& chainName = CBaseChainParams::MAIN);
+    ~InitWalletDirTestingSetup();
+    void SetWalletDir(const fs::path& walletdir_path);
+
+    fs::path m_datadir;
+    fs::path m_cwd;
+    std::map<std::string, fs::path> m_walletdir_path_cases;
+};
+
+#endif // BITCOIN_WALLET_TEST_INIT_TEST_FIXTURE_H
diff --git a/src/wallet/test/init_tests.cpp b/src/wallet/test/init_tests.cpp
new file mode 100644
index 0000000000..7048547b2b
--- /dev/null
+++ b/src/wallet/test/init_tests.cpp
@@ -0,0 +1,78 @@
+// Copyright (c) 2018 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#include <boost/test/unit_test.hpp>
+
+#include <test/test_bitcoin.h>
+#include <wallet/test/init_test_fixture.h>
+
+#include <init.h>
+#include <walletinitinterface.h>
+#include <wallet/wallet.h>
+
+
+BOOST_FIXTURE_TEST_SUITE(init_tests, InitWalletDirTestingSetup)
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_default)
+{
+    SetWalletDir(m_walletdir_path_cases["default"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == true);
+    fs::path walletdir = gArgs.GetArg("-walletdir", "");
+    fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
+    BOOST_CHECK(walletdir == expected_path);
+}
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_custom)
+{
+    SetWalletDir(m_walletdir_path_cases["custom"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == true);
+    fs::path walletdir = gArgs.GetArg("-walletdir", "");
+    fs::path expected_path = fs::canonical(m_walletdir_path_cases["custom"]);
+    BOOST_CHECK(walletdir == expected_path);
+}
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_does_not_exist)
+{
+    SetWalletDir(m_walletdir_path_cases["nonexistent"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == false);
+}
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_directory)
+{
+    SetWalletDir(m_walletdir_path_cases["file"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == false);
+}
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_is_not_relative)
+{
+    SetWalletDir(m_walletdir_path_cases["relative"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == false);
+}
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing)
+{
+    SetWalletDir(m_walletdir_path_cases["trailing"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == true);
+    fs::path walletdir = gArgs.GetArg("-walletdir", "");
+    fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
+    BOOST_CHECK(walletdir == expected_path);
+}
+
+BOOST_AUTO_TEST_CASE(walletinit_verify_walletdir_no_trailing2)
+{
+    SetWalletDir(m_walletdir_path_cases["trailing2"]);
+    bool result = g_wallet_init_interface.Verify();
+    BOOST_CHECK(result == true);
+    fs::path walletdir = gArgs.GetArg("-walletdir", "");
+    fs::path expected_path = fs::canonical(m_walletdir_path_cases["default"]);
+    BOOST_CHECK(walletdir == expected_path);
+}
+
+BOOST_AUTO_TEST_SUITE_END()