diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index e702be1314..60b9694cb1 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -1079,6 +1079,68 @@ static UniValue GetNewAddress() return ConnectAndCallRPC(&rh, "getnewaddress", /* args=*/{}, wallet_name); } +/** + * IsExclusivelyCliCommand checks if a string is a cli-command that needs to run exclusively + * @param[in] cli_command Reference to const string cli-command. + * @returns true if cli_command it's included in the set of the exclusively_commands. + */ +bool IsExclusivelyCliCommand(const std::string cli_command) +{ + // Initialize set with cli-commands that run exclusively + const std::set exclusively_commands{"-generate", "-netinfo", "-addrinfo", "-getinfo"}; + + // return if element was found or not + return (exclusively_commands.find(cli_command) != exclusively_commands.end()); +} + +/** + * ValidateCliCommand checks for: + * 1. duplicate command line arguments + * 2. no multiple bitcoin-cli side commands allowance + * 3. unrecognised cli-command or any other argument starting with a slash + * and throws an exception if any of the conditions are met. + * @throws std::runtime_error if there are invalid args. + */ +static void ValidateCliCommand(int argc, char *argv[]) +{ + std::set commands; + std::string exclusively_command; + for (int i = 1; i < argc; ++i) { + if (IsSwitchChar(argv[i][0])) { + std::string command(argv[i]); + // Check if the command line argument has an =, in that case, ignore what's after; + // e.g.: -color=never; we just want to check "-color" + std::size_t pos = command.find('='); + if (pos != std::string::npos) { + command = command.substr(0, pos); + } + + // Check if this command has already been seen + if (commands.find(command) != commands.end()) { + throw std::runtime_error(strprintf("Error: duplicate command line argument %s", command)); + } + + // Check if it's a cli-command that runs exclusively e.g. a user could run: + // "bitcoin-cli -regtest -generate -netinfo -addrinfo -getinfo" but it's confusing, + // only the last one will be executed so we make sure the user specify only one + if (IsExclusivelyCliCommand(command)) { + if (exclusively_command != "") { + throw std::runtime_error(strprintf("Error: you can only run one cli-command at a time, either %s or %s", exclusively_command, command)); + } + exclusively_command = command; + } + + // Check if the "-" command was recognised by the ArgsManager or interpreted as a parameter + // e.g.: bitcoin-cli -generate 1 -rpcwallet=xxx, is an invalid call + if (!gArgs.IsArgSet(command) && exclusively_command != "") { + throw std::runtime_error(strprintf("Error: invalid cli-command argument. If \"%s\" is a valid option try passing it before the \"%s\" command.", argv[i], exclusively_command)); + } + + commands.insert(command); + } + } +} + /** * Check bounds and set up args for RPC generatetoaddress params: nblocks, address, maxtries. * @param[in] address Reference to const string address to insert into the args. @@ -1100,6 +1162,7 @@ static int CommandLineRPC(int argc, char *argv[]) std::string strPrint; int nRet = 0; try { + ValidateCliCommand(argc, argv); // Skip switches while (argc > 1 && IsSwitchChar(argv[1][0])) { argc--; diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 8a99687c1d..ffb2083b7a 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -29,6 +29,9 @@ BLOCKS_VALUE_OF_ZERO = 'error: the first argument (number of blocks to generate, TOO_MANY_ARGS = 'error: too many arguments (maximum 2 for nblocks and maxtries)' WALLET_NOT_LOADED = 'Requested wallet does not exist or is not loaded' WALLET_NOT_SPECIFIED = "Multiple wallets are loaded. Please specify a wallet to use" +INVALID_CLI_COMMAND_ARGUMENT = 'Error: invalid cli-command argument. If \"{}\" is a valid option try passing it before the \"{}\" command.' +DUPLICATE_COMMAND_ERROR = 'Error: duplicate command line argument {}' +MULTIPLE_CLI_COMMAND_ERROR = "Error: you can only run one cli-command at a time, either {} or {}" def cli_get_info_string_to_dict(cli_get_info_string): @@ -247,6 +250,11 @@ class TestBitcoinCli(BitcoinTestFramework): assert_equal(len(generate["blocks"]), n1) assert_equal(self.nodes[0].getblockcount(), blocks + 1 + n1) + # Single or Multi wallet modes don't matter here as -generate command validation is performed before we send to RPC + # Note also that -rpcwallet arg intentionally adds some confusion but could be any arg starting with "-" + self.log.info('Test -generate with nblocks and -rpcwallet afterwards') + assert_raises_process_error(1, INVALID_CLI_COMMAND_ARGUMENT.format(rpcwallet3, "-generate"), self.nodes[0].cli('-generate', 1, rpcwallet3).send_cli) + self.log.info('Test -generate with nblocks and maxtries') generate = self.nodes[0].cli('-generate', n2, 1000000).send_cli() assert_equal(set(generate.keys()), {'address', 'blocks'}) @@ -263,6 +271,8 @@ class TestBitcoinCli(BitcoinTestFramework): assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate').echo) assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate', 'foo').echo) assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate', 0).echo) + + self.log.info('Test -generate with bad args & -rpcwallet=unloaded wallet') assert_raises_rpc_error(-18, WALLET_NOT_LOADED, self.nodes[0].cli(rpcwallet3, '-generate', 1, 2, 3).echo) # Test bitcoin-cli -generate with -rpcwallet in multiwallet mode. @@ -288,6 +298,10 @@ class TestBitcoinCli(BitcoinTestFramework): assert_equal(len(generate["blocks"]), n3) assert_equal(self.nodes[0].getblockcount(), blocks + 1 + n3) + # Note, same as above, -rpcwallet arg intentionally adds some confusion but could be any arg starting with "-" + self.log.info('Test -generate -rpcwallet with nblocks and -rpcwallet again afterwards') + assert_raises_process_error(1, DUPLICATE_COMMAND_ERROR.format("-rpcwallet"), self.nodes[0].cli(rpcwallet3, '-generate', 1, rpcwallet3).send_cli) + self.log.info('Test -generate -rpcwallet with nblocks and maxtries') generate = self.nodes[0].cli(rpcwallet2, '-generate', n4, 1000000).send_cli() assert_equal(set(generate.keys()), {'address', 'blocks'}) @@ -298,7 +312,12 @@ class TestBitcoinCli(BitcoinTestFramework): assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate').echo) assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate', 'foo').echo) assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate', 0).echo) + + self.log.info('Test -generate with bad args & without -rpcwallet in multiwallet mode') assert_raises_rpc_error(-19, WALLET_NOT_SPECIFIED, self.nodes[0].cli('-generate', 1, 2, 3).echo) + + self.log.info('Test running multiple cli-commands in one line') + assert_raises_process_error(1, MULTIPLE_CLI_COMMAND_ERROR.format("-generate", "-getinfo"), self.nodes[0].cli('-generate', '-getinfo', "-addrinfo").send_cli) else: self.log.info("*** Wallet not compiled; cli getwalletinfo and -getinfo wallet tests skipped") self.generate(self.nodes[0], 25) # maintain block parity with the wallet_compiled conditional branch