diff options
| -rw-r--r-- | doc/release-notes-26628.md | 4 | ||||
| -rw-r--r-- | src/rpc/client.cpp | 8 | ||||
| -rw-r--r-- | src/rpc/server.cpp | 5 | ||||
| -rw-r--r-- | src/test/rpc_tests.cpp | 8 | ||||
| -rwxr-xr-x | test/functional/interface_bitcoin_cli.py | 4 |
5 files changed, 25 insertions, 4 deletions
diff --git a/doc/release-notes-26628.md b/doc/release-notes-26628.md new file mode 100644 index 0000000000..48a07c1e81 --- /dev/null +++ b/doc/release-notes-26628.md @@ -0,0 +1,4 @@ +JSON-RPC +--- + +The JSON-RPC server now rejects requests where a parameter is specified multiple times with the same name, instead of silently overwriting earlier parameter values with later ones. (#26628) diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp index b3434b80c7..ea094976bf 100644 --- a/src/rpc/client.cpp +++ b/src/rpc/client.cpp @@ -289,6 +289,9 @@ UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<s std::string name = s.substr(0, pos); std::string value = s.substr(pos+1); + // Intentionally overwrite earlier named values with later ones as a + // convenience for scripts and command line users that want to merge + // options. if (!rpcCvtTable.convert(strMethod, name)) { // insert string value directly params.pushKV(name, value); @@ -299,7 +302,10 @@ UniValue RPCConvertNamedValues(const std::string &strMethod, const std::vector<s } if (!positional_args.empty()) { - params.pushKV("args", positional_args); + // Use __pushKV instead of pushKV to avoid overwriting an explicit + // "args" value with an implicit one. Let the RPC server handle the + // request as given. + params.__pushKV("args", positional_args); } return params; diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 80a22ff8ca..a026b7adfa 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -399,7 +399,10 @@ static inline JSONRPCRequest transformNamedArguments(const JSONRPCRequest& in, c const std::vector<UniValue>& values = in.params.getValues(); std::unordered_map<std::string, const UniValue*> argsIn; for (size_t i=0; i<keys.size(); ++i) { - argsIn[keys[i]] = &values[i]; + auto [_, inserted] = argsIn.emplace(keys[i], &values[i]); + if (!inserted) { + throw JSONRPCError(RPC_INVALID_PARAMETER, "Parameter " + keys[i] + " specified multiple times"); + } } // Process expected parameters. If any parameters were left unspecified in // the request before a parameter that was specified, null values need to be diff --git a/src/test/rpc_tests.cpp b/src/test/rpc_tests.cpp index 21ccbe9648..f9b8a47330 100644 --- a/src/test/rpc_tests.cpp +++ b/src/test/rpc_tests.cpp @@ -84,11 +84,15 @@ BOOST_FIXTURE_TEST_SUITE(rpc_tests, RPCTestingSetup) BOOST_AUTO_TEST_CASE(rpc_namedparams) { - const std::vector<std::string> arg_names{{"arg1", "arg2", "arg3", "arg4", "arg5"}}; + const std::vector<std::string> arg_names{"arg1", "arg2", "arg3", "arg4", "arg5"}; // Make sure named arguments are transformed into positional arguments in correct places separated by nulls BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg2": 2, "arg4": 4})"), arg_names).write(), "[null,2,null,4]"); + // Make sure named argument specified multiple times raises an exception + BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"arg2": 2, "arg2": 4})"), arg_names), UniValue, + HasJSON(R"({"code":-8,"message":"Parameter arg2 specified multiple times"})")); + // Make sure named and positional arguments can be combined. BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"arg5": 5, "args": [1, 2], "arg4": 4})"), arg_names).write(), "[1,2,null,4,5]"); @@ -100,7 +104,7 @@ BOOST_AUTO_TEST_CASE(rpc_namedparams) BOOST_CHECK_EXCEPTION(TransformParams(JSON(R"({"args": [1,2,3], "arg4": 4, "arg2": 2})"), arg_names), UniValue, HasJSON(R"({"code":-8,"message":"Parameter arg2 specified twice both as positional and named argument"})")); - // Make sure extra positional arguments can be passed through to the method implemenation, as long as they don't overlap with named arguments. + // Make sure extra positional arguments can be passed through to the method implementation, as long as they don't overlap with named arguments. BOOST_CHECK_EQUAL(TransformParams(JSON(R"({"args": [1,2,3,4,5,6,7,8,9,10]})"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]"); BOOST_CHECK_EQUAL(TransformParams(JSON(R"([1,2,3,4,5,6,7,8,9,10])"), arg_names).write(), "[1,2,3,4,5,6,7,8,9,10]"); } diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index b1369c2615..90a543b51b 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -90,6 +90,10 @@ class TestBitcoinCli(BitcoinTestFramework): assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", self.nodes[0].cli.echo, 0, 1, arg1=1) assert_raises_rpc_error(-8, "Parameter arg1 specified twice both as positional and named argument", self.nodes[0].cli.echo, 0, None, 2, arg1=1) + self.log.info("Test that later cli named arguments values silently overwrite earlier ones") + assert_equal(self.nodes[0].cli("-named", "echo", "arg0=0", "arg1=1", "arg2=2", "arg1=3").send_cli(), ['0', '3', '2']) + assert_raises_rpc_error(-8, "Parameter args specified multiple times", self.nodes[0].cli("-named", "echo", "args=[0,1,2,3]", "4", "5", "6", ).send_cli) + user, password = get_auth_cookie(self.nodes[0].datadir, self.chain) self.log.info("Test -stdinrpcpass option") |