mirror of
https://github.com/dashpay/dash.git
synced 2024-12-25 12:02:48 +01:00
Merge #12220: Error if relative -walletdir is specified
ec527c6
Don't allow relative -walletdir paths (Russell Yanofsky)
Pull request description:
This makes it an error to explicitly specify a non-absolute -walletdir path, and also adds a debug.log warning if a relative rather than absolute -datadir path is configured.
Specifying paths relative to the current working directory in a daemon process can be dangerous, because files can fail to be located even if the configuration doesn't change, but the daemon is started up differently.
Specifying a relative -datadir now adds a warning to the debug log. It would not be backwards-compatible to forbid relative -datadir paths entirely, and it could also be inconvenient for command line testing.
Specifying a relative -walletdir now results in a startup error. But since the -walletdir option is new in 0.16.0, there should be no compatibility issues. Another reason not to use working directory paths for -walletdir specifically is that the default -walletdir is a "wallets" subdirectory inside the datadir, so it could be surprising that setting -walletdir manually would choose a directory rooted in a completely different location.
Tree-SHA512: 67cbdae677f82487a9031c5ec96b0205a488ab08718a0f4f49365e025119f3d7f6cfc88ba1eba04c1ecd8b9561a5b2c42e2e1a267af7c08c76b83e5e361f5a31
This commit is contained in:
parent
cc2cd7291b
commit
64e33f715f
@ -1548,6 +1548,15 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
|
||||
LogPrintf("Using config file %s\n", GetConfigFile(gArgs.GetArg("-conf", BITCOIN_CONF_FILENAME)).string());
|
||||
LogPrintf("Using at most %i automatic connections (%i file descriptors available)\n", nMaxConnections, nFD);
|
||||
|
||||
// Warn about relative -datadir path.
|
||||
if (gArgs.IsArgSet("-datadir") && !fs::path(gArgs.GetArg("-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 Dash Core is started in the future "
|
||||
"from a different location, it will be unable to locate the current data files. There could "
|
||||
"also be data loss if Dash Core is started while in a temporary directory.\n",
|
||||
gArgs.GetArg("-datadir", ""), fs::current_path().string());
|
||||
}
|
||||
|
||||
InitSignatureCache();
|
||||
InitScriptExecutionCache();
|
||||
|
||||
|
@ -212,11 +212,15 @@ bool VerifyWallets()
|
||||
return true;
|
||||
}
|
||||
|
||||
if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {
|
||||
if (fs::exists(fs::system_complete(gArgs.GetArg("-walletdir", "")))) {
|
||||
return InitError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), gArgs.GetArg("-walletdir", "").c_str()));
|
||||
if (gArgs.IsArgSet("-walletdir")) {
|
||||
fs::path wallet_dir = gArgs.GetArg("-walletdir", "");
|
||||
if (!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()));
|
||||
} else if (!wallet_dir.is_absolute()) {
|
||||
return InitError(strprintf(_("Specified -walletdir \"%s\" is a relative path"), wallet_dir.string()));
|
||||
}
|
||||
return InitError(strprintf(_("Specified -walletdir \"%s\" does not exist"), gArgs.GetArg("-walletdir", "").c_str()));
|
||||
}
|
||||
|
||||
LogPrintf("Using wallet directory %s\n", GetWalletDir().string());
|
||||
|
@ -9,7 +9,7 @@ fs::path GetWalletDir()
|
||||
fs::path path;
|
||||
|
||||
if (gArgs.IsArgSet("-walletdir")) {
|
||||
path = fs::system_complete(gArgs.GetArg("-walletdir", ""));
|
||||
path = gArgs.GetArg("-walletdir", "");
|
||||
if (!fs::is_directory(path)) {
|
||||
// If the path specified doesn't exist, we return the deliberately
|
||||
// invalid empty string.
|
||||
|
@ -30,6 +30,10 @@ class MultiWalletTest(BitcoinTestFramework):
|
||||
|
||||
self.stop_node(0)
|
||||
|
||||
self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
|
||||
self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
|
||||
self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())
|
||||
|
||||
# should not initialize if there are duplicate wallets
|
||||
self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')
|
||||
|
||||
|
@ -254,18 +254,18 @@ class BitcoinTestFramework(object):
|
||||
for i in range(num_nodes):
|
||||
self.nodes.append(TestNode(old_num_nodes + i, self.options.tmpdir, extra_args[i], rpchost, timewait=timewait, binary=binary[i], stderr=stderr, mocktime=self.mocktime, coverage_dir=self.options.coveragedir, use_cli=self.options.usecli))
|
||||
|
||||
def start_node(self, i, extra_args=None, stderr=None):
|
||||
def start_node(self, i, *args, **kwargs):
|
||||
"""Start a dashd"""
|
||||
|
||||
node = self.nodes[i]
|
||||
|
||||
node.start(extra_args, stderr)
|
||||
node.start(*args, **kwargs)
|
||||
node.wait_for_rpc_connection()
|
||||
|
||||
if self.options.coveragedir is not None:
|
||||
coverage.write_all_rpc_commands(self.options.coveragedir, node.rpc)
|
||||
|
||||
def start_nodes(self, extra_args=None, stderr=None):
|
||||
def start_nodes(self, extra_args=None, stderr=None, *args, **kwargs):
|
||||
"""Start multiple dashds"""
|
||||
|
||||
if extra_args is None:
|
||||
@ -273,7 +273,7 @@ class BitcoinTestFramework(object):
|
||||
assert_equal(len(extra_args), self.num_nodes)
|
||||
try:
|
||||
for i, node in enumerate(self.nodes):
|
||||
node.start(extra_args[i], stderr)
|
||||
node.start(extra_args[i], stderr, *args, **kwargs)
|
||||
for node in self.nodes:
|
||||
node.wait_for_rpc_connection()
|
||||
except:
|
||||
@ -305,10 +305,10 @@ class BitcoinTestFramework(object):
|
||||
self.stop_node(i)
|
||||
self.start_node(i, extra_args)
|
||||
|
||||
def assert_start_raises_init_error(self, i, extra_args=None, expected_msg=None):
|
||||
def assert_start_raises_init_error(self, i, extra_args=None, expected_msg=None, *args, **kwargs):
|
||||
with tempfile.SpooledTemporaryFile(max_size=2**16) as log_stderr:
|
||||
try:
|
||||
self.start_node(i, extra_args, stderr=log_stderr)
|
||||
self.start_node(i, extra_args, stderr=log_stderr, *args, **kwargs)
|
||||
self.stop_node(i)
|
||||
except Exception as e:
|
||||
assert 'dashd exited' in str(e) # node must have shutdown
|
||||
|
@ -85,13 +85,13 @@ class TestNode():
|
||||
assert self.rpc_connected and self.rpc is not None, "Error: no RPC connection"
|
||||
return getattr(self.rpc, name)
|
||||
|
||||
def start(self, extra_args=None, stderr=None):
|
||||
def start(self, extra_args=None, stderr=None, *args, **kwargs):
|
||||
"""Start the node."""
|
||||
if extra_args is None:
|
||||
extra_args = self.extra_args
|
||||
if stderr is None:
|
||||
stderr = self.stderr
|
||||
self.process = subprocess.Popen(self.args + extra_args, stderr=stderr)
|
||||
self.process = subprocess.Popen(self.args + extra_args, stderr=stderr, *args, **kwargs)
|
||||
self.running = True
|
||||
self.log.debug("dashd started, waiting for RPC to come up")
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user