Skip to content

fix: don't treat arrays/objects as string literals for composite methods #6943

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 7, 2025
Merged

fix: don't treat arrays/objects as string literals for composite methods #6943

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 78 additions & 12 deletions src/rpc/client.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,17 @@
#include <tinyformat.h>
#include <util/system.h>

#include <set>
#include <map>
#include <string>
#include <string_view>

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

// clang-format off
Expand Down Expand Up @@ -253,36 +254,83 @@ static const CRPCConvertParam vRPCConvertParams[] =
{ "verifyislock", 3, "maxHeight" },
{ "submitchainlock", 2, "blockHeight" },
{ "mnauth", 0, "nodeId" },
{ "protx register", 3, "coreP2PAddrs", true },
{ "protx register_legacy", 3, "coreP2PAddrs", true },
{ "protx register_evo", 3, "coreP2PAddrs", true },
{ "protx register_evo", 10, "platformP2PAddrs", true },
{ "protx register_evo", 11, "platformHTTPSAddrs", true },
{ "protx register_fund", 2, "coreP2PAddrs", true },
{ "protx register_fund_legacy", 2, "coreP2PAddrs", true },
{ "protx register_fund_evo", 2, "coreP2PAddrs", true },
{ "protx register_fund_evo", 9, "platformP2PAddrs", true },
{ "protx register_fund_evo", 10, "platformHTTPSAddrs", true },
{ "protx register_prepare", 3, "coreP2PAddrs", true },
{ "protx register_prepare_legacy", 3, "coreP2PAddrs", true },
{ "protx register_prepare_evo", 3, "coreP2PAddrs", true },
{ "protx register_prepare_evo", 10, "platformP2PAddrs", true },
{ "protx register_prepare_evo", 11, "platformHTTPSAddrs", true },
{ "protx update_service", 2, "coreP2PAddrs", true },
{ "protx update_service_evo", 2, "coreP2PAddrs", true },
{ "protx update_service_evo", 5, "platformP2PAddrs", true },
{ "protx update_service_evo", 6, "platformHTTPSAddrs", true },
};
// clang-format on

class CRPCConvertTable
{
private:
std::set<std::pair<std::string, int>> members;
std::set<std::pair<std::string, std::string>> membersByName;
std::map<std::pair<std::string, int>, bool> members;
std::map<std::pair<std::string, std::string>, bool> membersByName;

std::string_view MaybeUnquoteString(std::string_view arg_value)
{
if (arg_value.size() >= 2 && ((arg_value.front() == '\'' && arg_value.back() == '\'') || (arg_value.front() == '\"' && arg_value.back() == '\"'))) {
return arg_value.substr(1, arg_value.size() - 2);
}
return arg_value;
}

bool LikelyJSONType(std::string_view arg_value)
{
arg_value = MaybeUnquoteString(arg_value);
return arg_value.size() >= 2 && ((arg_value.front() == '[' && arg_value.back() == ']') || (arg_value.front() == '{' && arg_value.back() == '}'));
}

public:
CRPCConvertTable();

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

/** Return arg_value as UniValue, and first parse it if it is a non-string parameter */
UniValue ArgToUniValue(std::string_view arg_value, const std::string& method, const std::string& param_name)
{
return membersByName.count({method, param_name}) > 0 ? ParseNonRFCJSONValue(arg_value) : arg_value;
if (const auto it = membersByName.find({method, param_name}); it != membersByName.end() && (!it->second || (it->second && LikelyJSONType(arg_value)))) {
return ParseNonRFCJSONValue(MaybeUnquoteString(arg_value));
}
return arg_value;
}

/** Check if we have any conversion rules for this method */
bool IsDefined(const std::string& method, bool named) const
{
return named ?
std::find_if(membersByName.begin(), membersByName.end(), [&method](const auto& kv) { return kv.first.first == method; }) != membersByName.end()
: std::find_if(members.begin(), members.end(), [&method](const auto& kv) { return kv.first.first == method; }) != members.end();
}
};

