From fa05626ca7a0fe896ac554c79eaea4c36acdf861 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Tue, 12 Feb 2019 20:16:29 -0500 Subject: [PATCH 1/2] rpc: Add RPCHelpMan::IsValidNumArgs() --- src/rpc/util.cpp | 29 ++++++++++++++++++++++------- src/rpc/util.h | 8 ++++++-- 2 files changed, 28 insertions(+), 9 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 023b4b674..e37af953f 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -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. @@ -315,6 +315,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; @@ -323,12 +334,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(arg.m_fallback); - } + const bool optional = arg.IsOptional(); ret += " "; if (optional) { if (!was_optional) ret += "( "; @@ -370,6 +376,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(m_fallback); + } +} + std::string RPCArg::ToDescriptionString() const { std::string ret; diff --git a/src/rpc/util.h b/src/rpc/util.h index 1c9ddcdf4..7b8dd5a9e 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -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. @@ -53,7 +53,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, @@ -110,6 +110,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). @@ -185,6 +187,8 @@ public: RPCHelpMan(std::string name, std::string description, std::vector 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; From fa4ce7038d444defe0b98a30097174c278054a33 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Wed, 13 Feb 2019 16:51:27 -0500 Subject: [PATCH 2/2] rpc: Actually throw help when passed invalid number of params --- src/rpc/blockchain.cpp | 10 +++++----- src/rpc/net.cpp | 16 ++++++++-------- test/functional/rpc_getblockstats.py | 5 +++++ 3 files changed, 18 insertions(+), 13 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 7fb9ff2ea..c73a8d48c 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -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); diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 6bbbbc987..c7b3478f4 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -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"); } diff --git a/test/functional/rpc_getblockstats.py b/test/functional/rpc_getblockstats.py index ca9e24367..feba16f1b 100755 --- a/test/functional/rpc_getblockstats.py +++ b/test/functional/rpc_getblockstats.py @@ -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()