diff options
author | Anthony Towns <aj@erisian.com.au> | 2023-01-13 20:27:59 +1000 |
---|---|---|
committer | Anthony Towns <aj@erisian.com.au> | 2023-01-20 06:24:15 +1000 |
commit | 3d1a4d8a45cd91bdfe0ef107c2e9c5e882b34155 (patch) | |
tree | 1df5a96cea64a12ec64c533cd3ebbfc3c845ccdc /src/rpc | |
parent | f4ef856375c5b295d78169b136c6aee928c19bc9 (diff) |
RPC: make RPCResult::MatchesType return useful errors
Diffstat (limited to 'src/rpc')
-rw-r--r-- | src/rpc/util.cpp | 102 | ||||
-rw-r--r-- | src/rpc/util.h | 6 |
2 files changed, 77 insertions, 31 deletions
diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 53785b1941..9c95e59943 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -2,6 +2,7 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. +#include <clientversion.h> #include <consensus/amount.h> #include <key_io.h> #include <outputtype.h> @@ -581,7 +582,26 @@ UniValue RPCHelpMan::HandleRequest(const JSONRPCRequest& request) const } UniValue ret = m_fun(*this, request); if (gArgs.GetBoolArg("-rpcdoccheck", DEFAULT_RPC_DOC_CHECK)) { - CHECK_NONFATAL(std::any_of(m_results.m_results.begin(), m_results.m_results.end(), [&ret](const RPCResult& res) { return res.MatchesType(ret); })); + UniValue mismatch{UniValue::VARR}; + for (const auto& res : m_results.m_results) { + UniValue match{res.MatchesType(ret)}; + if (match.isTrue()) { + mismatch.setNull(); + break; + } + mismatch.push_back(match); + } + if (!mismatch.isNull()) { + std::string explain{ + mismatch.empty() ? "no possible results defined" : + mismatch.size() == 1 ? mismatch[0].write(4) : + mismatch.write(4)}; + throw std::runtime_error{ + strprintf("Internal bug detected: RPC call \"%s\" returned incorrect type:\n%s\n%s %s\nPlease report this issue here: %s\n", + m_name, explain, + PACKAGE_NAME, FormatFullVersion(), + PACKAGE_BUGREPORT)}; + } } return ret; } @@ -860,53 +880,77 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const NONFATAL_UNREACHABLE(); } -bool RPCResult::MatchesType(const UniValue& result) const +static const std::optional<UniValue::VType> ExpectedType(RPCResult::Type type) { - if (m_skip_type_check) { - return true; - } - switch (m_type) { + using Type = RPCResult::Type; + switch (type) { case Type::ELISION: case Type::ANY: { - return true; + return std::nullopt; } case Type::NONE: { - return UniValue::VNULL == result.getType(); + return UniValue::VNULL; } case Type::STR: case Type::STR_HEX: { - return UniValue::VSTR == result.getType(); + return UniValue::VSTR; } case Type::NUM: case Type::STR_AMOUNT: case Type::NUM_TIME: { - return UniValue::VNUM == result.getType(); + return UniValue::VNUM; } case Type::BOOL: { - return UniValue::VBOOL == result.getType(); + return UniValue::VBOOL; } case Type::ARR_FIXED: case Type::ARR: { - if (UniValue::VARR != result.getType()) return false; + return UniValue::VARR; + } + case Type::OBJ_DYN: + case Type::OBJ: { + return UniValue::VOBJ; + } + } // no default case, so the compiler can warn about missing cases + NONFATAL_UNREACHABLE(); +} + +UniValue RPCResult::MatchesType(const UniValue& result) const +{ + if (m_skip_type_check) { + return true; + } + + const auto exp_type = ExpectedType(m_type); + if (!exp_type) return true; // can be any type, so nothing to check + + if (*exp_type != result.getType()) { + return strprintf("returned type is %s, but declared as %s in doc", uvTypeName(result.getType()), uvTypeName(*exp_type)); + } + + if (UniValue::VARR == result.getType()) { + UniValue errors(UniValue::VOBJ); for (size_t i{0}; i < result.get_array().size(); ++i) { // If there are more results than documented, re-use the last doc_inner. const RPCResult& doc_inner{m_inner.at(std::min(m_inner.size() - 1, i))}; - if (!doc_inner.MatchesType(result.get_array()[i])) return false; + UniValue match{doc_inner.MatchesType(result.get_array()[i])}; + if (!match.isTrue()) errors.pushKV(strprintf("%d", i), match); } - return true; // empty result array is valid + if (errors.empty()) return true; // empty result array is valid + return errors; } - case Type::OBJ_DYN: - case Type::OBJ: { - if (UniValue::VOBJ != result.getType()) return false; + + if (UniValue::VOBJ == result.getType()) { if (!m_inner.empty() && m_inner.at(0).m_type == Type::ELISION) return true; + UniValue errors(UniValue::VOBJ); if (m_type == Type::OBJ_DYN) { const RPCResult& doc_inner{m_inner.at(0)}; // Assume all types are the same, randomly pick the first for (size_t i{0}; i < result.get_obj().size(); ++i) { - if (!doc_inner.MatchesType(result.get_obj()[i])) { - return false; - } + UniValue match{doc_inner.MatchesType(result.get_obj()[i])}; + if (!match.isTrue()) errors.pushKV(result.getKeys()[i], match); } - return true; // empty result obj is valid + if (errors.empty()) return true; // empty result obj is valid + return errors; } std::set<std::string> doc_keys; for (const auto& doc_entry : m_inner) { @@ -916,7 +960,7 @@ bool RPCResult::MatchesType(const UniValue& result) const result.getObjMap(result_obj); for (const auto& result_entry : result_obj) { if (doc_keys.find(result_entry.first) == doc_keys.end()) { - return false; // missing documentation + errors.pushKV(result_entry.first, "key returned that was not in doc"); } } @@ -924,18 +968,18 @@ bool RPCResult::MatchesType(const UniValue& result) const const auto result_it{result_obj.find(doc_entry.m_key_name)}; if (result_it == result_obj.end()) { if (!doc_entry.m_optional) { - return false; // result is missing a required key + errors.pushKV(doc_entry.m_key_name, "key missing, despite not being optional in doc"); } continue; } - if (!doc_entry.MatchesType(result_it->second)) { - return false; // wrong type - } + UniValue match{doc_entry.MatchesType(result_it->second)}; + if (!match.isTrue()) errors.pushKV(doc_entry.m_key_name, match); } - return true; + if (errors.empty()) return true; + return errors; } - } // no default case, so the compiler can warn about missing cases - NONFATAL_UNREACHABLE(); + + return true; } void RPCResult::CheckInnerDoc() const diff --git a/src/rpc/util.h b/src/rpc/util.h index 387fee34d3..b4c83c6cda 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -324,8 +324,10 @@ 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; + /** Check whether the result JSON type matches. + * Returns true if type matches, or object describing error(s) if not. + */ + UniValue MatchesType(const UniValue& result) const; private: void CheckInnerDoc() const; |