CRPCConvertTable::CRPCConvertTable()
{
for (const auto& cp : vRPCConvertParams) {
members.emplace(cp.methodName, cp.paramIdx);
membersByName.emplace(cp.methodName, cp.paramName);
members.try_emplace({cp.methodName, cp.paramIdx}, cp.preserve_str);
membersByName.try_emplace({cp.methodName, cp.paramName}, cp.preserve_str);
}
}

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

UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::string> &strParams)
UniValue RPCConvertValues(std::string strMethod, const std::vector<std::string> &strParams)
{
UniValue params(UniValue::VARR);

// If we are using a subcommand that is in the table, update the method name
strMethod = [&strMethod, &strParams]() {
if (!strParams.empty() && strMethod.find(' ') == std::string::npos) {
std::string candidate{strMethod + " " + strParams[0]};
return rpcCvtTable.IsDefined(candidate, /*named=*/false) ? candidate : strMethod;
}
return strMethod;
}();

for (unsigned int idx = 0; idx < strParams.size(); idx++) {
std::string_view value{strParams[idx]};
params.push_back(rpcCvtTable.ArgToUniValue(value, strMethod, idx));
Expand All @@ -310,11 +367,20 @@ UniValue RPCConvertValues(const std::string &strMethod, const std::vector<std::s
return params;
}

UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<std::string> &strParams)
UniValue RPCConvertNamedValues(std::string strMethod, const std::vector<std::string> &strParams)
{
UniValue params(UniValue::VOBJ);
UniValue positional_args{UniValue::VARR};

// If we are using a subcommand that is in the table, update the method name
strMethod = [&strMethod, &strParams]() {
if (strMethod.find(' ') == std::string::npos && !strParams.empty() && strParams[0].find('=') == std::string::npos) {
std::string candidate{strMethod + " " + strParams[0]};
return rpcCvtTable.IsDefined(candidate, /*named=*/true) ? candidate : strMethod;
}
return strMethod;
}();

for (std::string_view s: strParams) {
size_t pos = s.find('=');
if (pos == std::string::npos) {
Expand Down
4 changes: 2 additions & 2 deletions src/rpc/client.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
#include <univalue.h>

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

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

/** Non-RFC4627 JSON parser, accepts internal values (such as numbers, true, false, null)
* as well as objects and arrays.
Expand Down
134 changes: 134 additions & 0 deletions src/test/rpc_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,4 +596,138 @@ BOOST_AUTO_TEST_CASE(rpc_bls)
BOOST_CHECK_EQUAL(r.get_obj().find_value("public").get_str(), "b379c28e0f50546906fe733f1222c8f7e39574d513790034f1fec1476286eb652a350c8c0e630cd2cc60d10c26d6f6ee");
}

BOOST_AUTO_TEST_CASE(rpc_convert_composite_commands)
{
UniValue result;

// Validate that array syntax is not interpreted as string literal
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));

BOOST_CHECK_EQUAL(result[0].get_str(), "register_prepare");
BOOST_CHECK_EQUAL(result[1].get_str(), "6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000");
BOOST_CHECK_EQUAL(result[2].get_str(), "1");
BOOST_CHECK(result[3].isArray());
BOOST_CHECK_EQUAL(result[3].size(), 2);
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
BOOST_CHECK_EQUAL(result[3][1].get_str(), "1.0.0.1:19999");
BOOST_CHECK_EQUAL(result[4].get_str(), "yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ");

// Validate that array syntax is not interpreted as string literal (named parameter)
BOOST_CHECK_NO_THROW(result = RPCConvertNamedValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"coreP2PAddrs=[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]",
"ownerAddress=yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));

BOOST_CHECK(result.exists("coreP2PAddrs"));
BOOST_CHECK(result["coreP2PAddrs"].isArray());
BOOST_CHECK_EQUAL(result["coreP2PAddrs"].size(), 2);
BOOST_CHECK_EQUAL(result["coreP2PAddrs"][0].get_str(), "1.1.1.1:19999");
BOOST_CHECK_EQUAL(result["coreP2PAddrs"][1].get_str(), "1.0.0.1:19999");
BOOST_CHECK_EQUAL(result["ownerAddress"].get_str(), "yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ");

// Validate that array syntax is parsed for all recognized fields
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_evo",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"[\"1.1.1.1:19999\"]",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ",
"93746e8731c57f87f79b3620a7982924e2931717d49540a85864bd543de11c43fb868fd63e501a1db37e19ed59ae6db4",
"yTretFTpoi3oQ3maZk5QadGaDWPiKnmDBc",
"0",
"yNbNZyCiTYSFtDwEXt7jChV7tZVYX862ua",
"f2dbd9b0a1f541a7c44d34a58674d0262f5feca5",
"[\"1.1.1.1:22000\"]",
"[\"1.1.1.1:22001\"]",
"yTG8jLL3MvteKXgbEcHyaN7JvTPCejQpSh"
}));

BOOST_CHECK(result[3].isArray());
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
BOOST_CHECK(result[10].isArray());
BOOST_CHECK_EQUAL(result[10][0].get_str(), "1.1.1.1:22000");
BOOST_CHECK(result[11].isArray());
BOOST_CHECK_EQUAL(result[11][0].get_str(), "1.1.1.1:22001");

// Validate that extra quotation doesn't cause string literal interpretation
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"\'[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]\'",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));
BOOST_CHECK(result[3].isArray());
BOOST_CHECK_EQUAL(result[3].size(), 2);
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
BOOST_CHECK_EQUAL(result[3][1].get_str(), "1.0.0.1:19999");

BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"\"[\"1.1.1.1:19999\",\"1.0.0.1:19999\"]\"",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));
BOOST_CHECK(result[3].isArray());
BOOST_CHECK_EQUAL(result[3].size(), 2);
BOOST_CHECK_EQUAL(result[3][0].get_str(), "1.1.1.1:19999");
BOOST_CHECK_EQUAL(result[3][1].get_str(), "1.0.0.1:19999");

// Validate parsing as string if *not* using array or object syntax
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"1.1.1.1:19999",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));

BOOST_CHECK(!result[3].isArray());
BOOST_CHECK_EQUAL(result[3].get_str(), "1.1.1.1:19999");

// Empty arrays should be recognized as arrays
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"[]",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));
BOOST_CHECK(result[3].isArray());
BOOST_CHECK(result[3].empty());

// Incomplete syntax should be interpreted as string
BOOST_CHECK_NO_THROW(result = RPCConvertValues("protx", {
"register_prepare",
"6fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d6190000000000",
"1",
"[",
"yhq7ifNCtTKEpY4Yu5XPCcztQco6Fh6JsZ"
}));
BOOST_CHECK(!result[3].isArray());
BOOST_CHECK_EQUAL(result[3].get_str(), "[");

// Sanity check to ensure that regular commands continue to behave as expected
BOOST_CHECK_NO_THROW(result = RPCConvertValues("getblockstats", {
"1000",
"[\"minfeerate\",\"avgfeerate\"]"
}));

BOOST_CHECK_EQUAL(result[0].getInt<int>(), 1000);
BOOST_CHECK(result[1].isArray());
BOOST_CHECK_EQUAL(result[1].size(), 2);
BOOST_CHECK_EQUAL(result[1][0].get_str(), "minfeerate");
BOOST_CHECK_EQUAL(result[1][1].get_str(), "avgfeerate");
}

BOOST_AUTO_TEST_SUITE_END()
4 changes: 3 additions & 1 deletion test/functional/rpc_help.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def process_mapping(fname):
if line.startswith('};'):
in_rpcs = False
elif '{' in line and '"' in line:
m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*") *},', line)
m = re.search(r'{ *("[^"]*"), *([0-9]+) *, *("[^"]*")(?:, *(true|false))? *},', line)
assert m, 'No match to table expression: %s' % line
name = parse_string(m.group(1))
idx = int(m.group(2))
Expand All @@ -59,6 +59,8 @@ def test_client_conversion_table(self):
mapping_client = process_mapping(file_conversion_table)
# Ignore echojson in client table
mapping_client = [m for m in mapping_client if m[0] != 'echojson']
# Filter out composite commands
mapping_client = [m for m in mapping_client if ' ' not in m[0]]

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