Skip to content

Commit 61fcef0

Browse files
author
MarcoFalke
committed
Merge #13112: Throw an error for unknown args
9030557 Test gArgs erroring on unknown args (Andrew Chow) 4f8704d Give an error and exit if there are unknown parameters (Andrew Chow) 174f7c8 Use a struct for arguments and nested map for categories (Andrew Chow) Pull request description: Following #13190, gArgs is aware of all of the command line arguments. This PR has gArgs check whether the arguments provided are actually valid arguments. When an unknown argument is encountered, an error is printed to stderr and the program exist. Since gArgs is used for everything that has command line arguments, `bitcoind`, `bitcoin-cli`, `bitcoin-qt`, `bitcoin-tx`, and `bench_bitcoin` are all effected by this change and all now have the same argument checking behavior. Closes #1044 Tree-SHA512: 388201319a7d6493204bb5433da47e8e6c8266882e809f6df45f86d925f1f320f2fd13edb3e57ffc6a37415dfdfc689f83929452bca224229783accb367032e7
2 parents c4cc8d9 + 9030557 commit 61fcef0

File tree

13 files changed

+282
-91
lines changed

13 files changed

+282
-91
lines changed

‎src/bench/bench_bitcoin.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,21 @@ static void SetupBenchArgs()
3333
gArgs.AddArg("-plot-plotlyurl=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), false, OptionsCategory::OPTIONS);
3434
gArgs.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), false, OptionsCategory::OPTIONS);
3535
gArgs.AddArg("-plot-height=<x>", strprintf("Plot height in pixel (default: %u)", DEFAULT_PLOT_HEIGHT), false, OptionsCategory::OPTIONS);
36+
37+
// Hidden
38+
gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
39+
gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
3640
}
3741

3842
int
3943
main(int argc, char** argv)
4044
{
4145
SetupBenchArgs();
42-
gArgs.ParseParameters(argc, argv);
46+
std::string error;
47+
if (!gArgs.ParseParameters(argc, argv, error)) {
48+
fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
49+
return false;
50+
}
4351

4452
if (HelpRequested(gArgs)) {
4553
std::cout << gArgs.GetHelpMessage();

‎src/bitcoin-cli.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,18 @@ static void SetupCliArgs()
4242
gArgs.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), false, OptionsCategory::OPTIONS);
4343
gArgs.AddArg("-rpcclienttimeout=<n>", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), false, OptionsCategory::OPTIONS);
4444
gArgs.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), false, OptionsCategory::OPTIONS);
45+
gArgs.AddArg("-rpccookiefile=<loc>", _("Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)"), false, OptionsCategory::OPTIONS);
4546
gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", false, OptionsCategory::OPTIONS);
4647
gArgs.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u or testnet: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort()), false, OptionsCategory::OPTIONS);
4748
gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", false, OptionsCategory::OPTIONS);
4849
gArgs.AddArg("-rpcwait", "Wait for RPC server to start", false, OptionsCategory::OPTIONS);
4950
gArgs.AddArg("-rpcwallet=<walletname>", "Send RPC for non-default wallet on RPC server (needs to exactly match corresponding -wallet option passed to bitcoind)", false, OptionsCategory::OPTIONS);
5051
gArgs.AddArg("-stdin", "Read extra arguments from standard input, one per line until EOF/Ctrl-D (recommended for sensitive information such as passphrases). When combined with -stdinrpcpass, the first line from standard input is used for the RPC password.", false, OptionsCategory::OPTIONS);
5152
gArgs.AddArg("-stdinrpcpass", strprintf("Read RPC password from standard input as a single line. When combined with -stdin, the first line from standard input is used for the RPC password."), false, OptionsCategory::OPTIONS);
53+
54+
// Hidden
55+
gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
56+
gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
5257
}
5358

