Skip to content

Commit

Permalink
Give an error and exit if there are unknown parameters
Browse files Browse the repository at this point in the history
If an unknown option is given via either the command line args or
the conf file, throw an error and exit

Update tests for ArgsManager knowing args

Ignore unknown options in the config file for bitcoin-cli

Fix tests and bitcoin-cli to match actual options used
  • Loading branch information
achow101 committed May 30, 2018
1 parent 174f7c8 commit 4f8704d
Show file tree
Hide file tree
Showing 12 changed files with 205 additions and 66 deletions.
10 changes: 9 additions & 1 deletion src/bench/bench_bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,21 @@ static void SetupBenchArgs()
gArgs.AddArg("-plot-plotlyurl=<uri>", strprintf("URL to use for plotly.js (default: %s)", DEFAULT_PLOT_PLOTLYURL), false, OptionsCategory::OPTIONS);
gArgs.AddArg("-plot-width=<x>", strprintf("Plot width in pixel (default: %u)", DEFAULT_PLOT_WIDTH), false, OptionsCategory::OPTIONS);
gArgs.AddArg("-plot-height=<x>", 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();
Expand Down
17 changes: 12 additions & 5 deletions src/bitcoin-cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,18 @@ static void SetupCliArgs()
gArgs.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), false, OptionsCategory::OPTIONS);
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);
gArgs.AddArg("-rpcconnect=<ip>", strprintf("Send commands to node running on <ip> (default: %s)", DEFAULT_RPCCONNECT), false, OptionsCategory::OPTIONS);
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);
gArgs.AddArg("-rpcpassword=<pw>", "Password for JSON-RPC connections", false, OptionsCategory::OPTIONS);
gArgs.AddArg("-rpcport=<port>", strprintf("Connect to JSON-RPC on <port> (default: %u or testnet: %u)", defaultBaseParams->RPCPort(), testnetBaseParams->RPCPort()), false, OptionsCategory::OPTIONS);
gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", false, OptionsCategory::OPTIONS);
gArgs.AddArg("-rpcwait", "Wait for RPC server to start", false, OptionsCategory::OPTIONS);
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);
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);
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);

// Hidden
gArgs.AddArg("-h", "", false, OptionsCategory::HIDDEN);
gArgs.AddArg("-help", "", false, OptionsCategory::HIDDEN);
}

//////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -80,7 +85,11 @@ static int AppInitRPC(int argc, char* argv[])
// 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 = strprintf("%s RPC client version", PACKAGE_NAME) + " " + FormatFullVersion() + "\n";
if (!gArgs.IsArgSet("-version")) {
Expand All @@ -104,10 +113,8 @@ static int AppInitRPC(int argc, char* argv[])
fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", 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 valid after this clause)
Expand Down
10 changes: 9 additions & 1 deletion src/bitcoin-tx.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ static void SetupBitcoinTxArgs()

gArgs.AddArg("load=NAME:FILENAME", "Load JSON file FILENAME into register NAME", false, OptionsCategory::REGISTER_COMMANDS);
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);
}

