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

kwvg · kwvg · commit d4e9a0bf85e8 · 2025-11-05T23:39:54.000+05:30
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.
diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
@@ -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
@@ -253,36 +254,74 @@ 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;
+
+    bool LikelyJSONType(std::string_view 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(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(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);
     }
 }
 
@@ -298,10 +337,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));
@@ -310,11 +358,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) {
diff --git a/src/rpc/client.h b/src/rpc/client.h
@@ -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.
diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp
@@ -596,4 +596,113 @@ 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",
+        "16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3",
+        "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(), "16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3");
+    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",
+        "16347a28f4e5edf39f4dceac60e2327931a25fdee1fb4b94b63eeacf0d5879e3",
+        "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 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()
diff --git a/test/functional/rpc_help.py b/test/functional/rpc_help.py
@@ -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))
@@ -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