5459
//////////////////////////////////////////////////////////////////////////////
@@ -80,7 +85,11 @@ static int AppInitRPC(int argc, char* argv[])
8085
// Parameters
8186
//
8287
SetupCliArgs();
83-
gArgs.ParseParameters(argc, argv);
88+
std::string error;
89+
if (!gArgs.ParseParameters(argc, argv, error)) {
90+
fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
91+
return EXIT_FAILURE;
92+
}
8493
if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
8594
std::string strUsage = strprintf("%s RPC client version", PACKAGE_NAME) + " " + FormatFullVersion() + "\n";
8695
if (!gArgs.IsArgSet("-version")) {
@@ -104,10 +113,8 @@ static int AppInitRPC(int argc, char* argv[])
104113
fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
105114
return EXIT_FAILURE;
106115
}
107-
try {
108-
gArgs.ReadConfigFiles();
109-
} catch (const std::exception& e) {
110-
fprintf(stderr,"Error reading configuration file: %s\n", e.what());
116+
if (!gArgs.ReadConfigFiles(error, true)) {
117+
fprintf(stderr, "Error reading configuration file: %s\n", error.c_str());
111118
return EXIT_FAILURE;
112119
}
113120
// Check for -testnet or -regtest parameter (BaseParams() calls are only valid after this clause)

‎src/bitcoin-tx.cpp

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ static void SetupBitcoinTxArgs()
6464

6565
gArgs.AddArg("load=NAME:FILENAME", "Load JSON file FILENAME into register NAME", false, OptionsCategory::REGISTER_COMMANDS);
6666
gArgs.AddArg("set=NAME:JSON-STRING", "Set register NAME to given JSON-STRING", false, OptionsCategory::REGISTER_COMMANDS);
67+
68+
// Hidden
69+
gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
70+
gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
6771
}
6872

6973
//
@@ -76,7 +80,11 @@ static int AppInitRawTx(int argc, char* argv[])
7680
// Parameters
7781
//
7882
SetupBitcoinTxArgs();
79-
gArgs.ParseParameters(argc, argv);
83+
std::string error;
84+
if (!gArgs.ParseParameters(argc, argv, error)) {
85+
fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
86+
return EXIT_FAILURE;
87+
}
8088

8189
// Check for -testnet or -regtest parameter (Params() calls are only valid after this clause)
8290
try {

‎src/bitcoind.cpp

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,11 @@ static bool AppInit(int argc, char* argv[])
6565
#if HAVE_DECL_DAEMON
6666
gArgs.AddArg("-daemon", "Run in the background as a daemon and accept commands", false, OptionsCategory::OPTIONS);
6767
#endif
68-
gArgs.ParseParameters(argc, argv);
68+
std::string error;
69+
if (!gArgs.ParseParameters(argc, argv, error)) {
70+
fprintf(stderr, "Error parsing command line arguments: %s\n", error.c_str());
71+
return false;
72+
}
6973

7074
// Process help and version before taking care about datadir
7175
if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) {
@@ -94,11 +98,8 @@ static bool AppInit(int argc, char* argv[])
9498
fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
9599
return false;
96100
}
97-
try
98-
{
99-
gArgs.ReadConfigFiles();
100-
} catch (const std::exception& e) {
101-
fprintf(stderr,"Error reading configuration file: %s\n", e.what());
101+
if (!gArgs.ReadConfigFiles(error)) {
102+
fprintf(stderr, "Error reading configuration file: %s\n", error.c_str());
102103
return false;
103104
}
104105
// Check for -testnet or -regtest parameter (Params() calls are only valid after this clause)

‎src/init.cpp

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,23 @@ void SetupServerArgs()
499499
gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", false, OptionsCategory::RPC);
500500
gArgs.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), true, OptionsCategory::RPC);
501501
gArgs.AddArg("-server", "Accept command line and JSON-RPC commands", false, OptionsCategory::RPC);
502+
503+
// Hidden options
504+
gArgs.AddArg("-rpcssl", "", false, OptionsCategory::HIDDEN);
505+
gArgs.AddArg("-benchmark", "", false, OptionsCategory::HIDDEN);
506+
gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
507+
gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
508+
gArgs.AddArg("-socks", "", false, OptionsCategory::HIDDEN);
509+
gArgs.AddArg("-tor", "", false, OptionsCategory::HIDDEN);
510+
gArgs.AddArg("-debugnet", "", false, OptionsCategory::HIDDEN);
511+
gArgs.AddArg("-whitelistalwaysrelay", "", false, OptionsCategory::HIDDEN);
512+
gArgs.AddArg("-prematurewitness", "", false, OptionsCategory::HIDDEN);
513+
gArgs.AddArg("-walletprematurewitness", "", false, OptionsCategory::HIDDEN);
514+
gArgs.AddArg("-promiscuousmempoolflags", "", false, OptionsCategory::HIDDEN);
515+
gArgs.AddArg("-blockminsize", "", false, OptionsCategory::HIDDEN);
516+
gArgs.AddArg("-dbcrashratio", "", false, OptionsCategory::HIDDEN);
517+
gArgs.AddArg("-forcecompactdb", "", false, OptionsCategory::HIDDEN);
518+
gArgs.AddArg("-usehd", "", false, OptionsCategory::HIDDEN);
502519
}
503520

