Merge #15401: rpc: Actually throw help when passed invalid number of params

fa4ce7038d rpc: Actually throw help when passed invalid number of params (MarcoFalke)
fa05626ca7 rpc: Add RPCHelpMan::IsValidNumArgs() (MarcoFalke)

Pull request description:

  Can be tested by

  * running the included test against an old binary (compiled without this patch)
  * calling `setban 1 "add" 3 4 5 6 7 8 9 0` in the gui

Tree-SHA512: aa6a25bbe6f40722913ea292252a62a4012c964eed9f4035335a2e2d13be98eb60f368e8a3251a104a26a62c08b2cb926b06e5ab1418ef1cf4abdd71d87c2919
This commit is contained in:
Wladimir J. van der Laan 2019-02-25 09:29:50 +01:00
commit 1a8a5ede9f
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D
5 changed files with 46 additions and 22 deletions

View file

@ -1,5 +1,5 @@
// Copyright (c) 2010 Satoshi Nakamoto
// Copyright (c) 2009-2018 The Bitcoin Core developers
// Copyright (c) 2009-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -1778,9 +1778,7 @@ static constexpr size_t PER_UTXO_OVERHEAD = sizeof(COutPoint) + sizeof(uint32_t)
static UniValue getblockstats(const JSONRPCRequest& request)
{
if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) {
throw std::runtime_error(
RPCHelpMan{"getblockstats",
const RPCHelpMan help{"getblockstats",
"\nCompute per block statistics for a given window. All amounts are in satoshis.\n"
"It won't work for some heights with pruning.\n"
"It won't work without -txindex for utxo_size_inc, *fee or *feerate stats.\n",
@ -1836,7 +1834,9 @@ static UniValue getblockstats(const JSONRPCRequest& request)
HelpExampleCli("getblockstats", "1000 '[\"minfeerate\",\"avgfeerate\"]'")
+ HelpExampleRpc("getblockstats", "1000 '[\"minfeerate\",\"avgfeerate\"]'")
},
}.ToString());
};
if (request.fHelp || !help.IsValidNumArgs(request.params.size())) {
throw std::runtime_error(help.ToString());
}
LOCK(cs_main);

View file

@ -523,13 +523,7 @@ static UniValue getnetworkinfo(const JSONRPCRequest& request)
static UniValue setban(const JSONRPCRequest& request)
{
std::string strCommand;
if (!request.params[1].isNull())
strCommand = request.params[1].get_str();
if (request.fHelp || request.params.size() < 2 ||
(strCommand != "add" && strCommand != "remove"))
throw std::runtime_error(
RPCHelpMan{"setban",
const RPCHelpMan help{"setban",
"\nAttempts to add or remove an IP/Subnet from the banned list.\n",
{
{"subnet", RPCArg::Type::STR, RPCArg::Optional::NO, "The IP/Subnet (see getpeerinfo for nodes IP) with an optional netmask (default is /32 = single IP)"},
@ -543,7 +537,13 @@ static UniValue setban(const JSONRPCRequest& request)
+ HelpExampleCli("setban", "\"192.168.0.0/24\" \"add\"")
+ HelpExampleRpc("setban", "\"192.168.0.6\", \"add\", 86400")
},
}.ToString());
};
std::string strCommand;
if (!request.params[1].isNull())
strCommand = request.params[1].get_str();
if (request.fHelp || !help.IsValidNumArgs(request.params.size()) || (strCommand != "add" && strCommand != "remove")) {
throw std::runtime_error(help.ToString());
}
if (!g_banman) {
throw JSONRPCError(RPC_DATABASE_ERROR, "Error: Ban database not loaded");
}

View file

@ -1,4 +1,4 @@
// Copyright (c) 2017-2018 The Bitcoin Core developers
// Copyright (c) 2017-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -314,6 +314,17 @@ std::string RPCExamples::ToDescriptionString() const
return m_examples.empty() ? m_examples : "\nExamples:\n" + m_examples;
}
bool RPCHelpMan::IsValidNumArgs(size_t num_args) const
{
size_t num_required_args = 0;
for (size_t n = m_args.size(); n > 0; --n) {
if (!m_args.at(n - 1).IsOptional()) {
num_required_args = n;
break;
}
}
return num_required_args <= num_args && num_args <= m_args.size();
}
std::string RPCHelpMan::ToString() const
{
std::string ret;
@ -322,12 +333,7 @@ std::string RPCHelpMan::ToString() const
ret += m_name;
bool was_optional{false};
for (const auto& arg : m_args) {
bool optional;
if (arg.m_fallback.which() == 1) {
optional = true;
} else {
optional = RPCArg::Optional::NO != boost::get<RPCArg::Optional>(arg.m_fallback);
}
const bool optional = arg.IsOptional();
ret += " ";
if (optional) {
if (!was_optional) ret += "( ";
@ -369,6 +375,15 @@ std::string RPCHelpMan::ToString() const
return ret;
}
bool RPCArg::IsOptional() const
{
if (m_fallback.which() == 1) {
return true;
} else {
return RPCArg::Optional::NO != boost::get<RPCArg::Optional>(m_fallback);
}
}
std::string RPCArg::ToDescriptionString() const
{
std::string ret;

View file

@ -1,4 +1,4 @@
// Copyright (c) 2017-2018 The Bitcoin Core developers
// Copyright (c) 2017-2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
@ -54,7 +54,7 @@ struct RPCArg {
/** Required arg */
NO,
/**
* Optinal arg that is a named argument and has a default value of
* Optional arg that is a named argument and has a default value of
* `null`. When possible, the default value should be specified.
*/
OMITTED_NAMED_ARG,
@ -111,6 +111,8 @@ struct RPCArg {
assert(type == Type::ARR || type == Type::OBJ);
}
bool IsOptional() const;
/**
* Return the type string of the argument.
* Set oneline to allow it to be overridden by a custom oneline type string (m_oneline_description).
@ -186,6 +188,8 @@ public:
RPCHelpMan(std::string name, std::string description, std::vector<RPCArg> args, RPCResults results, RPCExamples examples);
std::string ToString() const;
/** If the supplied number of args is neither too small nor too high */
bool IsValidNumArgs(size_t num_args) const;
private:
const std::string m_name;

View file

@ -178,5 +178,10 @@ class GetblockstatsTest(BitcoinTestFramework):
assert_raises_rpc_error(-5, 'Block not found', self.nodes[0].getblockstats,
hash_or_height='000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f')
# Invalid number of args
assert_raises_rpc_error(-1, 'getblockstats hash_or_height ( stats )', self.nodes[0].getblockstats, '00', 1, 2)
assert_raises_rpc_error(-1, 'getblockstats hash_or_height ( stats )', self.nodes[0].getblockstats)
if __name__ == '__main__':
GetblockstatsTest().main()