diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp --- a/src/bench/bench_bitcoin.cpp +++ b/src/bench/bench_bitcoin.cpp @@ -64,11 +64,20 @@ "-plot-height=", strprintf(_("Plot height in pixel (default: %u)"), DEFAULT_PLOT_HEIGHT), false, OptionsCategory::OPTIONS); + + // Hidden + gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN); } int main(int argc, char **argv) { SetupBenchArgs(); - gArgs.ParseParameters(argc, argv); + std::string error; + if (!gArgs.ParseParameters(argc, argv, error)) { + fprintf(stderr, "Error parsing command line arguments: %s\n", + error.c_str()); + return false; + } if (HelpRequested(gArgs)) { std::cout << gArgs.GetHelpMessage(); diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -60,6 +60,11 @@ strprintf(_("Send commands to node running on (default: %s)"), DEFAULT_RPCCONNECT), false, OptionsCategory::OPTIONS); + gArgs.AddArg( + "-rpccookiefile=", + _("Location of the auth cookie. Relative paths will be prefixed by a " + "net-specific datadir location. (default: data dir)"), + false, OptionsCategory::OPTIONS); gArgs.AddArg( "-rpcport=", strprintf( @@ -94,6 +99,10 @@ _("Send RPC for non-default wallet on RPC server (needs to exactly " "match corresponding -wallet option passed to bitcoind)"), false, OptionsCategory::OPTIONS); + + // Hidden + gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN); } ////////////////////////////////////////////////////////////////////////////// @@ -120,7 +129,12 @@ // Parameters // SetupCliArgs(); - gArgs.ParseParameters(argc, argv); + std::string error; + if (!gArgs.ParseParameters(argc, argv, error)) { + fprintf(stderr, "Error parsing command line arguments: %s\n", + error.c_str()); + return EXIT_FAILURE; + } if (argc < 2 || HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { std::string strUsage = PACKAGE_NAME " RPC client version " + FormatFullVersion() + "\n"; @@ -152,10 +166,9 @@ gArgs.GetArg("-datadir", "").c_str()); return EXIT_FAILURE; } - try { - gArgs.ReadConfigFiles(); - } catch (const std::exception &e) { - fprintf(stderr, "Error reading configuration file: %s\n", e.what()); + if (!gArgs.ReadConfigFiles(error, true)) { + fprintf(stderr, "Error reading configuration file: %s\n", + error.c_str()); return EXIT_FAILURE; } // Check for -testnet or -regtest parameter (BaseParams() calls are only diff --git a/src/bitcoin-tx.cpp b/src/bitcoin-tx.cpp --- a/src/bitcoin-tx.cpp +++ b/src/bitcoin-tx.cpp @@ -90,6 +90,10 @@ gArgs.AddArg("set=NAME:JSON-STRING", _("Set register NAME to given JSON-STRING"), false, OptionsCategory::REGISTER_COMMANDS); + + // Hidden + gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN); } // @@ -101,7 +105,12 @@ // Parameters // SetupBitcoinTxArgs(); - gArgs.ParseParameters(argc, argv); + std::string error; + if (!gArgs.ParseParameters(argc, argv, error)) { + fprintf(stderr, "Error parsing command line arguments: %s\n", + error.c_str()); + return EXIT_FAILURE; + } // Check for -testnet or -regtest parameter (Params() calls are only valid // after this clause) diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -76,7 +76,12 @@ _("Run in the background as a daemon and accept commands"), false, OptionsCategory::OPTIONS); #endif - gArgs.ParseParameters(argc, argv); + std::string error; + if (!gArgs.ParseParameters(argc, argv, error)) { + fprintf(stderr, "Error parsing command line arguments: %s\n", + error.c_str()); + return false; + } // Process help and version before taking care about datadir if (HelpRequested(gArgs) || gArgs.IsArgSet("-version")) { @@ -103,10 +108,9 @@ gArgs.GetArg("-datadir", "").c_str()); return false; } - try { - gArgs.ReadConfigFiles(); - } catch (const std::exception &e) { - fprintf(stderr, "Error reading configuration file: %s\n", e.what()); + if (!gArgs.ReadConfigFiles(error)) { + fprintf(stderr, "Error reading configuration file: %s\n", + error.c_str()); return false; } // Check for -testnet or -regtest parameter (Params() calls are only diff --git a/src/init.cpp b/src/init.cpp --- a/src/init.cpp +++ b/src/init.cpp @@ -974,6 +974,26 @@ strprintf("Timeout during HTTP requests (default: %d)", DEFAULT_HTTP_SERVER_TIMEOUT), true, OptionsCategory::RPC); + + // Hidden options + gArgs.AddArg("-rpcssl", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-benchmark", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-socks", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-tor", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-debugnet", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-whitelistalwaysrelay", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-blockminsize", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-dbcrashratio", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-forcecompactdb", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-usehd", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-parkdeepreorg", "", false, OptionsCategory::HIDDEN); + gArgs.AddArg("-replayprotectionactivationtime", "", false, + OptionsCategory::HIDDEN); + + // TODO remove after the Nov 2019 upgrade + gArgs.AddArg("-gravitonactivationtime", "", false, OptionsCategory::HIDDEN); } std::string LicenseInfo() { diff --git a/src/interfaces/node.h b/src/interfaces/node.h --- a/src/interfaces/node.h +++ b/src/interfaces/node.h @@ -40,7 +40,8 @@ virtual ~Node() {} //! Set command line arguments. - virtual void parseParameters(int argc, const char *const argv[]) = 0; + virtual bool parseParameters(int argc, const char *const argv[], + std::string &error) = 0; //! Set a command line argument if it doesn't already have a value virtual bool softSetArg(const std::string &arg, @@ -50,7 +51,7 @@ virtual bool softSetBoolArg(const std::string &arg, bool value) = 0; //! Load settings from configuration file. - virtual void readConfigFiles() = 0; + virtual bool readConfigFiles(std::string &error) = 0; //! Choose network parameters. virtual void selectParams(const std::string &network) = 0; diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp --- a/src/interfaces/node.cpp +++ b/src/interfaces/node.cpp @@ -51,10 +51,13 @@ namespace { class NodeImpl : public Node { - void parseParameters(int argc, const char *const argv[]) override { - gArgs.ParseParameters(argc, argv); + bool parseParameters(int argc, const char *const argv[], + std::string &error) override { + return gArgs.ParseParameters(argc, argv, error); + } + bool readConfigFiles(std::string &error) override { + return gArgs.ReadConfigFiles(error); } - void readConfigFiles() override { gArgs.ReadConfigFiles(); } bool softSetArg(const std::string &arg, const std::string &value) override { return gArgs.SoftSetArg(arg, value); diff --git a/src/qt/bitcoin.cpp b/src/qt/bitcoin.cpp --- a/src/qt/bitcoin.cpp +++ b/src/qt/bitcoin.cpp @@ -219,6 +219,9 @@ /// Get window identifier of QMainWindow (BitcoinGUI) WId getMainWinId() const; + /// Setup platform style + void setupPlatformStyle(); + public Q_SLOTS: void initializeResult(bool success); void shutdownResult(); @@ -294,10 +297,12 @@ #ifdef ENABLE_WALLET paymentServer(0), m_wallet_models(), #endif - returnValue(0) { + returnValue(0), platformStyle(0) { setQuitOnLastWindowClosed(false); +} - // UI per-platform customization. +void BitcoinApplication::setupPlatformStyle() { + // UI per-platform customization // This must be done inside the BitcoinApplication constructor, or after it, // because PlatformStyle::instantiate requires a QApplication. std::string platformName; @@ -625,16 +630,10 @@ std::unique_ptr node = interfaces::MakeNode(); - /// 1. Parse command-line options. These take precedence over anything else. - // Command-line options take precedence: - node->setupServerArgs(); - SetupUIArgs(); - node->parseParameters(argc, argv); - // Do not refer to data directory yet, this can be overridden by // Intro::pickDataDirectory - /// 2. Basic Qt initialization (not dependent on parameters or + /// 1. Basic Qt initialization (not dependent on parameters or /// configuration) Q_INIT_RESOURCE(bitcoin); Q_INIT_RESOURCE(bitcoin_locale); @@ -667,6 +666,24 @@ // behind-the-scenes in the 'Queued' connection case. qRegisterMetaType(); + /// 2. Parse command-line options. We do this after qt in order to show an + /// error if there are problems parsing these + // Command-line options take precedence: + node->setupServerArgs(); + SetupUIArgs(); + std::string error; + if (!node->parseParameters(argc, argv, error)) { + QMessageBox::critical( + 0, QObject::tr(PACKAGE_NAME), + QObject::tr("Error parsing command line arguments: %1.") + .arg(QString::fromStdString(error))); + return EXIT_FAILURE; + } + + // Now that the QApplication is setup and we have parsed our parameters, we + // can set the platform style + app.setupPlatformStyle(); + /// 3. Application identification // must be set before OptionsModel is initialized or translations are // loaded, as it is used to locate QSettings. @@ -715,14 +732,11 @@ .arg(QString::fromStdString(gArgs.GetArg("-datadir", "")))); return EXIT_FAILURE; } - try { - node->readConfigFiles(); - } catch (const std::exception &e) { + if (!node->readConfigFiles(error)) { QMessageBox::critical( 0, QObject::tr(PACKAGE_NAME), - QObject::tr("Error: Cannot parse configuration file: %1. Only use " - "key=value syntax.") - .arg(e.what())); + QObject::tr("Error: Cannot parse configuration file: %1.") + .arg(QString::fromStdString(error))); return EXIT_FAILURE; } diff --git a/src/rpc/test/server_tests.cpp b/src/rpc/test/server_tests.cpp --- a/src/rpc/test/server_tests.cpp +++ b/src/rpc/test/server_tests.cpp @@ -6,19 +6,24 @@ #include #include +#include + #include #include -#include - BOOST_FIXTURE_TEST_SUITE(server_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(server_IsDeprecatedRPCEnabled) { ArgsManager testArgs; + testArgs.AddArg("-deprecatedrpc", "", false, OptionsCategory::OPTIONS); + const char *argv_test[] = {"bitcoind", "-deprecatedrpc=foo", "-deprecatedrpc=bar"}; - testArgs.ParseParameters(3, (char **)argv_test); + std::string error; + BOOST_CHECK_MESSAGE(testArgs.ParseParameters(3, (char **)argv_test, error), + error); + BOOST_CHECK(IsDeprecatedRPCEnabled(testArgs, "foo") == true); BOOST_CHECK(IsDeprecatedRPCEnabled(testArgs, "bar") == true); BOOST_CHECK(IsDeprecatedRPCEnabled(testArgs, "bob") == false); diff --git a/src/test/getarg_tests.cpp b/src/test/getarg_tests.cpp --- a/src/test/getarg_tests.cpp +++ b/src/test/getarg_tests.cpp @@ -29,10 +29,19 @@ vecChar.push_back(s.c_str()); } - gArgs.ParseParameters(vecChar.size(), vecChar.data()); + std::string error; + gArgs.ParseParameters(vecChar.size(), vecChar.data(), error); +} + +static void SetupArgs(const std::vector &args) { + gArgs.ClearArgs(); + for (const std::string &arg : args) { + gArgs.AddArg(arg, "", false, OptionsCategory::OPTIONS); + } } BOOST_AUTO_TEST_CASE(boolarg) { + SetupArgs({"-foo"}); ResetArgs("-foo"); BOOST_CHECK(gArgs.GetBoolArg("-foo", false)); BOOST_CHECK(gArgs.GetBoolArg("-foo", true)); @@ -86,6 +95,7 @@ } BOOST_AUTO_TEST_CASE(stringarg) { + SetupArgs({"-foo", "-bar"}); ResetArgs(""); BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), ""); BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven"); @@ -108,6 +118,7 @@ } BOOST_AUTO_TEST_CASE(intarg) { + SetupArgs({"-foo", "-bar"}); ResetArgs(""); BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 11), 11); BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", 0), 0); @@ -126,6 +137,7 @@ } BOOST_AUTO_TEST_CASE(doubledash) { + SetupArgs({"-foo", "-bar"}); ResetArgs("--foo"); BOOST_CHECK_EQUAL(gArgs.GetBoolArg("-foo", false), true); @@ -135,6 +147,7 @@ } BOOST_AUTO_TEST_CASE(boolargno) { + SetupArgs({"-foo", "-bar"}); ResetArgs("-nofoo"); BOOST_CHECK(!gArgs.GetBoolArg("-foo", true)); BOOST_CHECK(!gArgs.GetBoolArg("-foo", false)); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -171,28 +171,37 @@ LOCK(cs_args); m_config_args.clear(); } - ReadConfigStream(streamConfig); + std::string error; + ReadConfigStream(streamConfig, error); } void SetNetworkOnlyArg(const std::string arg) { LOCK(cs_args); m_network_only_args.insert(arg); } + void SetupArgs(int argv, const char *args[]) { + for (int i = 0; i < argv; ++i) { + AddArg(args[i], "", false, OptionsCategory::OPTIONS); + } + } }; BOOST_AUTO_TEST_CASE(util_ParseParameters) { TestArgsManager testArgs; + const char *avail_args[] = {"-a", "-b", "-ccc", "-d"}; const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"}; - testArgs.ParseParameters(0, (char **)argv_test); + std::string error; + testArgs.SetupArgs(4, avail_args); + testArgs.ParseParameters(0, (char **)argv_test, error); BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); - testArgs.ParseParameters(1, (char **)argv_test); + testArgs.ParseParameters(1, (char **)argv_test, error); BOOST_CHECK(testArgs.GetOverrideArgs().empty() && testArgs.GetConfigArgs().empty()); - testArgs.ParseParameters(7, (char **)argv_test); + testArgs.ParseParameters(7, (char **)argv_test, error); // expectation: -ignored is ignored (program name argument), // -a, -b and -ccc end up in map, -d ignored because it is after // a non-option argument (non-GNU option parsing) @@ -217,9 +226,12 @@ BOOST_AUTO_TEST_CASE(util_GetBoolArg) { TestArgsManager testArgs; + const char *avail_args[] = {"-a", "-b", "-c", "-d", "-e", "-f"}; const char *argv_test[] = {"ignored", "-a", "-nob", "-c=0", "-d=1", "-e=false", "-f=true"}; - testArgs.ParseParameters(7, (char **)argv_test); + std::string error; + testArgs.SetupArgs(6, avail_args); + testArgs.ParseParameters(7, (char **)argv_test, error); // Each letter should be set. for (char opt : "abcdef") { @@ -251,8 +263,11 @@ TestArgsManager testArgs; // Params test + const char *avail_args[] = {"-foo", "-bar"}; const char *argv_test[] = {"ignored", "-nofoo", "-foo", "-nobar=0"}; - testArgs.ParseParameters(4, (char **)argv_test); + testArgs.SetupArgs(2, avail_args); + std::string error; + testArgs.ParseParameters(4, (char **)argv_test, error); // This was passed twice, second one overrides the negative setting. BOOST_CHECK(!testArgs.IsArgNegated("-foo")); @@ -264,7 +279,7 @@ // Config test const char *conf_test = "nofoo=1\nfoo=1\nnobar=0\n"; - testArgs.ParseParameters(1, (char **)argv_test); + testArgs.ParseParameters(1, (char **)argv_test, error); testArgs.ReadConfigString(conf_test); // This was passed twice, second one overrides the negative setting, @@ -279,7 +294,7 @@ // Combined test const char *combo_test_args[] = {"ignored", "-nofoo", "-bar"}; const char *combo_test_conf = "foo=1\nnobar=1\n"; - testArgs.ParseParameters(3, (char **)combo_test_args); + testArgs.ParseParameters(3, (char **)combo_test_args, error); testArgs.ReadConfigString(combo_test_conf); // Command line overrides, but doesn't erase old setting @@ -317,6 +332,9 @@ "iii=2\n"; TestArgsManager test_args; + const char *avail_args[] = {"-a", "-b", "-ccc", "-d", "-e", + "-fff", "-ggg", "-h", "-i", "-iii"}; + test_args.SetupArgs(10, avail_args); test_args.ReadConfigString(str_config); // expectation: a, b, ccc, d, fff, ggg, h, i end up in map @@ -609,6 +627,8 @@ BOOST_AUTO_TEST_CASE(util_GetChainName) { TestArgsManager test_args; + const char *avail_args[] = {"-testnet", "-regtest"}; + test_args.SetupArgs(2, avail_args); const char *argv_testnet[] = {"cmd", "-testnet"}; const char *argv_regtest[] = {"cmd", "-regtest"}; @@ -618,39 +638,40 @@ // equivalent to "-testnet" // regtest in testnet section is ignored const char *testnetconf = "testnet=1\nregtest=0\n[test]\nregtest=1"; + std::string error; - test_args.ParseParameters(0, (char **)argv_testnet); + test_args.ParseParameters(0, (char **)argv_testnet, error); BOOST_CHECK_EQUAL(test_args.GetChainName(), "main"); - test_args.ParseParameters(2, (char **)argv_testnet); + test_args.ParseParameters(2, (char **)argv_testnet, error); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char **)argv_regtest); + test_args.ParseParameters(2, (char **)argv_regtest, error); BOOST_CHECK_EQUAL(test_args.GetChainName(), "regtest"); - test_args.ParseParameters(3, (char **)argv_test_no_reg); + test_args.ParseParameters(3, (char **)argv_test_no_reg, error); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char **)argv_both); + test_args.ParseParameters(3, (char **)argv_both, error); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(0, (char **)argv_testnet); + test_args.ParseParameters(0, (char **)argv_testnet, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char **)argv_testnet); + test_args.ParseParameters(2, (char **)argv_testnet, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char **)argv_regtest); + test_args.ParseParameters(2, (char **)argv_regtest, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(3, (char **)argv_test_no_reg); + test_args.ParseParameters(3, (char **)argv_test_no_reg, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char **)argv_both); + test_args.ParseParameters(3, (char **)argv_both, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); @@ -658,23 +679,23 @@ // [test] regtest=1 potentially relevant) doesn't break things test_args.SelectConfigNetwork("test"); - test_args.ParseParameters(0, (char **)argv_testnet); + test_args.ParseParameters(0, (char **)argv_testnet, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char **)argv_testnet); + test_args.ParseParameters(2, (char **)argv_testnet, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(2, (char **)argv_regtest); + test_args.ParseParameters(2, (char **)argv_regtest, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); - test_args.ParseParameters(2, (char **)argv_test_no_reg); + test_args.ParseParameters(2, (char **)argv_test_no_reg, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_EQUAL(test_args.GetChainName(), "test"); - test_args.ParseParameters(3, (char **)argv_both); + test_args.ParseParameters(3, (char **)argv_both, error); test_args.ReadConfigString(testnetconf); BOOST_CHECK_THROW(test_args.GetChainName(), std::runtime_error); } diff --git a/src/util.h b/src/util.h --- a/src/util.h +++ b/src/util.h @@ -148,7 +148,8 @@ std::set m_network_only_args; std::map> m_available_args; - void ReadConfigStream(std::istream &stream); + bool ReadConfigStream(std::istream &stream, std::string &error, + bool ignore_invalid_keys = false); public: ArgsManager(); @@ -158,8 +159,9 @@ */ void SelectConfigNetwork(const std::string &network); - void ParseParameters(int argc, const char *const argv[]); - void ReadConfigFiles(); + bool ParseParameters(int argc, const char *const argv[], + std::string &error); + bool ReadConfigFiles(std::string &error, bool ignore_invalid_keys = false); /** * Log warnings for options in m_section_only_args when they are specified @@ -264,10 +266,20 @@ // Remove an arg setting, used only in testing void ClearArg(const std::string &strArg); + /** + * Clear available arguments + */ + void ClearArgs() { m_available_args.clear(); } + /** * Get the help string */ std::string GetHelpMessage(); + + /** + * Check whether we know of this arg + */ + bool IsArgKnown(const std::string &key, std::string &error); }; extern ArgsManager gArgs; diff --git a/src/util.cpp b/src/util.cpp --- a/src/util.cpp +++ b/src/util.cpp @@ -444,7 +444,8 @@ m_network = network; } -void ArgsManager::ParseParameters(int argc, const char *const argv[]) { +bool ArgsManager::ParseParameters(int argc, const char *const argv[], + std::string &error) { LOCK(cs_args); m_override_args.clear(); @@ -478,6 +479,14 @@ } else { m_override_args[key].push_back(val); } + + // Check that the arg is known + if (!(IsSwitchChar(key[0]) && key.size() == 1)) { + if (!IsArgKnown(key, error)) { + error = strprintf("Invalid parameter %s", key.c_str()); + return false; + } + } } // we do not allow -includeconf from command line, so we clear it here @@ -493,6 +502,25 @@ m_override_args.erase(it); } } + return true; +} + +bool ArgsManager::IsArgKnown(const std::string &key, std::string &error) { + size_t option_index = key.find('.'); + std::string arg_no_net; + if (option_index == std::string::npos) { + arg_no_net = key; + } else { + arg_no_net = + std::string("-") + key.substr(option_index + 1, std::string::npos); + } + + for (const auto &arg_map : m_available_args) { + if (arg_map.second.count(arg_no_net)) { + return true; + } + } + return false; } std::vector ArgsManager::GetArgs(const std::string &strArg) const { @@ -875,7 +903,8 @@ return pathConfigFile; } -void ArgsManager::ReadConfigStream(std::istream &stream) { +bool ArgsManager::ReadConfigStream(std::istream &stream, std::string &error, + bool ignore_invalid_keys) { LOCK(cs_args); std::set setOptions; @@ -887,15 +916,25 @@ it != end; ++it) { std::string strKey = std::string("-") + it->string_key; std::string strValue = it->value[0]; + if (InterpretNegatedOption(strKey, strValue)) { m_config_args[strKey].clear(); } else { m_config_args[strKey].push_back(strValue); } + + // Check that the arg is known + if (!IsArgKnown(strKey, error) && !ignore_invalid_keys) { + error = strprintf("Invalid configuration value %s", + it->string_key.c_str()); + return false; + } } + return true; } -void ArgsManager::ReadConfigFiles() { +bool ArgsManager::ReadConfigFiles(std::string &error, + bool ignore_invalid_keys) { { LOCK(cs_args); m_config_args.clear(); @@ -906,7 +945,9 @@ // ok to not have a config file if (stream.good()) { - ReadConfigStream(stream); + if (!ReadConfigStream(stream, error, ignore_invalid_keys)) { + return false; + } // if there is an -includeconf in the override args, but it is empty, // that means the user passed '-noincludeconf' on the command line, in // which case we should not include anything @@ -930,7 +971,10 @@ for (const std::string &to_include : includeconf) { fs::ifstream include_config(GetConfigFile(to_include)); if (include_config.good()) { - ReadConfigStream(include_config); + if (!ReadConfigStream(include_config, error, + ignore_invalid_keys)) { + return false; + } LogPrintf("Included configuration file %s\n", to_include.c_str()); } else { @@ -944,10 +988,11 @@ // If datadir is changed in .conf file: ClearDatadirCache(); if (!fs::is_directory(GetDataDir(false))) { - throw std::runtime_error( - strprintf("specified data directory \"%s\" does not exist.", - gArgs.GetArg("-datadir", "").c_str())); + error = strprintf("specified data directory \"%s\" does not exist.", + gArgs.GetArg("-datadir", "").c_str()); + return false; } + return true; } std::string ArgsManager::GetChainName() const { diff --git a/test/functional/feature_help.py b/test/functional/feature_help.py --- a/test/functional/feature_help.py +++ b/test/functional/feature_help.py @@ -39,6 +39,19 @@ output = self.nodes[0].process.stdout.read() assert b'version' in output self.log.info("Version text received: {} (...)".format(output[0:60])) + + # Test that arguments not in the help results in an error + self.log.info( + "Start bitcoind with -fakearg to make sure it does not start") + self.nodes[0].start(extra_args=['-fakearg'], + stderr=subprocess.PIPE, stdout=subprocess.PIPE) + # Node should exit immediately and output an error to stderr + ret_code = self.nodes[0].process.wait(timeout=1) + assert_equal(ret_code, 1) + output = self.nodes[0].process.stderr.read() + assert b'Error parsing command line arguments' in output + self.log.info("Error message received: {} (...)".format(output[0:60])) + # Clean up TestNode state self.nodes[0].running = False self.nodes[0].process = None diff --git a/test/lint/check-doc.py b/test/lint/check-doc.py --- a/test/lint/check-doc.py +++ b/test/lint/check-doc.py @@ -24,24 +24,6 @@ REGEX_ARG = r'(?:ForceSet|SoftSet|Get|Is)(?:Bool)?Args?(?:Set)?\(\s*"(-[^"]+)"' REGEX_DOC = r'AddArg\(\s*"(-[^"=]+?)(?:=|")' -# list unsupported, deprecated and duplicate args as they need no documentation -SET_DOC_OPTIONAL = set(['-benchmark', - '-blockminsize', - '-dbcrashratio', - '-debugnet', - '-forcecompactdb', - # TODO remove after the Nov 2019 upgrade - '-gravitonactivationtime', - '-h', - '-help', - '-parkdeepreorg', - '-replayprotectionactivationtime', - '-rpcssl', - '-socks', - '-tor', - '-usehd', - '-whitelistalwaysrelay']) - # list false positive unknows arguments SET_FALSE_POSITIVE_UNKNOWNS = set(['-zmqpubhashblock', '-zmqpubhashtx', @@ -72,14 +54,13 @@ args_docd |= set(re.findall(re.compile(REGEX_DOC), content)) args_used |= SET_FALSE_POSITIVE_UNKNOWNS - args_need_doc = args_used - args_docd - SET_DOC_OPTIONAL + args_need_doc = args_used - args_docd args_unknown = args_docd - args_used pp = PrettyPrinter() print("Args used : {}".format(len(args_used))) print("Args documented : {}".format(len(args_docd))) - print("Args undocumented: {} ({} don't need documentation)".format( - len(args_need_doc), len(SET_DOC_OPTIONAL))) + print("Args undocumented: {}".format(len(args_need_doc))) pp.pprint(args_need_doc) print("Args unknown : {}".format(len(args_unknown))) pp.pprint(args_unknown)