diff options
author | fanquake <fanquake@gmail.com> | 2023-01-25 15:01:45 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-01-25 15:25:49 +0000 |
commit | ab98673f058853e00c310afad57925f54c1ecfae (patch) | |
tree | b406d03c8979cea91f7e136c7bc34b1c6f45b572 /src/rpc | |
parent | 0486148f75547232377d344b37a542cea81023d9 (diff) | |
parent | fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac (diff) |
Merge bitcoin/bitcoin#26929: rpc: Throw more user friendly arg type check error (1.5/2)
fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac rpc: Throw more user friendly arg type check error (MarcoFalke)
Pull request description:
The arg type check error doesn't list which arg (position or name) failed. Fix that.
ACKs for top commit:
stickies-v:
ACK fafeddfe0e6748e9769ad3dd526a6c0eaf6f4aac - although I think the functional test isn't in a logical place (but not blocking)
Tree-SHA512: 17425aa145aab5045940ec74fff28f0e3b2b17ae55f91c4bb5cbcdff0ef13732f8e31621d85964dc2c04333ea37dbe528296ac61be27541384b44e37957555c8
Diffstat (limited to 'src/rpc')
-rw-r--r-- | src/rpc/util.cpp | 60 | ||||
-rw-r--r-- | src/rpc/util.h | 12 |
2 files changed, 39 insertions, 33 deletions
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 12877e94df..a1020c3b2b 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<std::string, UniValueType>& 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<UniValue::VType> 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 @@ -902,7 +910,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const NONFATAL_UNREACHABLE(); } -static const std::optional<UniValue::VType> ExpectedType(RPCResult::Type type) +static std::optional<UniValue::VType> ExpectedType(RPCResult::Type type) { using Type = RPCResult::Type; switch (type) { diff --git a/src/rpc/util.h b/src/rpc/util.h index 49d98c8365..e3783c8f76 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. */ @@ -210,8 +205,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; |