From 483f0dacc413f4b1ba1b74c2429c4367b87e7f11 Mon Sep 17 00:00:00 2001 From: Hodlinator <172445034+hodlinator@users.noreply.github.com> Date: Tue, 3 Dec 2024 09:45:47 +0100 Subject: [PATCH] args: Properly support -noconf -noconf would previously lead to an ifstream "successfully" being opened to the ".bitcoin"-directory (not a file). (Guards against the general case of directories as configs are added in grandchild commit to this one). Other users of AbsPathForConfigVal() in combination with negated args have been updated earlier in this PR ("args: Support -nopid" and "args: Support -norpccookiefile..."). --- src/common/config.cpp | 16 +++++++------- src/common/init.cpp | 49 ++++++++++++++++++++++++------------------- src/init/common.cpp | 6 ++++-- 3 files changed, 41 insertions(+), 30 deletions(-) diff --git a/src/common/config.cpp b/src/common/config.cpp index 98223fc3e3a..13db98171ae 100644 --- a/src/common/config.cpp +++ b/src/common/config.cpp @@ -128,12 +128,14 @@ 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 + 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()) { @@ -213,7 +215,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/init/common.cpp b/src/init/common.cpp index 8d0b008fb52..8f7a86b6886 100644 --- a/src/init/common.cpp +++ b/src/init/common.cpp @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -122,10 +123,11 @@ 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::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