From fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 20 Jan 2023 12:50:25 +0100 Subject: rpc: Throw more user friendly arg type check error --- src/rpc/util.cpp | 60 ++++++++++++++++++++++----------------- src/rpc/util.h | 12 ++++---- test/functional/rpc_blockchain.py | 12 ++++++++ 3 files changed, 51 insertions(+), 33 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 9c8b0b5642..9c29387dde 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -31,14 +31,6 @@ std::string GetAllOutputTypes() return Join(ret, ", "); } -void RPCTypeCheckArgument(const UniValue& value, const UniValueType& typeExpected) -{ - if (!typeExpected.typeAny && value.type() != typeExpected.type) { - throw JSONRPCError(RPC_TYPE_ERROR, - strprintf("JSON value of type %s is not of expected type %s", uvTypeName(value.type()), uvTypeName(typeExpected.type))); - } -} - void RPCTypeCheckObj(const UniValue& o, const std::map& typesExpected, bool fAllowNull, @@ -564,8 +556,16 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const if (request.mode == JSONRPCRequest::GET_HELP || !IsValidNumArgs(request.params.size())) { throw std::runtime_error(ToString()); } + UniValue arg_mismatch{UniValue::VOBJ}; for (size_t i{0}; i < m_args.size(); ++i) { - m_args.at(i).MatchesType(request.params[i]); + const auto& arg{m_args.at(i)}; + UniValue match{arg.MatchesType(request.params[i])}; + if (!match.isTrue()) { + arg_mismatch.pushKV(strprintf("Position %s (%s)", i + 1, arg.m_names), std::move(match)); + } + } + if (!arg_mismatch.empty()) { + throw JSONRPCError(RPC_TYPE_ERROR, strprintf("Wrong type passed:\n%s", arg_mismatch.write(4))); } UniValue ret = m_fun(*this, request); if (gArgs.GetBoolArg("-rpcdoccheck", DEFAULT_RPC_DOC_CHECK)) { @@ -684,42 +684,50 @@ UniValue RPCHelpMan::GetArgMap() const return arr; } -void RPCArg::MatchesType(const UniValue& request) const +static std::optional ExpectedType(RPCArg::Type type) { - if (m_opts.skip_type_check) return; - if (IsOptional() && request.isNull()) return; - switch (m_type) { + using Type = RPCArg::Type; + switch (type) { case Type::STR_HEX: case Type::STR: { - RPCTypeCheckArgument(request, UniValue::VSTR); - return; + return UniValue::VSTR; } case Type::NUM: { - RPCTypeCheckArgument(request, UniValue::VNUM); - return; + return UniValue::VNUM; } case Type::AMOUNT: { // VNUM or VSTR, checked inside AmountFromValue() - return; + return std::nullopt; } case Type::RANGE: { // VNUM or VARR, checked inside ParseRange() - return; + return std::nullopt; } case Type::BOOL: { - RPCTypeCheckArgument(request, UniValue::VBOOL); - return; + return UniValue::VBOOL; } case Type::OBJ: case Type::OBJ_USER_KEYS: { - RPCTypeCheckArgument(request, UniValue::VOBJ); - return; + return UniValue::VOBJ; } case Type::ARR: { - RPCTypeCheckArgument(request, UniValue::VARR); - return; + return UniValue::VARR; } } // no default case, so the compiler can warn about missing cases + NONFATAL_UNREACHABLE(); +} + +UniValue RPCArg::MatchesType(const UniValue& request) const +{ + if (m_opts.skip_type_check) return true; + if (IsOptional() && request.isNull()) return true; + const auto exp_type{ExpectedType(m_type)}; + if (!exp_type) return true; // nothing to check + + if (*exp_type != request.getType()) { + return strprintf("JSON value of type %s is not of expected type %s", uvTypeName(request.getType()), uvTypeName(*exp_type)); + } + return true; } std::string RPCArg::GetFirstName() const @@ -903,7 +911,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const NONFATAL_UNREACHABLE(); } -static const std::optional ExpectedType(RPCResult::Type type) +static std::optional ExpectedType(RPCResult::Type type) { using Type = RPCResult::Type; switch (type) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 27363eeb50..0d60e29fa4 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -62,11 +62,6 @@ struct UniValueType { UniValue::VType type; }; -/** - * Type-check one argument; throws JSONRPCError if wrong type given. - */ -void RPCTypeCheckArgument(const UniValue& value, const UniValueType& typeExpected); - /* Check for expected keys/value types in an Object. */ @@ -214,8 +209,11 @@ struct RPCArg { bool IsOptional() const; - /** Check whether the request JSON type matches. */ - void MatchesType(const UniValue& request) const; + /** + * Check whether the request JSON type matches. + * Returns true if type matches, or object describing error(s) if not. + */ + UniValue MatchesType(const UniValue& request) const; /** Return the first of all aliases */ std::string GetFirstName() const; diff --git a/test/functional/rpc_blockchain.py b/test/functional/rpc_blockchain.py index 19c73eebf0..7a0cedb1f5 100755 --- a/test/functional/rpc_blockchain.py +++ b/test/functional/rpc_blockchain.py @@ -25,6 +25,7 @@ from decimal import Decimal import http.client import os import subprocess +import textwrap from test_framework.blocktools import ( MAX_FUTURE_BLOCK_TIME, @@ -429,6 +430,17 @@ class BlockchainTest(BitcoinTestFramework): def _test_getnetworkhashps(self): self.log.info("Test getnetworkhashps") hashes_per_second = self.nodes[0].getnetworkhashps() + assert_raises_rpc_error( + -3, + textwrap.dedent(""" + Wrong type passed: + { + "Position 1 (nblocks)": "JSON value of type string is not of expected type number", + "Position 2 (height)": "JSON value of type array is not of expected type number" + } + """).strip(), + lambda: self.nodes[0].getnetworkhashps("a", []), + ) # This should be 2 hashes every 10 minutes or 1/300 assert abs(hashes_per_second * 300 - 1) < 0.0001 -- cgit v1.2.3