aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRyan Ofsky <ryan@ofsky.org>2022-12-02 17:53:58 -0500
committerRyan Ofsky <ryan@ofsky.org>2022-12-02 17:53:58 -0500
commitd1ca56382512df3084fce7353bf1e8b66cae61bc (patch)
treee5884bb502558af0a4ce26989f31fa5f6a2d3461
parent6bd1d20b8cf27aa72ec2907342787e6fc9f94c50 (diff)
bitcoin-cli: Make it an error to specify the "args" parameter two different ways
MarcoFalke reported the case of positional arguments silently overwriting the named "args" parameter in bitcoin-cli https://github.com/bitcoin/bitcoin/pull/19762#discussion_r1035761471 and this behavior is confusing and was not intended when support for "args" parameters was added to bitcoin-cli in #19762. Instead of letting one "args" value overwrite the other in the client, just pass the values to the server verbatim, and let the error be handled server side.
-rw-r--r--src/rpc/client.cpp8
-rwxr-xr-xtest/functional/interface_bitcoin_cli.py2
2 files changed, 8 insertions, 2 deletions
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/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py
index afba8dba85..90a543b51b 100755
--- a/test/functional/interface_bitcoin_cli.py
+++ b/test/functional/interface_bitcoin_cli.py
@@ -92,7 +92,7 @@ class TestBitcoinCli(BitcoinTestFramework):
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_equal(self.nodes[0].cli("-named", "echo", "args=[0,1,2,3]", "4", "5", "6", ).send_cli(), ['4', '5', '6'])
+ 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)