From 2be1604405ffcf06e605bea61572fb209fd043a2 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Sat, 3 Apr 2021 09:25:48 +0200 Subject: [PATCH] Merge #20459: rpc: Fail to return undocumented return values fa8192f42e1d24444f1d0433c96dbce1adf76967 rpc: Fail to return undocumented return values (MarcoFalke) Pull request description: Currently a few return values are undocumented. This is causing confusion at the least. See for example #18476 Fix this by treating it as an internal bug to return undocumented return values. ACKs for top commit: ryanofsky: Code review ACK fa8192f42e1d24444f1d0433c96dbce1adf76967. Only changes: rebase, no const_cast suggestion, and tostring cleanups needed after suggestion Tree-SHA512: c006905639bafe3045de152b00c34d9864731becb3c4f468bdd61a392f10d7e7cd89a54862c8daa8c11ac4eea0eb5f13b0f647d21e21a0a797b54191cff7238c --- src/qt/test/rpcnestedtests.cpp | 2 +- src/rpc/misc.cpp | 2 +- src/rpc/server.cpp | 5 ++-- src/rpc/util.cpp | 42 +++++++++++++++++++++++++++++++++- src/rpc/util.h | 5 +++- 5 files changed, 50 insertions(+), 6 deletions(-) diff --git a/src/qt/test/rpcnestedtests.cpp b/src/qt/test/rpcnestedtests.cpp index 36f4ffc136..bb6e07663a 100644 --- a/src/qt/test/rpcnestedtests.cpp +++ b/src/qt/test/rpcnestedtests.cpp @@ -26,7 +26,7 @@ static RPCHelpMan rpcNestedTest_rpc() {"arg2", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}, {"arg3", RPCArg::Type::STR, RPCArg::Optional::OMITTED, ""}, }, - {}, + RPCResult{RPCResult::Type::ANY, "", ""}, RPCExamples{""}, [](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { return request.params.write(0, 0); diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 0a28cf8862..08f46113da 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -1392,7 +1392,7 @@ static RPCHelpMan echo(const std::string& name) {"arg8", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, {"arg9", RPCArg::Type::STR, RPCArg::Optional::OMITTED_NAMED_ARG, ""}, }, - RPCResult{RPCResult::Type::NONE, "", "Returns whatever was passed in"}, + RPCResult{RPCResult::Type::ANY, "", "Returns whatever was passed in"}, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 73ba4b75c6..eb179b143b 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -171,8 +171,9 @@ static RPCHelpMan help() {"command", RPCArg::Type::STR, /* default */ "all commands", "The command to get help on"}, {"subcommand", RPCArg::Type::STR, /* default */ "all subcommands", "The subcommand to get help on."}, }, - RPCResult{ - RPCResult::Type::STR, "", "The help text" + { + RPCResult{RPCResult::Type::STR, "", "The help text"}, + RPCResult{RPCResult::Type::ANY, "", ""}, }, RPCExamples{""}, [&](const RPCHelpMan& self, const JSONRPCRequest& jsonRequest) -> UniValue diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 265fb438c8..3b02da632f 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -476,6 +476,7 @@ std::string RPCResults::ToDescriptionString() const { std::string result; for (const auto& r : m_results) { + if (r.m_type == RPCResult::Type::ANY) continue; // for testing only if (r.m_cond.empty()) { result += "\nResult:\n"; } else { @@ -493,7 +494,7 @@ std::string RPCExamples::ToDescriptionString() const return m_examples.empty() ? m_examples : "\nExamples:\n" + m_examples; } -UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) +UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const { if (request.mode == JSONRPCRequest::GET_ARGS) { return GetArgMap(); @@ -710,6 +711,9 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const sections.PushSection({indent + "..." + maybe_separator, m_description}); return; } + case Type::ANY: { + CHECK_NONFATAL(false); // Only for testing + } case Type::NONE: { sections.PushSection({indent + "null" + maybe_separator, Description("json null")}); return; @@ -775,6 +779,42 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const CHECK_NONFATAL(false); } +bool RPCResult::MatchesType(const UniValue& result) const +{ + switch (m_type) { + case Type::ELISION: { + return false; + } + case Type::ANY: { + return true; + } + case Type::NONE: { + return UniValue::VNULL == result.getType(); + } + case Type::STR: + case Type::STR_HEX: { + return UniValue::VSTR == result.getType(); + } + case Type::NUM: + case Type::STR_AMOUNT: + case Type::NUM_TIME: { + return UniValue::VNUM == result.getType(); + } + case Type::BOOL: { + return UniValue::VBOOL == result.getType(); + } + case Type::ARR_FIXED: + case Type::ARR: { + return UniValue::VARR == result.getType(); + } + case Type::OBJ_DYN: + case Type::OBJ: { + return UniValue::VOBJ == result.getType(); + } + } // no default case, so the compiler can warn about missing cases + CHECK_NONFATAL(false); +} + std::string RPCArg::ToStringObj(const bool oneline) const { std::string res; diff --git a/src/rpc/util.h b/src/rpc/util.h index ae422dc744..c9192a3a10 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -230,6 +230,7 @@ struct RPCResult { NUM, BOOL, NONE, + ANY, //!< Special type to disable type checks (for testing only) STR_AMOUNT, //!< Special string to represent a floating point amount STR_HEX, //!< Special string with only hex chars OBJ_DYN, //!< Special dictionary with keys that are not literals @@ -302,6 +303,8 @@ struct RPCResult { std::string ToStringObj() const; /** Return the description string, including the result type. */ std::string ToDescriptionString() const; + /** Check whether the result JSON type matches. */ + bool MatchesType(const UniValue& result) const; }; struct RPCResults { @@ -340,7 +343,7 @@ public: using RPCMethodImpl = std::function; RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples, RPCMethodImpl fun); - UniValue HandleRequest(const JSONRPCRequest& request); + UniValue HandleRequest(const JSONRPCRequest& request) const; std::string ToString() const; /** Return the named args that need to be converted from string to another JSON type */ UniValue GetArgMap() const;