diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index dd7b21e5671..5c5965245bc 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -82,7 +82,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-conf=", strprintf("Specify configuration file. Relative paths will be prefixed by datadir location. (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-generate", strprintf("Generate blocks, equivalent to RPC getnewaddress followed by RPC generatetoaddress. Optional positional integer " "arguments are number of blocks to generate (default: %s) and maximum iterations to try (default: %s), equivalent to " @@ -94,7 +94,7 @@ static void SetupCliArgs(ArgsManager& argsman) argsman.AddArg("-netinfo", strprintf("Get network peer connection information from the remote server. An optional argument from 0 to %d can be passed for different peers listings (default: 0). If a non-zero value is passed, an additional \"outonly\" (or \"o\") argument can be passed to see outbound peers only. Pass \"help\" (or \"h\") for detailed help documentation.", NETINFO_MAX_LEVEL), ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS); SetupChainParamsBaseOptions(argsman); - argsman.AddArg("-color=", strprintf("Color setting for CLI output (default: %s). Valid values: always, auto (add color codes when standard output is connected to a terminal and OS is not WIN32), never.", DEFAULT_COLOR_SETTING), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); + argsman.AddArg("-color=", strprintf("Color setting for CLI output (default: %s). Valid values: always, auto (add color codes when standard output is connected to a terminal and OS is not WIN32), never. Only applies to the output of -getinfo.", DEFAULT_COLOR_SETTING), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcclienttimeout=", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcconnect=", strprintf("Send commands to node running on (default: %s)", DEFAULT_RPCCONNECT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -145,10 +145,10 @@ static int AppInitRPC(int argc, char* argv[]) tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error); return EXIT_FAILURE; } - if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { + if (argc < 2 || HelpRequested(gArgs) || gArgs.GetBoolArg("-version", false)) { std::string strUsage = CLIENT_NAME " RPC client version " + FormatFullVersion() + "\n"; - if (gArgs.IsArgSet("-version")) { + if (gArgs.GetBoolArg("-version", false)) { strUsage += FormatParagraph(LicenseInfo()); } else { strUsage += "\n" diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp index 7f96c626b17..a627c24e868 100644 --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -105,11 +105,11 @@ static int AppInitRawTx(int argc, char* argv[]) fCreateBlank = gArgs.GetBoolArg("-create", false); - if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { + if (argc < 2 || HelpRequested(gArgs) || gArgs.GetBoolArg("-version", false)) { // First part of help message is specific to this utility std::string strUsage = CLIENT_NAME " bitcoin-tx utility version " + FormatFullVersion() + "\n"; - if (gArgs.IsArgSet("-version")) { + if (gArgs.GetBoolArg("-version", false)) { strUsage += FormatParagraph(LicenseInfo()); } else { strUsage += "\n" diff --git a/src/bitcoin-util.cpp b/src/bitcoin-util.cpp index c7d10b6622b..ff2e4fb8005 100644 --- a/src/bitcoin-util.cpp +++ b/src/bitcoin-util.cpp @@ -50,11 +50,11 @@ static int AppInitUtil(ArgsManager& args, int argc, char* argv[]) return EXIT_FAILURE; } - if (HelpRequested(args) || args.IsArgSet("-version")) { + if (HelpRequested(args) || args.GetBoolArg("-version", false)) { // First part of help message is specific to this utility std::string strUsage = CLIENT_NAME " bitcoin-util utility version " + FormatFullVersion() + "\n"; - if (args.IsArgSet("-version")) { + if (args.GetBoolArg("-version", false)) { strUsage += FormatParagraph(LicenseInfo()); } else { strUsage += "\n" diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index be316209455..033cf7a0f39 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -34,7 +34,7 @@ static void SetupWalletToolArgs(ArgsManager& argsman) SetupChainParamsBaseOptions(argsman); argsman.AddArg("-version", "Print version and exit", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-wallet=", "Specify wallet name", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-dumpfile=", "When used with 'dump', writes out the records to this file. When used with 'createfromdump', loads the records into a new wallet.", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-debug=", "Output debugging information (default: 0).", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST); @@ -60,10 +60,10 @@ static std::optional WalletAppInit(ArgsManager& args, int argc, char* argv[ return EXIT_FAILURE; } const bool missing_args{argc < 2}; - if (missing_args || HelpRequested(args) || args.IsArgSet("-version")) { + if (missing_args || HelpRequested(args) || args.GetBoolArg("-version", false)) { std::string strUsage = strprintf("%s bitcoin-wallet utility version", CLIENT_NAME) + " " + FormatFullVersion() + "\n"; - if (args.IsArgSet("-version")) { + if (args.GetBoolArg("-version", false)) { strUsage += FormatParagraph(LicenseInfo()); } else { strUsage += "\n" diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index 09a35d4bad4..5d5f06127d2 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -135,10 +135,10 @@ static bool ParseArgs(NodeContext& node, int argc, char* argv[]) static bool ProcessInitCommands(ArgsManager& args) { // Process help and version before taking care about datadir - if (HelpRequested(args) || args.IsArgSet("-version")) { + if (HelpRequested(args) || args.GetBoolArg("-version", false)) { std::string strUsage = CLIENT_NAME " daemon version " + FormatFullVersion() + "\n"; - if (args.IsArgSet("-version")) { + if (args.GetBoolArg("-version", false)) { strUsage += FormatParagraph(LicenseInfo()); } else { strUsage += "\n" diff --git a/src/common/config.cpp b/src/common/config.cpp index 98223fc3e3a..fac4aa314c5 100644 --- a/src/common/config.cpp +++ b/src/common/config.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -128,12 +129,18 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) } const auto conf_path{GetConfigFilePath()}; - std::ifstream stream{conf_path}; - - // not ok to have a config file specified that cannot be opened - if (IsArgSet("-conf") && !stream.good()) { - error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path)); - return false; + std::ifstream stream; + if (!conf_path.empty()) { // path is empty when -noconf is specified + if (fs::is_directory(conf_path)) { + error = strprintf("Config file \"%s\" is a directory.", fs::PathToString(conf_path)); + return false; + } + stream = std::ifstream{conf_path}; + // If the file is explicitly specified, it must be readable + if (IsArgSet("-conf") && !stream.good()) { + error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path)); + return false; + } } // ok to not have a config file if (stream.good()) { @@ -175,7 +182,12 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) const size_t default_includes = add_includes({}); for (const std::string& conf_file_name : conf_file_names) { - std::ifstream conf_file_stream{AbsPathForConfigVal(*this, fs::PathFromString(conf_file_name), /*net_specific=*/false)}; + const auto include_conf_path{AbsPathForConfigVal(*this, fs::PathFromString(conf_file_name), /*net_specific=*/false)}; + if (fs::is_directory(include_conf_path)) { + error = strprintf("Included config file \"%s\" is a directory.", fs::PathToString(include_conf_path)); + return false; + } + std::ifstream conf_file_stream{include_conf_path}; if (conf_file_stream.good()) { if (!ReadConfigStream(conf_file_stream, conf_file_name, error, ignore_invalid_keys)) { return false; @@ -213,7 +225,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys) fs::path AbsPathForConfigVal(const ArgsManager& args, const fs::path& path, bool net_specific) { - if (path.is_absolute()) { + if (path.is_absolute() || path.empty()) { return path; } return fsbridge::AbsPathJoin(net_specific ? args.GetDataDirNet() : args.GetDataDirBase(), path); diff --git a/src/common/init.cpp b/src/common/init.cpp index 412d73aec70..5a704404689 100644 --- a/src/common/init.cpp +++ b/src/common/init.cpp @@ -62,29 +62,36 @@ std::optional InitConfig(ArgsManager& args, SettingsAbortFn setting fs::create_directories(net_path / "wallets"); } - // Show an error or warning if there is a bitcoin.conf file in the + // Show an error or warn/log if there is a bitcoin.conf file in the // datadir that is being ignored. const fs::path base_config_path = base_path / BITCOIN_CONF_FILENAME; - if (fs::exists(base_config_path) && !fs::equivalent(orig_config_path, base_config_path)) { - const std::string cli_config_path = args.GetArg("-conf", ""); - const std::string config_source = cli_config_path.empty() - ? strprintf("data directory %s", fs::quoted(fs::PathToString(orig_datadir_path))) - : strprintf("command line argument %s", fs::quoted("-conf=" + cli_config_path)); - const std::string error = strprintf( - "Data directory %1$s contains a %2$s file which is ignored, because a different configuration file " - "%3$s from %4$s is being used instead. Possible ways to address this would be to:\n" - "- Delete or rename the %2$s file in data directory %1$s.\n" - "- Change datadir= or conf= options to specify one configuration file, not two, and use " - "includeconf= to include any other configuration files.\n" - "- Set allowignoredconf=1 option to treat this condition as a warning, not an error.", - fs::quoted(fs::PathToString(base_path)), - fs::quoted(BITCOIN_CONF_FILENAME), - fs::quoted(fs::PathToString(orig_config_path)), - config_source); - if (args.GetBoolArg("-allowignoredconf", false)) { - LogPrintf("Warning: %s\n", error); - } else { - return ConfigError{ConfigStatus::FAILED, Untranslated(error)}; + if (fs::exists(base_config_path)) { + if (orig_config_path.empty()) { + LogInfo( + "Data directory %s contains a %s file which is explicitly ignored using -noconf.", + fs::quoted(fs::PathToString(base_path)), + fs::quoted(BITCOIN_CONF_FILENAME)); + } else if (!fs::equivalent(orig_config_path, base_config_path)) { + const std::string cli_config_path = args.GetArg("-conf", ""); + const std::string config_source = cli_config_path.empty() + ? strprintf("data directory %s", fs::quoted(fs::PathToString(orig_datadir_path))) + : strprintf("command line argument %s", fs::quoted("-conf=" + cli_config_path)); + std::string error = strprintf( + "Data directory %1$s contains a %2$s file which is ignored, because a different configuration file " + "%3$s from %4$s is being used instead. Possible ways to address this would be to:\n" + "- Delete or rename the %2$s file in data directory %1$s.\n" + "- Change datadir= or conf= options to specify one configuration file, not two, and use " + "includeconf= to include any other configuration files.", + fs::quoted(fs::PathToString(base_path)), + fs::quoted(BITCOIN_CONF_FILENAME), + fs::quoted(fs::PathToString(orig_config_path)), + config_source); + if (args.GetBoolArg("-allowignoredconf", false)) { + LogWarning("%s", error); + } else { + error += "\n- Set allowignoredconf=1 option to treat this condition as a warning, not an error."; + return ConfigError{ConfigStatus::FAILED, Untranslated(error)}; + } } } diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 69dd821dc04..5d906ffa0c2 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -134,8 +134,6 @@ static bool multiUserAuthorized(std::string strUserPass) static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUsernameOut) { - if (strRPCUserColonPass.empty()) // Belt-and-suspenders measure if InitRPCAuthentication was not called - return false; if (strAuth.substr(0, 6) != "Basic ") return false; std::string_view strUserPass64 = TrimStringView(std::string_view{strAuth}.substr(6)); @@ -147,8 +145,9 @@ static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUserna if (strUserPass.find(':') != std::string::npos) strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':')); - //Check if authorized under single-user field - if (TimingResistantEqual(strUserPass, strRPCUserColonPass)) { + // Check if authorized under single-user field. + // (strRPCUserColonPass is empty when -norpccookiefile is specified). + if (!strRPCUserColonPass.empty() && TimingResistantEqual(strUserPass, strRPCUserColonPass)) { return true; } return multiUserAuthorized(strUserPass); @@ -294,22 +293,26 @@ static bool InitRPCAuthentication() { if (gArgs.GetArg("-rpcpassword", "") == "") { - LogInfo("Using random cookie authentication.\n"); - std::optional cookie_perms{std::nullopt}; auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")}; if (cookie_perms_arg) { auto perm_opt = InterpretPermString(*cookie_perms_arg); if (!perm_opt) { - LogInfo("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.\n", *cookie_perms_arg); + LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg); return false; } cookie_perms = *perm_opt; } + assert(strRPCUserColonPass.empty()); // Only support initializing once if (!GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)) { return false; } + if (strRPCUserColonPass.empty()) { + LogInfo("RPC authentication cookie file generation is disabled."); + } else { + LogInfo("Using random cookie authentication."); + } } else { LogPrintf("Config options rpcuser and rpcpassword will soon be deprecated. Locally-run instances may remove rpcuser to use cookie-based auth, or may be replaced with rpcauth. Please see share/rpcauth for rpcauth auth generation.\n"); strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", ""); diff --git a/src/init.cpp b/src/init.cpp index f79ebe881dc..19efc69aaa2 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -175,6 +175,8 @@ static fs::path GetPidFile(const ArgsManager& args) [[nodiscard]] static bool CreatePidFile(const ArgsManager& args) { + if (args.IsArgNegated("-pid")) return true; + std::ofstream file{GetPidFile(args)}; if (file) { #ifdef WIN32 @@ -483,7 +485,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-blocksonly", strprintf("Whether to reject transactions from network peers. Disables automatic broadcast and rebroadcast of transactions, unless the source peer has the 'forcerelay' permission. RPC transactions are not affected. (default: %u)", DEFAULT_BLOCKSONLY), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-coinstatsindex", strprintf("Maintain coinstats index used by the gettxoutsetinfo RPC (default: %u)", DEFAULT_COINSTATSINDEX), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-conf=", strprintf("Specify path to read-only configuration file. Relative paths will be prefixed by datadir location (only useable from command line, not configuration file) (default: %s)", BITCOIN_CONF_FILENAME), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-datadir=", "Specify data directory", ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-dbbatchsize", strprintf("Maximum database write batch size in bytes (default: %u)", nDefaultDbBatchSize), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS); argsman.AddArg("-dbcache=", strprintf("Maximum database cache size MiB (minimum %d, default: %d). Make sure you have enough RAM. In addition, unused memory allocated to the mempool is shared with this cache (see -maxmempool).", nMinDbCache, nDefaultDbCache), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-includeconf=", "Specify additional configuration file, relative to the -datadir path (only useable from configuration file, not command line)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); diff --git a/src/init/common.cpp b/src/init/common.cpp index 8d0b008fb52..3a8d75a6269 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -122,10 +123,13 @@ bool StartLogging(const ArgsManager& args) // Only log conf file usage message if conf file actually exists. fs::path config_file_path = args.GetConfigFilePath(); - if (fs::exists(config_file_path)) { + if (args.IsArgNegated("-conf")) { + LogInfo("Config file: "); + } else if (fs::is_directory(config_file_path)) { + LogWarning("Config file: %s (is directory, not file)", fs::PathToString(config_file_path)); + } else if (fs::exists(config_file_path)) { LogPrintf("Config file: %s\n", fs::PathToString(config_file_path)); } else if (args.IsArgSet("-conf")) { - // Warn if no conf file exists at path provided by user InitWarning(strprintf(_("The specified config file %s does not exist"), fs::PathToString(config_file_path))); } else { // Not categorizing as "Warning" because it's the default behavior diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp index 5c1ae895b1d..59eeeb02cd0 100644 --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -582,8 +582,8 @@ int GuiMain(int argc, char* argv[]) // Show help message immediately after parsing command-line options (for "-lang") and setting locale, // but before showing splash screen. - if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { - HelpMessageDialog help(nullptr, gArgs.IsArgSet("-version")); + if (HelpRequested(gArgs) || gArgs.GetBoolArg("-version", false)) { + HelpMessageDialog help(nullptr, gArgs.GetBoolArg("-version", false)); help.showOrPrint(); return EXIT_SUCCESS; } diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index afd98f88754..c0ae94bc4f2 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -86,6 +86,9 @@ static const char* const COOKIEAUTH_FILE = ".cookie"; static fs::path GetAuthCookieFile(bool temp=false) { fs::path arg = gArgs.GetPathArg("-rpccookiefile", COOKIEAUTH_FILE); + if (arg.empty()) { + return {}; // -norpccookiefile was specified + } if (temp) { arg += ".tmp"; } @@ -106,9 +109,12 @@ bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie */ std::ofstream file; fs::path filepath_tmp = GetAuthCookieFile(true); + if (filepath_tmp.empty()) { + return true; // -norpccookiefile + } file.open(filepath_tmp); if (!file.is_open()) { - LogInfo("Unable to open cookie authentication file %s for writing\n", fs::PathToString(filepath_tmp)); + LogWarning("Unable to open cookie authentication file %s for writing", fs::PathToString(filepath_tmp)); return false; } file << cookie; @@ -116,14 +122,14 @@ bool GenerateAuthCookie(std::string* cookie_out, std::optional cookie fs::path filepath = GetAuthCookieFile(false); if (!RenameOver(filepath_tmp, filepath)) { - LogInfo("Unable to rename cookie authentication file %s to %s\n", fs::PathToString(filepath_tmp), fs::PathToString(filepath)); + LogWarning("Unable to rename cookie authentication file %s to %s", fs::PathToString(filepath_tmp), fs::PathToString(filepath)); return false; } if (cookie_perms) { std::error_code code; fs::permissions(filepath, cookie_perms.value(), fs::perm_options::replace, code); if (code) { - LogInfo("Unable to set permissions on cookie authentication file %s\n", fs::PathToString(filepath_tmp)); + LogWarning("Unable to set permissions on cookie authentication file %s", fs::PathToString(filepath)); return false; } } @@ -142,6 +148,9 @@ bool GetAuthCookie(std::string *cookie_out) std::ifstream file; std::string cookie; fs::path filepath = GetAuthCookieFile(); + if (filepath.empty()) { + return true; // -norpccookiefile + } file.open(filepath); if (!file.is_open()) return false; diff --git a/test/functional/combine_logs.py b/test/functional/combine_logs.py index 33c81bde131..57fd0710b69 100755 --- a/test/functional/combine_logs.py +++ b/test/functional/combine_logs.py @@ -79,11 +79,12 @@ def read_logs(tmp_dir): Delegates to generator function get_log_events() to provide individual log events for each of the input log files.""" - # Find out what the folder is called that holds the debug.log file - glob = pathlib.Path(tmp_dir).glob('node0/**/debug.log') - path = next(glob, None) - if path: - assert next(glob, None) is None # more than one debug.log, should never happen + # Find out what the folder is called that holds node 0's debug.log file + debug_logs = list(pathlib.Path(tmp_dir).glob('node0/**/debug.log')) + if len(debug_logs) > 0: + assert len(debug_logs) < 2, 'Max one debug.log is supported, ' \ + 'found several:\n\t' + '\n\t'.join([str(f) for f in debug_logs]) + path = debug_logs[0] chain = re.search(r'node0/(.+?)/debug\.log$', path.as_posix()).group(1) # extract the chain name else: chain = 'regtest' # fallback to regtest (should only happen when none exists) diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 44c7edf9626..1852fd28215 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -27,9 +27,74 @@ class ConfArgsTest(BitcoinTestFramework): self.wallet_names = [] self.disable_autoconnect = False + # Overridden to avoid attempt to sync not yet started nodes. + def setup_network(self): + self.setup_nodes() + + # Overriden to not start nodes automatically - doing so is the + # responsibility of each test function. + def setup_nodes(self): + self.add_nodes(self.num_nodes, self.extra_args) + # Ensure a log file exists as TestNode.assert_debug_log() expects it. + self.nodes[0].debug_log_path.parent.mkdir() + self.nodes[0].debug_log_path.touch() + + def test_dir_config(self): + self.log.info('Error should be emitted if config file is a directory') + conf_path = self.nodes[0].datadir_path / 'bitcoin.conf' + os.rename(conf_path, conf_path.with_suffix('.confbkp')) + conf_path.mkdir() + self.stop_node(0) + self.nodes[0].assert_start_raises_init_error( + extra_args=['-regtest'], + expected_msg=f'Error: Error reading configuration file: Config file "{conf_path}" is a directory.', + ) + conf_path.rmdir() + os.rename(conf_path.with_suffix('.confbkp'), conf_path) + + self.log.debug('Verifying includeconf directive pointing to directory is caught') + with open(conf_path, 'a', encoding='utf-8') as conf: + conf.write(f'includeconf={self.nodes[0].datadir_path}\n') + self.nodes[0].assert_start_raises_init_error( + extra_args=['-regtest'], + expected_msg=f'Error: Error reading configuration file: Included config file "{self.nodes[0].datadir_path}" is a directory.', + ) + + self.nodes[0].replace_in_config([(f'includeconf={self.nodes[0].datadir_path}', '')]) + + def test_negated_config(self): + self.log.info('Disabling configuration via -noconf') + + conf_path = self.nodes[0].datadir_path / 'bitcoin.conf' + with open(conf_path, encoding='utf-8') as conf: + settings = [f'-{line.rstrip()}' for line in conf if len(line) > 1 and line[0] != '['] + os.rename(conf_path, conf_path.with_suffix('.confbkp')) + + self.log.debug('Verifying garbage in config can be detected') + with open(conf_path, 'a', encoding='utf-8') as conf: + conf.write(f'garbage\n') + self.nodes[0].assert_start_raises_init_error( + extra_args=['-regtest'], + expected_msg='Error: Error reading configuration file: parse error on line 1: garbage', + ) + + self.log.debug('Verifying that disabling of the config file means garbage inside of it does ' \ + 'not prevent the node from starting, and message about existing config file is logged') + ignored_file_message = [f'[InitConfig] Data directory "{self.nodes[0].datadir_path}" contains a "bitcoin.conf" file which is explicitly ignored using -noconf.'] + with self.nodes[0].assert_debug_log(timeout=60, expected_msgs=ignored_file_message): + self.start_node(0, extra_args=settings + ['-noconf']) + self.stop_node(0) + + self.log.debug('Verifying no message appears when removing config file') + os.remove(conf_path) + with self.nodes[0].assert_debug_log(timeout=60, expected_msgs=[], unexpected_msgs=ignored_file_message): + self.start_node(0, extra_args=settings + ['-noconf']) + self.stop_node(0) + + os.rename(conf_path.with_suffix('.confbkp'), conf_path) + def test_config_file_parser(self): self.log.info('Test config file parser') - self.stop_node(0) # Check that startup fails if conf= is set in bitcoin.conf or in an included conf file bad_conf_file_path = self.nodes[0].datadir_path / "bitcoin_bad.conf" @@ -162,12 +227,11 @@ class ConfArgsTest(BitcoinTestFramework): ) def test_log_buffer(self): - self.stop_node(0) with self.nodes[0].assert_debug_log(expected_msgs=['Warning: parsed potentially confusing double-negative -connect=0\n']): self.start_node(0, extra_args=['-noconnect=0']) + self.stop_node(0) def test_args_log(self): - self.stop_node(0) self.log.info('Test config args logging') with self.nodes[0].assert_debug_log( expected_msgs=[ @@ -196,10 +260,10 @@ class ConfArgsTest(BitcoinTestFramework): '-rpcuser=secret-rpcuser', '-torpassword=secret-torpassword', ]) + self.stop_node(0) def test_networkactive(self): self.log.info('Test -networkactive option') - self.stop_node(0) with self.nodes[0].assert_debug_log(expected_msgs=['SetNetworkActive: true\n']): self.start_node(0) @@ -222,16 +286,12 @@ class ConfArgsTest(BitcoinTestFramework): self.stop_node(0) with self.nodes[0].assert_debug_log(expected_msgs=['SetNetworkActive: false\n']): self.start_node(0, extra_args=['-nonetworkactive=1']) + self.stop_node(0) def test_seed_peers(self): self.log.info('Test seed peers') default_data_dir = self.nodes[0].datadir_path peer_dat = default_data_dir / 'peers.dat' - # Only regtest has no fixed seeds. To avoid connections to random - # nodes, regtest is the only network where it is safe to enable - # -fixedseeds in tests - util.assert_equal(self.nodes[0].getblockchaininfo()['chain'],'regtest') - self.stop_node(0) # No peers.dat exists and -dnsseed=1 # We expect the node will use DNS Seeds, but Regtest mode does not have @@ -248,6 +308,12 @@ class ConfArgsTest(BitcoinTestFramework): timeout=10, ): self.start_node(0, extra_args=['-dnsseed=1', '-fixedseeds=1', f'-mocktime={start}']) + + # Only regtest has no fixed seeds. To avoid connections to random + # nodes, regtest is the only network where it is safe to enable + # -fixedseeds in tests + util.assert_equal(self.nodes[0].getblockchaininfo()['chain'],'regtest') + with self.nodes[0].assert_debug_log(expected_msgs=[ "Adding fixed seeds as 60 seconds have passed and addrman is empty", ]): @@ -294,13 +360,13 @@ class ConfArgsTest(BitcoinTestFramework): "Adding fixed seeds as 60 seconds have passed and addrman is empty", ]): self.nodes[0].setmocktime(start + 65) + self.stop_node(0) def test_connect_with_seednode(self): self.log.info('Test -connect with -seednode') seednode_ignored = ['-seednode is ignored when -connect is used\n'] dnsseed_ignored = ['-dnsseed is ignored when -connect is used and -proxy is specified\n'] addcon_thread_started = ['addcon thread start\n'] - self.stop_node(0) # When -connect is supplied, expanding addrman via getaddr calls to ADDR_FETCH(-seednode) # nodes is irrelevant and -seednode is ignored. @@ -325,6 +391,7 @@ class ConfArgsTest(BitcoinTestFramework): with self.nodes[0].assert_debug_log(expected_msgs=addcon_thread_started, unexpected_msgs=seednode_ignored): self.restart_node(0, extra_args=[connect_arg, '-seednode=fakeaddress2']) + self.stop_node(0) def test_ignored_conf(self): self.log.info('Test error is triggered when the datadir in use contains a bitcoin.conf file that would be ignored ' @@ -423,6 +490,8 @@ class ConfArgsTest(BitcoinTestFramework): self.test_networkactive() self.test_connect_with_seednode() + self.test_dir_config() + self.test_negated_config() self.test_config_file_parser() self.test_config_file_log() self.test_invalid_command_line_options() diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 3fe6570dd16..134b4e566be 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -164,6 +164,9 @@ class TestBitcoinCli(BitcoinTestFramework): self.log.info("Test connecting with non-existing RPC cookie file") assert_raises_process_error(1, "Could not locate RPC credentials", self.nodes[0].cli('-rpccookiefile=does-not-exist', '-rpcpassword=').echo) + self.log.info("Test connecting without RPC cookie file and with password arg") + assert_equal(BLOCKS, self.nodes[0].cli('-norpccookiefile', f'-rpcuser={user}', f'-rpcpassword={password}').getblockcount()) + self.log.info("Test -getinfo with arguments fails") assert_raises_process_error(1, "-getinfo takes no arguments", self.nodes[0].cli('-getinfo').help) diff --git a/test/functional/rpc_users.py b/test/functional/rpc_users.py index 49eb64abada..75507877d54 100755 --- a/test/functional/rpc_users.py +++ b/test/functional/rpc_users.py @@ -22,13 +22,13 @@ import sys from typing import Optional -def call_with_auth(node, user, password): +def call_with_auth(node, user, password, method="getbestblockhash"): url = urllib.parse.urlparse(node.url) headers = {"Authorization": "Basic " + str_to_b64str('{}:{}'.format(user, password))} conn = http.client.HTTPConnection(url.hostname, url.port) conn.connect() - conn.request('POST', '/', '{"method": "getbestblockhash"}', headers) + conn.request('POST', '/', f'{{"method": "{method}"}}', headers) resp = conn.getresponse() conn.close() return resp @@ -121,6 +121,25 @@ class HTTPBasicsTest(BitcoinTestFramework): for perm in ["owner", "group", "all"]: test_perm(perm) + def test_norpccookiefile(self, node0_cookie_path): + assert self.nodes[0].is_node_stopped(), "We expect previous test to stopped the node" + assert not node0_cookie_path.exists() + + self.log.info('Starting with -norpccookiefile') + # Start, but don't wait for RPC connection as TestNode.wait_for_rpc_connection() requires the cookie. + with self.nodes[0].busy_wait_for_debug_log([b'init message: Done loading']): + self.nodes[0].start(extra_args=["-norpccookiefile"]) + assert not node0_cookie_path.exists() + + self.log.info('Testing user/password authentication still works without cookie file') + assert_equal(200, call_with_auth(self.nodes[0], "rt", self.rtpassword).status) + # After confirming that we could log in, check that cookie file does not exist. + assert not node0_cookie_path.exists() + + # Need to shut down in slightly unorthodox way since cookie auth can't be used + assert_equal(200, call_with_auth(self.nodes[0], "rt", self.rtpassword, method="stop").status) + self.nodes[0].wait_until_stopped() + def run_test(self): self.conf_setup() self.log.info('Check correctness of the rpcauth config option') @@ -166,11 +185,19 @@ class HTTPBasicsTest(BitcoinTestFramework): self.stop_node(0) self.log.info('Check that failure to write cookie file will abort the node gracefully') - (self.nodes[0].chain_path / ".cookie.tmp").mkdir() + cookie_path = self.nodes[0].chain_path / ".cookie" + cookie_path_tmp = self.nodes[0].chain_path / ".cookie.tmp" + cookie_path_tmp.mkdir() self.nodes[0].assert_start_raises_init_error(expected_msg=init_error) + cookie_path_tmp.rmdir() + assert not cookie_path.exists() + self.restart_node(0) + assert cookie_path.exists() + self.stop_node(0) self.test_rpccookieperms() + self.test_norpccookiefile(cookie_path) if __name__ == '__main__': HTTPBasicsTest(__file__).main()