LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

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

Browse source 
2d471636eb wallet: Remove trailing separators from -walletdir arg (Pierre Rochard)
ea3009ee94 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
This commit is contained in:
Wladimir J. van der Laan 2018-10-18 10:58:18 +02:00
commit d98777f302
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
6 changed files with 153 additions and 4 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
build_msvc/test_bitcoin/test_bitcoin.vcxproj
View file

@ -24,7 +24,7 @@
<ClCompile Include="..\..\src\wallet\test\*_tests.cpp" />
<ClCompile Include="..\..\src\test\test_bitcoin.cpp" />
<ClCompile Include="..\..\src\test\test_bitcoin_main.cpp" />
<ClCompile Include="..\..\src\wallet\test\wallet_test_fixture.cpp" />
<ClCompile Include="..\..\src\wallet\test\*_fixture.cpp" />
</ItemGroup>
<ItemGroup>
<ProjectReference Include="..\libbitcoinconsensus\libbitcoinconsensus.vcxproj">

7
src/Makefile.test.include
View file

@ -111,11 +111,14 @@ BITCOIN_TESTS += \
wallet/test/psbt_wallet_tests.cpp \
wallet/test/wallet_tests.cpp \
wallet/test/wallet_crypto_tests.cpp \
wallet/test/coinselector_tests.cpp
wallet/test/coinselector_tests.cpp \
wallet/test/init_tests.cpp
BITCOIN_TEST_SUITE += \
wallet/test/wallet_test_fixture.cpp \
wallet/test/wallet_test_fixture.h
wallet/test/wallet_test_fixture.h \
wallet/test/init_test_fixture.cpp \
wallet/test/init_test_fixture.h
endif
test_test_bitcoin_SOURCES = $(BITCOIN_TEST_SUITE) $(BITCOIN_TESTS) $(JSON_TEST_FILES) $(RAW_TEST_FILES)

7
src/wallet/init.cpp
View file

@ -182,13 +182,18 @@ bool WalletInit::Verify() const
if (gArgs.IsArgSet("-walletdir")) {
fs::path wallet_dir = gArgs.GetArg("-walletdir", "");
if (!fs::exists(wallet_dir)) {
boost::system::error_code error;
// The canonical path cleans the path, preventing >1 Berkeley environment instances for the same directory
fs::path canonical_wallet_dir = fs::canonical(wallet_dir, error);
if (error || !fs::exists(wallet_dir)) {
return InitError(strprintf(_("Specified -walletdir \"%s\" does not exist"), wallet_dir.string()));
} else if (!fs::is_directory(wallet_dir)) {
return InitError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), wallet_dir.string()));
// The canonical path transforms relative paths into absolute ones, so we check the non-canonical version
} else if (!wallet_dir.is_absolute()) {
return InitError(strprintf(_("Specified -walletdir \"%s\" is a relative path"), wallet_dir.string()));
}
gArgs.ForceSetArg("-walletdir", canonical_wallet_dir.string());
}
LogPrintf("Using wallet directory %s\n", GetWalletDir().string());

42
src/wallet/test/init_test_fixture.cpp Normal file
View file

@ -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());
}

21
src/wallet/test/init_test_fixture.h Normal file
View file

@ -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

78
src/wallet/test/init_tests.cpp Normal file
View file

@ -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()