504521
std::string LicenseInfo()

‎src/interfaces/node.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ namespace {
4848

4949
class NodeImpl : public Node
5050
{
51-
void parseParameters(int argc, const char* const argv[]) override
51+
bool parseParameters(int argc, const char* const argv[], std::string& error) override
5252
{
53-
gArgs.ParseParameters(argc, argv);
53+
return gArgs.ParseParameters(argc, argv, error);
5454
}
55-
void readConfigFiles() override { gArgs.ReadConfigFiles(); }
55+
bool readConfigFiles(std::string& error) override { return gArgs.ReadConfigFiles(error); }
5656
bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
5757
bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
5858
void selectParams(const std::string& network) override { SelectParams(network); }

‎src/interfaces/node.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class Node
3838
virtual ~Node() {}
3939

4040
//! Set command line arguments.
41-
virtual void parseParameters(int argc, const char* const argv[]) = 0;
41+
virtual bool parseParameters(int argc, const char* const argv[], std::string& error) = 0;
4242

4343
//! Set a command line argument if it doesn't already have a value
4444
virtual bool softSetArg(const std::string& arg, const std::string& value) = 0;
@@ -47,7 +47,7 @@ class Node
4747
virtual bool softSetBoolArg(const std::string& arg, bool value) = 0;
4848

4949
//! Load settings from configuration file.
50-
virtual void readConfigFiles() = 0;
50+
virtual bool readConfigFiles(std::string& error) = 0;
5151

5252
//! Choose network parameters.
5353
virtual void selectParams(const std::string& network) = 0;

‎src/qt/bitcoin.cpp

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,9 @@ class BitcoinApplication: public QApplication
229229
/// Get window identifier of QMainWindow (BitcoinGUI)
230230
WId getMainWinId() const;
231231

232+
/// Setup platform style
233+
void setupPlatformStyle();
234+
232235
public Q_SLOTS:
233236
void initializeResult(bool success);
234237
void shutdownResult();
@@ -315,10 +318,14 @@ BitcoinApplication::BitcoinApplication(interfaces::Node& node, int &argc, char *
315318
paymentServer(0),
316319
m_wallet_models(),
317320
#endif
318-
returnValue(0)
321+
returnValue(0),
322+
platformStyle(0)
319323
{
320324
setQuitOnLastWindowClosed(false);
325+
}
321326

327+
void BitcoinApplication::setupPlatformStyle()
328+
{
322329
// UI per-platform customization
323330
// This must be done inside the BitcoinApplication constructor, or after it, because
324331
// PlatformStyle::instantiate requires a QApplication
@@ -562,15 +569,9 @@ int main(int argc, char *argv[])
562569

563570
std::unique_ptr<interfaces::Node> node = interfaces::MakeNode();
564571

565-
/// 1. Parse command-line options. These take precedence over anything else.
566-
// Command-line options take precedence:
567-
node->setupServerArgs();
568-
SetupUIArgs();
569-
node->parseParameters(argc, argv);
570-
571572
// Do not refer to data directory yet, this can be overridden by Intro::pickDataDirectory
572573

573-
/// 2. Basic Qt initialization (not dependent on parameters or configuration)
574+
/// 1. Basic Qt initialization (not dependent on parameters or configuration)
574575
#if QT_VERSION < 0x050000
575576
// Internal string conversion is all UTF-8
576577
QTextCodec::setCodecForTr(QTextCodec::codecForName("UTF-8"));
@@ -609,6 +610,20 @@ int main(int argc, char *argv[])
609610
qRegisterMetaType<WalletModel*>("WalletModel*");
610611
#endif
611612

613+
/// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
614+
// Command-line options take precedence:
615+
node->setupServerArgs();
616+
SetupUIArgs();
617+
std::string error;
618+
if (!node->parseParameters(argc, argv, error)) {
619+
QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
620+
QObject::tr("Error parsing command line arguments: %1.").arg(QString::fromStdString(error)));
621+
return EXIT_FAILURE;
622+
}
623+
624+
// Now that the QApplication is setup and we have parsed our parameters, we can set the platform style
625+
app.setupPlatformStyle();
626+
612627
/// 3. Application identification
613628
// must be set before OptionsModel is initialized or translations are loaded,
614629
// as it is used to locate QSettings
@@ -644,11 +659,9 @@ int main(int argc, char *argv[])
644659
QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", ""))));
645660
return EXIT_FAILURE;
646661
}
647-
try {
648-
node->readConfigFiles();
649-
} catch (const std::exception& e) {
662+
if (!node->readConfigFiles(error)) {
650663
QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
651-
QObject::tr("Error: Cannot parse configuration file: %1. Only use key=value syntax.").arg(e.what()));
664+
QObject::tr("Error: Cannot parse configuration file: %1.").arg(QString::fromStdString(error)));
652665
return EXIT_FAILURE;
653666
}
654667

‎src/test/getarg_tests.cpp

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,21 @@ static void ResetArgs(const std::string& strArg)
2727
for (std::string& s : vecArg)
2828
vecChar.push_back(s.c_str());
2929

30-
gArgs.ParseParameters(vecChar.size(), vecChar.data());
30+
std::string error;
31+
gArgs.ParseParameters(vecChar.size(), vecChar.data(), error);
32+
}
33+
34+
static void SetupArgs(const std::vector<std::string>& args)
35+
{
36+
gArgs.ClearArgs();
37+
for (const std::string& arg : args) {
38+
gArgs.AddArg(arg, "", false, OptionsCategory::OPTIONS);
39+
}
3140
}
3241

