Skip to content

Commit f35e479

Browse files
fanquakevijaydasmp
authored andcommitted
Merge bitcoin#23083: rpc: Fail to return undocumented or misdocumented JSON
fc892c3 rpc: Fail to return undocumented or misdocumented JSON (MarcoFalke) f4bc4a7 rpc: Add m_skip_type_check to RPCResult (MarcoFalke) Pull request description: This avoids documentation shortcomings such as the ones fixed in commit e7b6272, 138d55e, 577bd51, f8c84e0, 0ee9a00, 13f4185, or faecb2e ACKs for top commit: fanquake: ACK fc892c3 - tested that this catches issue, i.e bitcoin#24691: Tree-SHA512: 9d0d7e6291bfc6f67541a4ff746d374ad8751fefcff6d103d8621c0298b190ab1d209ce96cfc3a0d4a6a5460a9f9bb790eb96027b16e5ff91f2512e40c92ca84
1 parent f0a1d7c commit f35e479

File tree

3 files changed

+57
-10
lines changed

3 files changed

+57
-10
lines changed

src/rpc/util.cpp

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
725725
// Elements in a JSON structure (dictionary or array) are separated by a comma
726726
const std::string maybe_separator{outer_type != OuterType::NONE ? "," : ""};
727727

728-
// The key name if recursed into an dictionary
728+
// The key name if recursed into a dictionary
729729
const std::string maybe_key{
730730
outer_type == OuterType::OBJ ?
731731
"\"" + this->m_key_name + "\" : " :
@@ -816,10 +816,11 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const
816816

817817
bool RPCResult::MatchesType(const UniValue& result) const
818818
{
819-
switch (m_type) {
820-
case Type::ELISION: {
821-
return false;
819+
if (m_skip_type_check) {
820+
return true;
822821
}
822+
switch (m_type) {
823+
case Type::ELISION:
823824
case Type::ANY: {
824825
return true;
825826
}
@@ -840,11 +841,52 @@ bool RPCResult::MatchesType(const UniValue& result) const
840841
}
841842
case Type::ARR_FIXED:
842843
case Type::ARR: {
843-
return UniValue::VARR == result.getType();
844+
if (UniValue::VARR != result.getType()) return false;
845+
for (size_t i{0}; i < result.get_array().size(); ++i) {
846+
// If there are more results than documented, re-use the last doc_inner.
847+
const RPCResult& doc_inner{m_inner.at(std::min(m_inner.size() - 1, i))};
848+
if (!doc_inner.MatchesType(result.get_array()[i])) return false;
849+
}
850+
return true; // empty result array is valid
844851
}
845852
case Type::OBJ_DYN:
846853
case Type::OBJ: {
847-
return UniValue::VOBJ == result.getType();
854+
if (UniValue::VOBJ != result.getType()) return false;
855+
if (!m_inner.empty() && m_inner.at(0).m_type == Type::ELISION) return true;
856+
if (m_type == Type::OBJ_DYN) {
857+
const RPCResult& doc_inner{m_inner.at(0)}; // Assume all types are the same, randomly pick the first
858+
for (size_t i{0}; i < result.get_obj().size(); ++i) {
859+
if (!doc_inner.MatchesType(result.get_obj()[i])) {
860+
return false;
861+
}
862+
}
863+
return true; // empty result obj is valid
864+
}
865+
std::set<std::string> doc_keys;
866+
for (const auto& doc_entry : m_inner) {
867+
doc_keys.insert(doc_entry.m_key_name);
868+
}
869+
std::map<std::string, UniValue> result_obj;
870+
result.getObjMap(result_obj);
871+
for (const auto& result_entry : result_obj) {
872+
if (doc_keys.find(result_entry.first) == doc_keys.end()) {
873+
return false; // missing documentation
874+
}
875+
}
876+
877+
for (const auto& doc_entry : m_inner) {
878+
const auto result_it{result_obj.find(doc_entry.m_key_name)};
879+
if (result_it == result_obj.end()) {
880+
if (!doc_entry.m_optional) {
881+
return false; // result is missing a required key
882+
}
883+
continue;
884+
}
885+
if (!doc_entry.MatchesType(result_it->second)) {
886+
return false; // wrong type
887+
}
888+
}
889+
return true;
848890
}
849891
} // no default case, so the compiler can warn about missing cases
850892
NONFATAL_UNREACHABLE();

src/rpc/util.h

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,7 @@ struct RPCResult {
266266
const std::string m_key_name; //!< Only used for dicts
267267
const std::vector<RPCResult> m_inner; //!< Only used for arrays or dicts
268268
const bool m_optional;
269+
const bool m_skip_type_check;
269270
const std::string m_description;
270271
const std::string m_cond;
271272

@@ -280,6 +281,7 @@ struct RPCResult {
280281
m_key_name{std::move(m_key_name)},
281282
m_inner{std::move(inner)},
282283
m_optional{optional},
284+
m_skip_type_check{false},
283285
m_description{std::move(description)},
284286
m_cond{std::move(cond)}
285287
{
@@ -300,11 +302,13 @@ struct RPCResult {
300302
const std::string m_key_name,
301303
const bool optional,
302304
const std::string description,
303-
const std::vector<RPCResult> inner = {})
305+
const std::vector<RPCResult> inner = {},
306+
bool skip_type_check = false)
304307
: m_type{std::move(type)},
305308
m_key_name{std::move(m_key_name)},
306309
m_inner{std::move(inner)},
307310
m_optional{optional},
311+
m_skip_type_check{skip_type_check},
308312
m_description{std::move(description)},
309313
m_cond{}
310314
{
@@ -315,8 +319,9 @@ struct RPCResult {
315319
const Type type,
316320
const std::string m_key_name,
317321
const std::string description,
318-
const std::vector<RPCResult> inner = {})
319-
: RPCResult{type, m_key_name, false, description, inner} {}
322+
const std::vector<RPCResult> inner = {},
323+
bool skip_type_check = false)
324+
: RPCResult{type, m_key_name, false, description, inner, skip_type_check} {}
320325

321326
/** Append the sections of the result. */
322327
void ToSections(Sections& sections, OuterType outer_type = OuterType::NONE, const int current_indent = 0) const;

src/wallet/rpc/wallet.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ static RPCHelpMan getwalletinfo()
189189
{
190190
{RPCResult::Type::NUM, "duration", "elapsed seconds since scan start"},
191191
{RPCResult::Type::NUM, "progress", "scanning progress percentage [0.0, 1.0]"},
192-
}},
192+
}, /*skip_type_check=*/true},
193193
{RPCResult::Type::BOOL, "descriptors", "whether this wallet uses descriptors for scriptPubKey management"},
194194
{RPCResult::Type::BOOL, "external_signer", "whether this wallet is configured to use an external signer such as a hardware wallet"},
195195
RESULT_LAST_PROCESSED_BLOCK,

0 commit comments

Comments
 (0)