Skip to content

Commit 9da6fe0

Browse files
kwvgkwvg
committed
fix: don't treat arrays/objects as string literals for composite methods
We currently do not process them as numbers or other non-string types as this current fix aims to preserve the string representation as expected by RPCs that support extended addresses. A future improvement could allow for full integration but that is currently out of scope.
1 parent 7d301e5 commit 9da6fe0

File tree

4 files changed

+217
-15
lines changed

4 files changed

+217
-15
lines changed

‎src/rpc/client.cpp‎

Lines changed: 78 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,16 +8,17 @@
88
#include <tinyformat.h>
99
#include <util/system.h>
1010

11-
#include <set>
11+
#include <map>
1212
#include <string>
1313
#include <string_view>
1414

1515
class CRPCConvertParam
1616
{
1717
public:
18-
std::string methodName; //!< method whose params want conversion
19-
int paramIdx; //!< 0-based idx of param to convert
20-
std::string paramName; //!< parameter name
18+
std::string methodName; //!< method whose params want conversion
19+
int paramIdx; //!< 0-based idx of param to convert
20+
std::string paramName; //!< parameter name
21+
bool preserve_str{false}; //!< only parse if array or object
2122
};
2223

2324
// clang-format off
@@ -253,36 +254,83 @@ static const CRPCConvertParam vRPCConvertParams[] =
253254
{ "verifyislock", 3, "maxHeight" },
254255
{ "submitchainlock", 2, "blockHeight" },
255256
{ "mnauth", 0, "nodeId" },
257+
{ "protx register", 3, "coreP2PAddrs", true },
258+
{ "protx register_legacy", 3, "coreP2PAddrs", true },
259+
{ "protx register_evo", 3, "coreP2PAddrs", true },
260+
{ "protx register_evo", 10, "platformP2PAddrs", true },
261+
{ "protx register_evo", 11, "platformHTTPSAddrs", true },
262+
{ "protx register_fund", 2, "coreP2PAddrs", true },
263+
{ "protx register_fund_legacy", 2, "coreP2PAddrs", true },
264+
{ "protx register_fund_evo", 2, "coreP2PAddrs", true },
265+
{ "protx register_fund_evo", 9, "platformP2PAddrs", true },
266+
{ "protx register_fund_evo", 10, "platformHTTPSAddrs", true },
267+
{ "protx register_prepare", 3, "coreP2PAddrs", true },
268+
{ "protx register_prepare_legacy", 3, "coreP2PAddrs", true },
269+
{ "protx register_prepare_evo", 3, "coreP2PAddrs", true },
270+
{ "protx register_prepare_evo", 10, "platformP2PAddrs", true },
271+
{ "protx register_prepare_evo", 11, "platformHTTPSAddrs", true },
272+
{ "protx update_service", 2, "coreP2PAddrs", true },
273+
{ "protx update_service_evo", 2, "coreP2PAddrs", true },
274+
{ "protx update_service_evo", 5, "platformP2PAddrs", true },
275+
{ "protx update_service_evo", 6, "platformHTTPSAddrs", true },
256276
};
257277
// clang-format on
258278

259279
class CRPCConvertTable
260280
{
261281
private:
262-
std::set<std::pair<std::string, int>> members;
263-
std::set<std::pair<std::string, std::string>> membersByName;
282+
std::map<std::pair<std::string, int>, bool> members;
283+
std::map<std::pair<std::string, std::string>, bool> membersByName;
284+
285+
std::string_view MaybeUnquoteString(std::string_view arg_value)
286+
{
287+
if (arg_value.size() >= 2 && ((arg_value.front() == '\'' && arg_value.back() == '\'') || (arg_value.front() == '\"' && arg_value.back() == '\"'))) {
288+
return arg_value.substr(1, arg_value.size() - 2);
289+
}
290+
return arg_value;
291+
}
292+
293+
bool LikelyJSONType(std::string_view arg_value)
294+
{
295+
arg_value = MaybeUnquoteString(arg_value);
296+
return arg_value.size() >= 2 && ((arg_value.front() == '[' && arg_value.back() == ']') || (arg_value.front() == '{' && arg_value.back() == '}'));
297+
}
264298

265299
public:
266300
CRPCConvertTable();
267301

268302
/** Return arg_value as UniValue, and first parse it if it is a non-string parameter */
269303
UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, int param_idx)
270304
{
271-
return members.count({method, param_idx}) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value;
305+
if (const auto it = members.find({method, param_idx}); it != members.end() && (!it->second || (it->second && LikelyJSONType(arg_value)))) {
306+
return ParseNonRFCJSONValue(MaybeUnquoteString(arg_value));
307+
}
308+
return arg_value;
272309
}
273310