3342
BOOST_AUTO_TEST_CASE(boolarg)
3443
{
44+
SetupArgs({"-foo"});
3545
ResetArgs("-foo");
3646
BOOST_CHECK(gArgs.GetBoolArg("-foo", false));
3747
BOOST_CHECK(gArgs.GetBoolArg("-foo", true));
@@ -84,6 +94,7 @@ BOOST_AUTO_TEST_CASE(boolarg)
8494

8595
BOOST_AUTO_TEST_CASE(stringarg)
8696
{
97+
SetupArgs({"-foo", "-bar"});
8798
ResetArgs("");
8899
BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "");
89100
BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven");
@@ -108,6 +119,7 @@ BOOST_AUTO_TEST_CASE(stringarg)
108119

109120
BOOST_AUTO_TEST_CASE(intarg)
110121
{
122+
SetupArgs({"-foo", "-bar"});
111123
ResetArgs("");
112124
BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11);
113125
BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 0), 0);
@@ -127,6 +139,7 @@ BOOST_AUTO_TEST_CASE(intarg)
127139

128140
BOOST_AUTO_TEST_CASE(doubledash)
129141
{
142+
SetupArgs({"-foo", "-bar"});
130143
ResetArgs("--foo");
131144
BOOST_CHECK_EQUAL(gArgs.GetBoolArg("-foo", false), true);
132145

@@ -137,6 +150,7 @@ BOOST_AUTO_TEST_CASE(doubledash)
137150

138151
BOOST_AUTO_TEST_CASE(boolargno)
139152
{
153+
SetupArgs({"-foo", "-bar"});
140154
ResetArgs("-nofoo");
141155
BOOST_CHECK(!gArgs.GetBoolArg("-foo", true));
142156
BOOST_CHECK(!gArgs.GetBoolArg("-foo", false));

0 commit comments

Comments
 (0)