diff options
author | fanquake <fanquake@gmail.com> | 2022-12-10 09:54:23 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-12-10 09:58:33 +0000 |
commit | 3b5fb6e77a93f58b3d03b1eec3595f5c45e633a9 (patch) | |
tree | 9b5c23e1609cd7c9ad093c248adf494d46dd1349 | |
parent | 9e229a542ff2107be43eff2e4b992841367f0366 (diff) | |
parent | fa0153e609caf61a59efb0779e754861edc1684d (diff) |
Merge bitcoin/bitcoin#26213: rpc: Strict type checking for RPC boolean parameters
fa0153e609caf61a59efb0779e754861edc1684d refactor: Replace isTrue with get_bool (MarcoFalke)
fa2cc5d1d66aa00e828d1bb65b9923f76fbdf4e1 bugfix: Strict type checking for RPC boolean parameters (MarcoFalke)
Pull request description:
ACKs for top commit:
ryanofsky:
Code review ACK fa0153e609caf61a59efb0779e754861edc1684d
furszy:
Code ACK fa0153e6
Tree-SHA512: b221f823c69d90c94447fd491071ff3659cfd512872b495ebc3e711f50633351974102c9ef7e50fa4a393c4131d349adea8fd41cc9d66f1f31e1f5e7a5f78757
-rw-r--r-- | doc/release-notes-26213.md | 8 | ||||
-rw-r--r-- | src/rpc/net.cpp | 6 | ||||
-rw-r--r-- | src/rpc/rawtransaction.cpp | 4 | ||||
-rw-r--r-- | src/wallet/rpc/spend.cpp | 5 | ||||
-rwxr-xr-x | test/functional/rpc_net.py | 3 |
5 files changed, 16 insertions, 10 deletions
diff --git a/doc/release-notes-26213.md b/doc/release-notes-26213.md new file mode 100644 index 0000000000..e78d718ca9 --- /dev/null +++ b/doc/release-notes-26213.md @@ -0,0 +1,8 @@ +Low-level changes +================= + +- Previously `setban`, `addpeeraddress`, `walletcreatefundedpsbt`, methods + allowed non-boolean and non-null values to be passed as boolean parameters. + Any string, number, array, or object value that was passed would be treated + as false. After this change, passing any value except `true`, `false`, or + `null` now triggers a JSON value is not of expected type error. (#26213) diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp index 8d7f4e7f5b..1a4cd09284 100644 --- a/src/rpc/net.cpp +++ b/src/rpc/net.cpp @@ -731,9 +731,7 @@ static RPCHelpMan setban() if (!request.params[2].isNull()) banTime = request.params[2].getInt<int64_t>(); - bool absolute = false; - if (request.params[3].isTrue()) - absolute = true; + const bool absolute{request.params[3].isNull() ? false : request.params[3].get_bool()}; if (isSubnet) { node.banman->Ban(subNet, banTime, absolute); @@ -942,7 +940,7 @@ static RPCHelpMan addpeeraddress() const std::string& addr_string{request.params[0].get_str()}; const auto port{request.params[1].getInt<uint16_t>()}; - const bool tried{request.params[2].isTrue()}; + const bool tried{request.params[2].isNull() ? false : request.params[2].get_bool()}; UniValue obj(UniValue::VOBJ); CNetAddr net_addr; diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d654de1862..400f2f1507 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -304,7 +304,7 @@ static RPCHelpMan createrawtransaction() std::optional<bool> rbf; if (!request.params[3].isNull()) { - rbf = request.params[3].isTrue(); + rbf = request.params[3].get_bool(); } CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); @@ -1449,7 +1449,7 @@ static RPCHelpMan createpsbt() std::optional<bool> rbf; if (!request.params[3].isNull()) { - rbf = request.params[3].isTrue(); + rbf = request.params[3].get_bool(); } CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); diff --git a/src/wallet/rpc/spend.cpp b/src/wallet/rpc/spend.cpp index 7ab4044bf5..fb5e9a425e 100644 --- a/src/wallet/rpc/spend.cpp +++ b/src/wallet/rpc/spend.cpp @@ -1651,11 +1651,8 @@ RPCHelpMan walletcreatefundedpsbt() CAmount fee; int change_position; - bool rbf{wallet.m_signal_rbf}; const UniValue &replaceable_arg = options["replaceable"]; - if (!replaceable_arg.isNull()) { - rbf = replaceable_arg.isTrue(); - } + const bool rbf{replaceable_arg.isNull() ? wallet.m_signal_rbf : replaceable_arg.get_bool()}; CMutableTransaction rawTx = ConstructTransaction(request.params[0], request.params[1], request.params[2], rbf); CCoinControl coin_control; // Automatically select coins, unless at least one is manually selected. Can diff --git a/test/functional/rpc_net.py b/test/functional/rpc_net.py index 0501befe0f..06e76c4f92 100755 --- a/test/functional/rpc_net.py +++ b/test/functional/rpc_net.py @@ -307,6 +307,9 @@ class NetTest(BitcoinTestFramework): assert_equal(node.addpeeraddress(address="", port=8333), {"success": False}) assert_equal(node.getnodeaddresses(count=0), []) + self.log.debug("Test that non-bool tried fails") + assert_raises_rpc_error(-3, "JSON value of type string is not of expected type bool", self.nodes[0].addpeeraddress, address="1.2.3.4", tried="True", port=1234) + self.log.debug("Test that adding an address with invalid port fails") assert_raises_rpc_error(-1, "JSON integer out of range", self.nodes[0].addpeeraddress, address="1.2.3.4", port=-1) assert_raises_rpc_error(-1, "JSON integer out of range", self.nodes[0].addpeeraddress,address="1.2.3.4", port=65536) |