From 8838c4f171f3c3e568d237b216167fddb521d0a7 Mon Sep 17 00:00:00 2001 From: naiyoma Date: Sat, 18 May 2024 23:16:31 +0300 Subject: [PATCH 1/2] common/args.h: automate check for multiple cli commands Co-authored-by: Anthony Towns --- src/bitcoin-cli.cpp | 9 +++++---- src/common/args.cpp | 21 +++++++++++++++++++++ src/common/args.h | 8 ++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 44fc2731639..d0f90768ba2 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -86,10 +86,10 @@ static void SetupCliArgs(ArgsManager& argsman) "arguments are number of blocks to generate (default: %s) and maximum iterations to try (default: %s), equivalent to " "RPC generatetoaddress nblocks and maxtries arguments. Example: bitcoin-cli -generate 4 1000", DEFAULT_NBLOCKS, DEFAULT_MAX_TRIES), - ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-addrinfo", "Get the number of addresses known to the node, per network and total, after filtering for quality and recency. The total number of addresses known to the node may be higher.", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - argsman.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the output of -getinfo is the result of multiple non-atomic requests. Some entries in the output may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); - 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); + ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS); + argsman.AddArg("-addrinfo", "Get the number of addresses known to the node, per network and total, after filtering for quality and recency. The total number of addresses known to the node may be higher.", ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS); + argsman.AddArg("-getinfo", "Get general information from the remote server. Note that unlike server-side RPC calls, the output of -getinfo is the result of multiple non-atomic requests. Some entries in the output may represent results from different states (e.g. wallet balance may be as of a different block from the chain state reported)", ArgsManager::ALLOW_ANY, OptionsCategory::CLI_COMMANDS); + 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::CLI_COMMANDS); SetupChainParamsBaseOptions(argsman); argsman.AddArg("-color=", 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); @@ -1209,6 +1209,7 @@ static int CommandLineRPC(int argc, char *argv[]) fputc('\n', stdout); } } + gArgs.CheckMultipleCLIArgs(); std::unique_ptr rh; std::string method; if (gArgs.IsArgSet("-getinfo")) { diff --git a/src/common/args.cpp b/src/common/args.cpp index caff36fdb30..28d3093c170 100644 --- a/src/common/args.cpp +++ b/src/common/args.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #ifdef WIN32 #include /* for codecvt_utf8_utf16 */ @@ -587,6 +588,23 @@ void ArgsManager::AddHiddenArgs(const std::vector& names) } } +void ArgsManager::CheckMultipleCLIArgs() const +{ + LOCK(cs_args); + std::vector found{}; + auto cmds = m_available_args.find(OptionsCategory::CLI_COMMANDS); + if (cmds != m_available_args.end()) { + for (const auto& [cmd, argspec] : cmds->second) { + if (IsArgSet(cmd)) { + found.push_back(cmd); + } + } + if (found.size() > 1) { + throw std::runtime_error(strprintf("Only one of %s may be specified.", util::Join(found, ", "))); + } + } +} + std::string ArgsManager::GetHelpMessage() const { const bool show_debug = GetBoolArg("-help-debug", false); @@ -634,6 +652,9 @@ std::string ArgsManager::GetHelpMessage() const case OptionsCategory::REGISTER_COMMANDS: usage += HelpMessageGroup("Register Commands:"); break; + case OptionsCategory::CLI_COMMANDS: + usage += HelpMessageGroup("CLI Commands:"); + break; default: break; } diff --git a/src/common/args.h b/src/common/args.h index 78a61313b91..d9d14b1c01b 100644 --- a/src/common/args.h +++ b/src/common/args.h @@ -63,6 +63,7 @@ enum class OptionsCategory { GUI, COMMANDS, REGISTER_COMMANDS, + CLI_COMMANDS, HIDDEN // Always the last option to avoid printing these in the help }; @@ -363,6 +364,13 @@ protected: m_network_only_args.clear(); } + /** + * Check CLI command args + * + * @throws std::runtime_error when multiple CLI_COMMAND arguments are specified + */ + void CheckMultipleCLIArgs() const; + /** * Get the help string */ From c8e6771af002eaf15567794fcdc57fdb0e3fb140 Mon Sep 17 00:00:00 2001 From: naiyoma Date: Fri, 26 Jul 2024 19:27:25 +0300 Subject: [PATCH 2/2] test: restrict multiple CLI arguments --- src/test/fuzz/system.cpp | 2 +- test/functional/interface_bitcoin_cli.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/test/fuzz/system.cpp b/src/test/fuzz/system.cpp index 73ae89b52a7..75f444e84f2 100644 --- a/src/test/fuzz/system.cpp +++ b/src/test/fuzz/system.cpp @@ -53,7 +53,7 @@ FUZZ_TARGET(system, .init = initialize_system) args_manager.SoftSetBoolArg(fuzzed_data_provider.ConsumeRandomLengthString(16), fuzzed_data_provider.ConsumeBool()); }, [&] { - const OptionsCategory options_category = fuzzed_data_provider.PickValueInArray({OptionsCategory::OPTIONS, OptionsCategory::CONNECTION, OptionsCategory::WALLET, OptionsCategory::WALLET_DEBUG_TEST, OptionsCategory::ZMQ, OptionsCategory::DEBUG_TEST, OptionsCategory::CHAINPARAMS, OptionsCategory::NODE_RELAY, OptionsCategory::BLOCK_CREATION, OptionsCategory::RPC, OptionsCategory::GUI, OptionsCategory::COMMANDS, OptionsCategory::REGISTER_COMMANDS, OptionsCategory::HIDDEN}); + const OptionsCategory options_category = fuzzed_data_provider.PickValueInArray({OptionsCategory::OPTIONS, OptionsCategory::CONNECTION, OptionsCategory::WALLET, OptionsCategory::WALLET_DEBUG_TEST, OptionsCategory::ZMQ, OptionsCategory::DEBUG_TEST, OptionsCategory::CHAINPARAMS, OptionsCategory::NODE_RELAY, OptionsCategory::BLOCK_CREATION, OptionsCategory::RPC, OptionsCategory::GUI, OptionsCategory::COMMANDS, OptionsCategory::REGISTER_COMMANDS, OptionsCategory::CLI_COMMANDS, OptionsCategory::HIDDEN}); // Avoid hitting: // common/args.cpp:563: void ArgsManager::AddArg(const std::string &, const std::string &, unsigned int, const OptionsCategory &): Assertion `ret.second' failed. const std::string argument_name = GetArgumentName(fuzzed_data_provider.ConsumeRandomLengthString(16)); diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index a6628dcbf3f..f215bad13c1 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -381,6 +381,9 @@ class TestBitcoinCli(BitcoinTestFramework): assert_raises_process_error(1, "Could not connect to the server", self.nodes[0].cli('-rpcwait', '-rpcwaittimeout=5').echo) assert_greater_than_or_equal(time.time(), start_time + 5) + self.log.info("Test that only one of -addrinfo, -generate, -getinfo, -netinfo may be specified at a time") + assert_raises_process_error(1, "Only one of -getinfo, -netinfo may be specified", self.nodes[0].cli('-getinfo', '-netinfo').send_cli) + if __name__ == '__main__': TestBitcoinCli().main()