274311
/** Return arg_value as UniValue, and first parse it if it is a non-string parameter */
275312
UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, const std::string& param_name)
276313
{
277-
return membersByName.count({method, param_name}) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value;
314+
if (const auto it = membersByName.find({method, param_name}); it != membersByName.end() && (!it->second || (it->second && LikelyJSONType(arg_value)))) {
315+
return ParseNonRFCJSONValue(MaybeUnquoteString(arg_value));
316+
}
317+
return arg_value;
318+
}
319+
320+
/** Check if we have any conversion rules for this method */
321+
bool IsDefined(const std::string& method, bool named) const
322+
{
323+
return named ?
324+
std::find_if(membersByName.begin(), membersByName.end(), [&method](const auto& kv) { return kv.first.first == method; }) != membersByName.end()
325+
: std::find_if(members.begin(), members.end(), [&method](const auto& kv) { return kv.first.first == method; }) != members.end();
278326
}
279327
};
280328

281329
CRPCConvertTable::CRPCConvertTable()
282330
{
283331
for (const auto& cp : vRPCConvertParams) {
284-
members.emplace(cp.methodName, cp.paramIdx);
285-
membersByName.emplace(cp.methodName, cp.paramName);
332+
members.try_emplace({cp.methodName, cp.paramIdx}, cp.preserve_str);
333+
membersByName.try_emplace({cp.methodName, cp.paramName}, cp.preserve_str);
286334
}
287335
}
288336

@@ -298,10 +346,19 @@ UniValue ParseNonRFCJSONValue(std::string_view raw)
298346
return parsed;
299347
}
300348

