Merge bitcoin/bitcoin#23083: rpc: Fail to return undocumented or misdocumented JSON

fc892c3a80 rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke)
f4bc4a705a rpc: Add m_skip_type_check to RPCResult (MarcoFalke)

Pull request description:

  This avoids documentation shortcomings such as the ones fixed in commit e7b6272b30, 138d55e6a0, 577bd51a4b, f8c84e047c, 0ee9a00f90, 13f41855c5, or faecb2ee0a

ACKs for top commit:
  fanquake:
    ACK fc892c3a80 - tested that this catches issue, i.e #24691:

Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
This commit is contained in:
fanquake 2022-03-28 11:51:39 +01:00
commit a13946b822
No known key found for this signature in database
GPG key ID: 2EEB9F5CC09526C1
3 changed files with 57 additions and 10 deletions

View file

@ -774,7 +774,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
// Elements in a JSON structure (dictionary or array) are separated by a comma
const std::string maybe_separator{outer_type != OuterType::NONE ? "," : ""};
// The key name if recursed into an dictionary
// The key name if recursed into a dictionary
const std::string maybe_key{
outer_type == OuterType::OBJ ?
"\"" + this->m_key_name + "\" : " :
@ -865,10 +865,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
bool RPCResult::MatchesType(const UniValue& result) const
{
switch (m_type) {
case Type::ELISION: {
return false;
if (m_skip_type_check) {
return true;
}
switch (m_type) {
case Type::ELISION:
case Type::ANY: {
return true;
}
@ -889,11 +890,52 @@ bool RPCResult::MatchesType(const UniValue& result) const
}
case Type::ARR_FIXED:
case Type::ARR: {
return UniValue::VARR == result.getType();
if (UniValue::VARR != result.getType()) return false;
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;
}
return true; // empty result array is valid
}
case Type::OBJ_DYN:
case Type::OBJ: {
return UniValue::VOBJ == result.getType();
if (UniValue::VOBJ != result.getType()) return false;
if (!m_inner.empty() && m_inner.at(0).m_type == Type::ELISION) return true;
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;
}
}
return true; // empty result obj is valid
}
std::set<std::string> doc_keys;
for (const auto& doc_entry : m_inner) {
doc_keys.insert(doc_entry.m_key_name);
}
std::map<std::string, UniValue> result_obj;
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
}
}
for (const auto& doc_entry : m_inner) {
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
}
continue;
}
if (!doc_entry.MatchesType(result_it->second)) {
return false; // wrong type
}
}
return true;
}
} // no default case, so the compiler can warn about missing cases
CHECK_NONFATAL(false);

View file

@ -256,6 +256,7 @@ struct RPCResult {
const std::string m_key_name; //!< Only used for dicts
const std::vector<RPCResult> m_inner; //!< Only used for arrays or dicts
const bool m_optional;
const bool m_skip_type_check;
const std::string m_description;
const std::string m_cond;
@ -270,6 +271,7 @@ struct RPCResult {
m_key_name{std::move(m_key_name)},
m_inner{std::move(inner)},
m_optional{optional},
m_skip_type_check{false},
m_description{std::move(description)},
m_cond{std::move(cond)}
{
@ -290,11 +292,13 @@ struct RPCResult {
const std::string m_key_name,
const bool optional,
const std::string description,
const std::vector<RPCResult> inner = {})
const std::vector<RPCResult> inner = {},
bool skip_type_check = false)
: m_type{std::move(type)},
m_key_name{std::move(m_key_name)},
m_inner{std::move(inner)},
m_optional{optional},
m_skip_type_check{skip_type_check},
m_description{std::move(description)},
m_cond{}
{
@ -305,8 +309,9 @@ struct RPCResult {
const Type type,
const std::string m_key_name,
const std::string description,
const std::vector<RPCResult> inner = {})
: RPCResult{type, m_key_name, false, description, inner} {}
const std::vector<RPCResult> inner = {},
bool skip_type_check = false)
: RPCResult{type, m_key_name, false, description, inner, skip_type_check} {}
/** Append the sections of the result. */
void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const;

View file

@ -56,7 +56,7 @@ static RPCHelpMan getwalletinfo()
{
{RPCResult::Type::NUM, "duration", "elapsed seconds since scan start"},
{RPCResult::Type::NUM, "progress", "scanning progress percentage [0.0, 1.0]"},
}},
}, /*skip_type_check=*/true},
{RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"},
{RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"},
}},