//
Expand All @@ -76,7 +80,11 @@ static int AppInitRawTx(int argc, char* argv[])
// 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)
try {
Expand Down
13 changes: 7 additions & 6 deletions src/bitcoind.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,11 @@ static bool AppInit(int argc, char* argv[])
#if HAVE_DECL_DAEMON
gArgs.AddArg("-daemon", "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")) {
Expand Down Expand Up @@ -94,11 +98,8 @@ static bool AppInit(int argc, char* argv[])
fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", 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 valid after this clause)
Expand Down
17 changes: 17 additions & 0 deletions src/init.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,23 @@ void SetupServerArgs()
gArgs.AddArg("-rpcuser=<user>", "Username for JSON-RPC connections", false, OptionsCategory::RPC);
gArgs.AddArg("-rpcworkqueue=<n>", strprintf("Set the depth of the work queue to service RPC calls (default: %d)", DEFAULT_HTTP_WORKQUEUE), true, OptionsCategory::RPC);
gArgs.AddArg("-server", "Accept command line and JSON-RPC commands", false, 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("-prematurewitness", "", false, OptionsCategory::HIDDEN);
gArgs.AddArg("-walletprematurewitness", "", false, OptionsCategory::HIDDEN);
gArgs.AddArg("-promiscuousmempoolflags", "", 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);
}

std::string LicenseInfo()
Expand Down
6 changes: 3 additions & 3 deletions src/interfaces/node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,11 @@ namespace {

class NodeImpl : public Node
{
void parseParameters(int argc, const char* const argv[]) override
bool parseParameters(int argc, const char* const argv[], std::string& error) override
{
gArgs.ParseParameters(argc, argv);
return gArgs.ParseParameters(argc, argv, error);
}
void readConfigFiles() override { gArgs.ReadConfigFiles(); }
bool readConfigFiles(std::string& error) override { return gArgs.ReadConfigFiles(error); }
bool softSetArg(const std::string& arg, const std::string& value) override { return gArgs.SoftSetArg(arg, value); }
bool softSetBoolArg(const std::string& arg, bool value) override { return gArgs.SoftSetBoolArg(arg, value); }
void selectParams(const std::string& network) override { SelectParams(network); }
Expand Down
4 changes: 2 additions & 2 deletions src/interfaces/node.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class Node
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, const std::string& value) = 0;
Expand All @@ -47,7 +47,7 @@ class Node
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;
Expand Down
37 changes: 25 additions & 12 deletions src/qt/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,9 @@ class BitcoinApplication: public QApplication
/// Get window identifier of QMainWindow (BitcoinGUI)
WId getMainWinId() const;

/// Setup platform style
void setupPlatformStyle();

public Q_SLOTS:
void initializeResult(bool success);
void shutdownResult();
Expand Down Expand Up @@ -315,10 +318,14 @@ BitcoinApplication::BitcoinApplication(interfaces::Node& node, int &argc, char *
paymentServer(0),
m_wallet_models(),
#endif
returnValue(0)
returnValue(0),
platformStyle(0)
{
setQuitOnLastWindowClosed(false);
}

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

std::unique_ptr<interfaces::Node> 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 configuration)
/// 1. Basic Qt initialization (not dependent on parameters or configuration)
#if QT_VERSION < 0x050000
// Internal string conversion is all UTF-8
QTextCodec::setCodecForTr(QTextCodec::codecForName("UTF-8"));
Expand Down Expand Up @@ -609,6 +610,20 @@ int main(int argc, char *argv[])
qRegisterMetaType<WalletModel*>("WalletModel*");
#endif

/// 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
Expand Down Expand Up @@ -644,11 +659,9 @@ int main(int argc, char *argv[])
QObject::tr("Error: Specified data directory \"%1\" does not exist.").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;
}

Expand Down
16 changes: 15 additions & 1 deletion src/test/getarg_tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,21 @@ static void ResetArgs(const std::string& strArg)
for (std::string& s : vecArg)
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<std::string>& 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));
Expand Down Expand Up @@ -84,6 +94,7 @@ BOOST_AUTO_TEST_CASE(boolarg)

BOOST_AUTO_TEST_CASE(stringarg)
{
SetupArgs({"-foo", "-bar"});
ResetArgs("");
BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", ""), "");
BOOST_CHECK_EQUAL(gArgs.GetArg("-foo", "eleven"), "eleven");
Expand All @@ -108,6 +119,7 @@ BOOST_AUTO_TEST_CASE(stringarg)

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);
Expand All @@ -127,6 +139,7 @@ BOOST_AUTO_TEST_CASE(intarg)

BOOST_AUTO_TEST_CASE(doubledash)
{
SetupArgs({"-foo", "-bar"});
ResetArgs("--foo");
BOOST_CHECK_EQUAL(gArgs.GetBoolArg("-foo", false), true);

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

BOOST_AUTO_TEST_CASE(boolargno)
{
SetupArgs({"-foo", "-bar"});
ResetArgs("-nofoo");
BOOST_CHECK(!gArgs.GetBoolArg("-foo", true));
BOOST_CHECK(!gArgs.GetBoolArg("-foo", false));
Expand Down
Loading

0 comments on commit 4f8704d

Please sign in to comment.