Skip to content

Commit 07f39b7

Browse files
luke-jrluke-jr
committed
Merge branch 'improve_args_pr31212-28' into rpccookieperms_octal_compat-28+knots
2 parents 1248d0d + c58ca37 commit 07f39b7

16 files changed

+210
-73
lines changed

‎src/bitcoin-cli.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ static void SetupCliArgs(ArgsManager& argsman)
9393
argsman.AddArg("-netinfo", "Get network peer connection information from the remote server. An optional integer argument from 0 to 4 can be passed for different peers listings (default: 0). Pass \"help\" for detailed help documentation.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
9494

9595
SetupChainParamsBaseOptions(argsman);
96-
argsman.AddArg("-color=<when>", 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);
96+
argsman.AddArg("-color=<when>", 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);
9797
argsman.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
9898
argsman.AddArg("-rpcclienttimeout=<n>", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
9999
argsman.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
@@ -144,10 +144,10 @@ static int AppInitRPC(int argc, char* argv[])
144144
tfm::format(std::cerr, "Error parsing command line arguments: %s\n", error);
145145
return EXIT_FAILURE;
146146
}
147-
if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
147+
if (argc < 2 || HelpRequested(gArgs) || gArgs.GetBoolArg("-version", false)) {
148148
std::string strUsage = PACKAGE_NAME " RPC client version " + FormatFullVersion() + "\n";
149149

150-
if (gArgs.IsArgSet("-version")) {
150+
if (gArgs.GetBoolArg("-version", false)) {
151151
strUsage += FormatParagraph(LicenseInfo());
152152
} else {
153153
strUsage += "\n"

‎src/bitcoin-tx.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,11 +105,11 @@ static int AppInitRawTx(int argc, char* argv[])
105105

106106
fCreateBlank = gArgs.GetBoolArg("-create", false);
107107

108-
if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
108+
if (argc < 2 || HelpRequested(gArgs) || gArgs.GetBoolArg("-version", false)) {
109109
// First part of help message is specific to this utility
110110
std::string strUsage = PACKAGE_NAME " bitcoin-tx utility version " + FormatFullVersion() + "\n";
111111

112-
if (gArgs.IsArgSet("-version")) {
112+
if (gArgs.GetBoolArg("-version", false)) {
113113
strUsage += FormatParagraph(LicenseInfo());
114114
} else {
115115
strUsage += "\n"

‎src/bitcoin-util.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,11 @@ static int AppInitUtil(ArgsManager& args, int argc, char* argv[])
5050
return EXIT_FAILURE;
5151
}
5252

53-
if (HelpRequested(args) || args.IsArgSet("-version")) {
53+
if (HelpRequested(args) || args.GetBoolArg("-version", false)) {
5454
// First part of help message is specific to this utility
5555
std::string strUsage = PACKAGE_NAME " bitcoin-util utility version " + FormatFullVersion() + "\n";
5656

57-
if (args.IsArgSet("-version")) {
57+
if (args.GetBoolArg("-version", false)) {
5858
strUsage += FormatParagraph(LicenseInfo());
5959
} else {
6060
strUsage += "\n"

‎src/bitcoin-wallet.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,10 +60,10 @@ static std::optional<int> WalletAppInit(ArgsManager& args, int argc, char* argv[
6060
return EXIT_FAILURE;
6161
}
6262
const bool missing_args{argc < 2};
63-
if (missing_args || HelpRequested(args) || args.IsArgSet("-version")) {
63+
if (missing_args || HelpRequested(args) || args.GetBoolArg("-version", false)) {
6464
std::string strUsage = strprintf("%s bitcoin-wallet version", PACKAGE_NAME) + " " + FormatFullVersion() + "\n";
6565

66-
if (args.IsArgSet("-version")) {
66+
if (args.GetBoolArg("-version", false)) {
6767
strUsage += FormatParagraph(LicenseInfo());
6868
} else {
6969
strUsage += "\n"

‎src/bitcoind.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ static bool ParseArgs(ArgsManager& args, int argc, char* argv[])
134134
static bool ProcessInitCommands(ArgsManager& args)
135135
{
136136
// Process help and version before taking care about datadir
137-
if (HelpRequested(args) || args.IsArgSet("-version")) {
137+
if (HelpRequested(args) || args.GetBoolArg("-version", false)) {
138138
std::string strUsage = PACKAGE_NAME " version " + FormatFullVersion() + "\n";
139139

140-
if (args.IsArgSet("-version")) {
140+
if (args.GetBoolArg("-version", false)) {
141141
strUsage += FormatParagraph(LicenseInfo());
142142
} else {
143143
strUsage += "\nUsage: bitcoind [options] Start " PACKAGE_NAME "\n"

‎src/common/config.cpp

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <algorithm>
1717
#include <cassert>
1818
#include <cstdlib>
19+
#include <filesystem>
1920
#include <fstream>
2021
#include <iostream>
2122
#include <list>
@@ -128,12 +129,18 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
128129
}
129130

130131
const auto conf_path{GetConfigFilePath()};
131-
std::ifstream stream{conf_path};
132-
133-
// not ok to have a config file specified that cannot be opened
134-
if (IsArgSet("-conf") && !stream.good()) {
135-
error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path));
136-
return false;
132+
std::ifstream stream;
133+
if (!conf_path.empty()) { // path is empty when -noconf is specified
134+
if (fs::is_directory(conf_path)) {
135+
error = strprintf("Config file \"%s\" is a directory.", fs::PathToString(conf_path));
136+
return false;
137+
}
138+
stream = std::ifstream{conf_path};
139+
// If the file is explicitly specified, it must be readable
140+
if (IsArgSet("-conf") && !stream.good()) {
141+
error = strprintf("specified config file \"%s\" could not be opened.", fs::PathToString(conf_path));
142+
return false;
143+
}
137144
}
138145
// ok to not have a config file
139146
if (stream.good()) {
@@ -175,7 +182,12 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
175182
const size_t default_includes = add_includes({});
176183

177184
for (const std::string& conf_file_name : conf_file_names) {
178-
std::ifstream conf_file_stream{AbsPathForConfigVal(*this, fs::PathFromString(conf_file_name), /*net_specific=*/false)};
185+
const auto include_conf_path{AbsPathForConfigVal(*this, fs::PathFromString(conf_file_name), /*net_specific=*/false)};
186+
if (fs::is_directory(include_conf_path)) {
187+
error = strprintf("Included config file \"%s\" is a directory.", fs::PathToString(include_conf_path));
188+
return false;
189+
}
190+
std::ifstream conf_file_stream{include_conf_path};
179191
if (conf_file_stream.good()) {
180192
if (!ReadConfigStream(conf_file_stream, conf_file_name, error, ignore_invalid_keys)) {
181193
return false;
@@ -213,7 +225,7 @@ bool ArgsManager::ReadConfigFiles(std::string& error, bool ignore_invalid_keys)
213225

214226
fs::path AbsPathForConfigVal(const ArgsManager& args, const fs::path& path, bool net_specific)
215227
{
216-
if (path.is_absolute()) {
228+
if (path.is_absolute() || path.empty()) {
217229
return path;
218230
}
219231
return fsbridge::AbsPathJoin(net_specific ? args.GetDataDirNet() : args.GetDataDirBase(), path);

‎src/common/init.cpp

Lines changed: 28 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,29 +62,36 @@ std::optional<ConfigError> InitConfig(ArgsManager& args, SettingsAbortFn setting
6262
fs::create_directories(net_path / "wallets");
6363
}
6464

65-
// Show an error or warning if there is a bitcoin.conf file in the
65+
// Show an error or warn/log if there is a bitcoin.conf file in the
6666
// datadir that is being ignored.
6767
const fs::path base_config_path = base_path / BITCOIN_CONF_FILENAME;
68-
if (fs::exists(base_config_path) && !fs::equivalent(orig_config_path, base_config_path)) {
69-
const std::string cli_config_path = args.GetArg("-conf", "");
70-
const std::string config_source = cli_config_path.empty()
71-
? strprintf("data directory %s", fs::quoted(fs::PathToString(orig_datadir_path)))
72-
: strprintf("command line argument %s", fs::quoted("-conf=" + cli_config_path));
73-
const std::string error = strprintf(
74-
"Data directory %1$s contains a %2$s file which is ignored, because a different configuration file "
75-
"%3$s from %4$s is being used instead. Possible ways to address this would be to:\n"
76-
"- Delete or rename the %2$s file in data directory %1$s.\n"
77-
"- Change datadir= or conf= options to specify one configuration file, not two, and use "
78-
"includeconf= to include any other configuration files.\n"
79-
"- Set allowignoredconf=1 option to treat this condition as a warning, not an error.",
80-
fs::quoted(fs::PathToString(base_path)),
81-
fs::quoted(BITCOIN_CONF_FILENAME),
82-
fs::quoted(fs::PathToString(orig_config_path)),
83-
config_source);
84-
if (args.GetBoolArg("-allowignoredconf", false)) {
85-
LogPrintf("Warning: %s\n", error);
86-
} else {
87-
return ConfigError{ConfigStatus::FAILED, Untranslated(error)};
68+
if (fs::exists(base_config_path)) {
69+
if (orig_config_path.empty()) {
70+
LogInfo(
71+
"Data directory %s contains a %s file which is explicitly ignored using -noconf.",
72+
fs::quoted(fs::PathToString(base_path)),
73+
fs::quoted(BITCOIN_CONF_FILENAME));
74+
} else if (!fs::equivalent(orig_config_path, base_config_path)) {
75+
const std::string cli_config_path = args.GetArg("-conf", "");
76+
const std::string config_source = cli_config_path.empty()
77+
? strprintf("data directory %s", fs::quoted(fs::PathToString(orig_datadir_path)))
78+
: strprintf("command line argument %s", fs::quoted("-conf=" + cli_config_path));
79+
std::string error = strprintf(
80+
"Data directory %1$s contains a %2$s file which is ignored, because a different configuration file "
81+
"%3$s from %4$s is being used instead. Possible ways to address this would be to:\n"
82+
"- Delete or rename the %2$s file in data directory %1$s.\n"
83+
"- Change datadir= or conf= options to specify one configuration file, not two, and use "
84+
"includeconf= to include any other configuration files.",
85+
fs::quoted(fs::PathToString(base_path)),
86+
fs::quoted(BITCOIN_CONF_FILENAME),
87+
fs::quoted(fs::PathToString(orig_config_path)),
88+
config_source);
89+
if (args.GetBoolArg("-allowignoredconf", false)) {
90+
LogWarning("%s", error);
91+
} else {
92+
error += "\n- Set allowignoredconf=1 option to treat this condition as a warning, not an error.";
93+
return ConfigError{ConfigStatus::FAILED, Untranslated(error)};
94+
}
8895
}
8996
}
9097

‎src/httprpc.cpp

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ static bool multiUserAuthorized(std::string strUserPass)
134134

135135
static bool RPCAuthorized(const std::string& strAuth, std::string& strAuthUsernameOut)
136136
{
137-
if (strRPCUserColonPass.empty()) // Belt-and-suspenders measure if InitRPCAuthentication was not called
138-
return false;
139137
if (strAuth.substr(0, 6) != "Basic ")
140138
return false;
141139
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
147145
if (strUserPass.find(':') != std::string::npos)
148146
strAuthUsernameOut = strUserPass.substr(0, strUserPass.find(':'));
149147

150-
//Check if authorized under single-user field
151-
if (TimingResistantEqual(strUserPass, strRPCUserColonPass)) {
148+
// Check if authorized under single-user field.
149+
// (strRPCUserColonPass is empty when -norpccookiefile is specified).
150+
if (!strRPCUserColonPass.empty() && TimingResistantEqual(strUserPass, strRPCUserColonPass)) {
152151
return true;
153152
}
154153
return multiUserAuthorized(strUserPass);
@@ -294,22 +293,26 @@ static bool InitRPCAuthentication()
294293
{
295294
if (gArgs.GetArg("-rpcpassword", "") == "")
296295
{
297-
LogInfo("Using random cookie authentication.\n");
298-
299296
std::optional<fs::perms> cookie_perms{std::nullopt};
300297
auto cookie_perms_arg{gArgs.GetArg("-rpccookieperms")};
301298
if (cookie_perms_arg) {
302299
auto perm_opt = InterpretPermString(*cookie_perms_arg);
303300
if (!perm_opt) {
304-
LogInfo("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.\n", *cookie_perms_arg);
301+
LogError("Invalid -rpccookieperms=%s; must be one of 'owner', 'group', or 'all'.", *cookie_perms_arg);
305302
return false;
306303
}
307304
cookie_perms = *perm_opt;
308305
}
309306

307+
assert(strRPCUserColonPass.empty()); // Only support initializing once
310308
if (!GenerateAuthCookie(&strRPCUserColonPass, cookie_perms)) {
311309
return false;
312310
}
311+
if (strRPCUserColonPass.empty()) {
312+
LogInfo("RPC authentication cookie file generation is disabled.");
313+
} else {
314+
LogInfo("Using random cookie authentication.");
315+
}
313316
} else {
314317
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");
315318
strRPCUserColonPass = gArgs.GetArg("-rpcuser", "") + ":" + gArgs.GetArg("-rpcpassword", "");

‎src/init.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,8 @@ static fs::path GetPidFile(const ArgsManager& args)
171171

172172
[[nodiscard]] static bool CreatePidFile(const ArgsManager& args)
173173
{
174+
if (args.IsArgNegated("-pid")) return true;
175+
174176
std::ofstream file{GetPidFile(args)};
175177
if (file) {
176178
#ifdef WIN32

‎src/init/common.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <util/translation.h>
1818

1919
#include <algorithm>
20+
#include <filesystem>
2021
#include <string>
2122
#include <vector>
2223

@@ -122,10 +123,13 @@ bool StartLogging(const ArgsManager& args)
122123

123124
// Only log conf file usage message if conf file actually exists.
124125
fs::path config_file_path = args.GetConfigFilePath();
125-
if (fs::exists(config_file_path)) {
126+
if (args.IsArgNegated("-conf")) {
127+
LogInfo("Config file: <disabled>");
128+
} else if (fs::is_directory(config_file_path)) {
129+
LogWarning("Config file: %s (is directory, not file)", fs::PathToString(config_file_path));
130+
} else if (fs::exists(config_file_path)) {
126131
LogPrintf("Config file: %s\n", fs::PathToString(config_file_path));
127132
} else if (args.IsArgSet("-conf")) {
128-
// Warn if no conf file exists at path provided by user
129133
InitWarning(strprintf(_("The specified config file %s does not exist"), fs::PathToString(config_file_path)));
130134
} else {
131135
// Not categorizing as "Warning" because it's the default behavior

0 commit comments

Comments
 (0)