301-
UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::string> &strParams)
349+
UniValue RPCConvertValues(std::string strMethod, const std::vector<std::string> &strParams)
302350
{
303351
UniValue params(UniValue::VARR);
304352

353+
// If we are using a subcommand that is in the table, update the method name
354+
strMethod = [&strMethod, &strParams]() {
355+
if (!strParams.empty() && strMethod.find(' ') == std::string::npos) {
356+
std::string candidate{strMethod + " " + strParams[0]};
357+
return rpcCvtTable.IsDefined(candidate, /*named=*/false) ? candidate : strMethod;
358+
}
359+
return strMethod;
360+
}();
361+
305362
for (unsigned int idx = 0; idx < strParams.size(); idx++) {
306363
std::string_view value{strParams[idx]};
307364
params.push_back(rpcCvtTable.ArgToUniValue(value, strMethod, idx));
@@ -310,11 +367,20 @@ UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::s
310367
return params;
311368
}
312369

313-
UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<std::string> &strParams)
370+
UniValue RPCConvertNamedValues(std::string strMethod, const std::vector<std::string> &strParams)
314371
{
315372
UniValue params(UniValue::VOBJ);
316373
UniValue positional_args{UniValue::VARR};
317374

375+
// If we are using a subcommand that is in the table, update the method name
376+
strMethod = [&strMethod, &strParams]() {
377+
if (strMethod.find(' ') == std::string::npos && !strParams.empty() && strParams[0].find('=') == std::string::npos) {
378+
std::string candidate{strMethod + " " + strParams[0]};
379+
return rpcCvtTable.IsDefined(candidate, /*named=*/true) ? candidate : strMethod;
380+
}
381+
return strMethod;
382+
}();
383+
318384
for (std::string_view s: strParams) {
319385
size_t pos = s.find('=');
320386
if (pos == std::string::npos) {

‎src/rpc/client.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
#include <univalue.h>
1313

1414
/** Convert positional arguments to command-specific RPC representation */
15-
UniValue RPCConvertValues(const std::string& strMethod, const std::vector<std::string>& strParams);
15+
UniValue RPCConvertValues(std::string strMethod, const std::vector<std::string>& strParams);
1616

1717
/** Convert named arguments to command-specific RPC representation */
18-
UniValue RPCConvertNamedValues(const std::string& strMethod, const std::vector<std::string>& strParams);
18+
UniValue RPCConvertNamedValues(std::string strMethod, const std::vector<std::string>& strParams);
1919

2020
/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null)
2121
* as well as objects and arrays.

‎src/test/rpc_tests.cpp‎

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -596,4 +596,138 @@ BOOST_AUTO_TEST_CASE(rpc_bls)
596596
BOOST_CHECK_EQUAL(r.get_obj().find_value("public").get_str(), "b379c28e0f50546906fe733f1222c8f7e39574d513790034f1fec1476286eb652a350c8c0e630cd2cc60d10c26d6f6ee");
597597
}
598598

599+
BOOST_AUTO_TEST_CASE(rpc_convert_composite_commands)
600+
{
601+
UniValue result;
602+
603+
// Validate that array syntax is not interpreted as string literal
604+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
605+
"register_prepare",
606+
"16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3",
607+
"1",
608+
"[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]",
609+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
610+
}));
611+
612+
BOOST_CHECK_EQUAL(result[0].get_str(), "register_prepare");
613+
BOOST_CHECK_EQUAL(result[1].get_str(), "16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3");
614+
BOOST_CHECK_EQUAL(result[2].get_str(), "1");
615+
BOOST_CHECK(result[3].isArray());
616+
BOOST_CHECK_EQUAL(result[3].size(), 2);
617+
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
618+
BOOST_CHECK_EQUAL(result[3][1].get_str(), "1.0.0.1:19999");
619+
BOOST_CHECK_EQUAL(result[4].get_str(), "yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ");
620+
621+
// Validate that array syntax is not interpreted as string literal (named parameter)
622+
BOOST_CHECK_NO_THROW(result = RPCConvertNamedValues("protx", {
623+
"register_prepare",
624+
"16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3",
625+
"1",
626+
"coreP2PAddrs=[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]",
627+
"ownerAddress=yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
628+
}));
629+
630+
BOOST_CHECK(result.exists("coreP2PAddrs"));
631+
BOOST_CHECK(result["coreP2PAddrs"].isArray());
632+
BOOST_CHECK_EQUAL(result["coreP2PAddrs"].size(), 2);
633+
BOOST_CHECK_EQUAL(result["coreP2PAddrs"][0].get_str(), "1.1.1.1:19999");
634+
BOOST_CHECK_EQUAL(result["coreP2PAddrs"][1].get_str(), "1.0.0.1:19999");
635+
BOOST_CHECK_EQUAL(result["ownerAddress"].get_str(), "yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ");
636+
637+
// Validate that array syntax is parsed for all recognized fields
638+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
639+
"register_evo",
640+
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
641+
"1",
642+
"[\"1.1.1.1:19999\"]",
643+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ",
644+
"93746e8731c57f87f79b3620a7982924e2931717d49540a85864bd543de11c43fb868fd63e501a1db37e19ed59ae6db4",
645+
"yTretFTpoi3oQ3maZk5QadGaDWPiKnmDBc",
646+
"0",
647+
"yNbNZyCiTYSFtDwEXt7jChV7tZVYX862ua",
648+
"f2dbd9b0a1f541a7c44d34a58674d0262f5feca5",
649+
"[\"1.1.1.1:22000\"]",
650+
"[\"1.1.1.1:22001\"]",
651+
"yTG8jLL3MvteKXgbEcHyaN7JvTPCejQpSh"
652+
}));
653+
654+
BOOST_CHECK(result[3].isArray());
655+
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
656+
BOOST_CHECK(result[10].isArray());
657+
BOOST_CHECK_EQUAL(result[10][0].get_str(), "1.1.1.1:22000");
658+
BOOST_CHECK(result[11].isArray());
659+
BOOST_CHECK_EQUAL(result[11][0].get_str(), "1.1.1.1:22001");
660+
661+
// Validate that extra quotation doesn't cause string literal interpretation
662+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
663+
"register_prepare",
664+
"16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3",
665+
"1",
666+
"\'[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]\'",
667+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
668+
}));
669+
BOOST_CHECK(result[3].isArray());
670+
BOOST_CHECK_EQUAL(result[3].size(), 2);
671+
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
672+
BOOST_CHECK_EQUAL(result[3][1].get_str(), "1.0.0.1:19999");
673+
674+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
675+
"register_prepare",
676+
"16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3",
677+
"1",
678+
"\"[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]\"",
679+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
680+
}));
681+
BOOST_CHECK(result[3].isArray());
682+
BOOST_CHECK_EQUAL(result[3].size(), 2);
683+
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
684+
BOOST_CHECK_EQUAL(result[3][1].get_str(), "1.0.0.1:19999");
685+
686+
// Validate parsing as string if *not* using array or object syntax
687+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
688+
"register_prepare",
689+
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
690+
"1",
691+
"1.1.1.1:19999",
692+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
693+
}));
694+
695+
BOOST_CHECK(!result[3].isArray());
696+
BOOST_CHECK_EQUAL(result[3].get_str(), "1.1.1.1:19999");
697+
698+
// Empty arrays should be recognized as arrays
699+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
700+
"register_prepare",
701+
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
702+
"1",
703+
"[]",
704+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
705+
}));
706+
BOOST_CHECK(result[3].isArray());
707+
BOOST_CHECK(result[3].empty());
708+
709+
// Incomplete syntax should be interpreted as string
710+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
711+
"register_prepare",
712+
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
713+
"1",
714+
"[",
715+
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
716+
}));
717+
BOOST_CHECK(!result[3].isArray());
718+
BOOST_CHECK_EQUAL(result[3].get_str(), "[");
719+
720+
// Sanity check to ensure that regular commands continue to behave as expected
721+
BOOST_CHECK_NO_THROW(result = RPCConvertValues("getblockstats", {
722+
"1000",
723+
"[\"minfeerate\",\"avgfeerate\"]"
724+
}));
725+
726+
BOOST_CHECK_EQUAL(result[0].getInt<int>(), 1000);
727+
BOOST_CHECK(result[1].isArray());
728+
BOOST_CHECK_EQUAL(result[1].size(), 2);
729+
BOOST_CHECK_EQUAL(result[1][0].get_str(), "minfeerate");
730+
BOOST_CHECK_EQUAL(result[1][1].get_str(), "avgfeerate");
731+
}
732+
599733
BOOST_AUTO_TEST_SUITE_END()

‎test/functional/rpc_help.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ def process_mapping(fname):
3232
if line.startswith('};'):
3333
in_rpcs = False
3434
elif '{' in line and '"' in line:
35-
m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*") *},', line)
35+
m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*")(?:, *(true|false))? *},', line)
3636
assert m, 'No match to table expression: %s' % line
3737
name = parse_string(m.group(1))
3838
idx = int(m.group(2))
@@ -59,6 +59,8 @@ def test_client_conversion_table(self):
5959
mapping_client = process_mapping(file_conversion_table)
6060
# Ignore echojson in client table
6161
mapping_client = [m for m in mapping_client if m[0] != 'echojson']
62+
# Filter out composite commands
63+
mapping_client = [m for m in mapping_client if ' ' not in m[0]]
6264

6365
mapping_server = self.nodes[0].help("dump_all_command_conversions")
6466
# Filter all RPCs whether they need conversion

0 commit comments

Comments
 (0)