From 734b9669ff7b2f5e2820993443a6f868f6b0b20a Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 22 Apr 2022 11:09:45 +0200 Subject: [PATCH 1/2] test: add getblockfrompeer coverage of invalid inputs --- test/functional/rpc_getblockfrompeer.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index b65322d920..cc9ccf3066 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -49,14 +49,17 @@ class GetBlockFromPeerTest(BitcoinTestFramework): assert_equal(len(peers), 1) peer_0_peer_1_id = peers[0]["id"] - self.log.info("Arguments must be sensible") - assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", 0) + self.log.info("Arguments must be valid") + assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", peer_0_peer_1_id) + assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getblockfrompeer, 1234, peer_0_peer_1_id) + assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].getblockfrompeer, short_tip, "0") self.log.info("We must already have the header") assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0) self.log.info("Non-existent peer generates error") - assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_0_peer_1_id + 1) + for peer_id in [-1, peer_0_peer_1_id + 1]: + assert_raises_rpc_error(-1, "Peer does not exist", self.nodes[0].getblockfrompeer, short_tip, peer_id) self.log.info("Successful fetch") result = self.nodes[0].getblockfrompeer(short_tip, peer_0_peer_1_id) From 2ef5294a5bb68ceb3797d2638567a172cc21699f Mon Sep 17 00:00:00 2001 From: Jon Atack Date: Fri, 22 Apr 2022 11:11:39 +0200 Subject: [PATCH 2/2] rpc: add RPCTypeCheck for getblockfrompeer inputs --- src/rpc/blockchain.cpp | 5 +++++ test/functional/rpc_getblockfrompeer.py | 4 ++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f46e5e9fef..659e9dcd6a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -439,6 +439,11 @@ static RPCHelpMan getblockfrompeer() }, [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue { + RPCTypeCheck(request.params, { + UniValue::VSTR, // blockhash + UniValue::VNUM, // peer_id + }); + const NodeContext& node = EnsureAnyNodeContext(request.context); ChainstateManager& chainman = EnsureChainman(node); PeerManager& peerman = EnsurePeerman(node); diff --git a/test/functional/rpc_getblockfrompeer.py b/test/functional/rpc_getblockfrompeer.py index cc9ccf3066..a379132db4 100755 --- a/test/functional/rpc_getblockfrompeer.py +++ b/test/functional/rpc_getblockfrompeer.py @@ -51,8 +51,8 @@ class GetBlockFromPeerTest(BitcoinTestFramework): self.log.info("Arguments must be valid") assert_raises_rpc_error(-8, "hash must be of length 64 (not 4, for '1234')", self.nodes[0].getblockfrompeer, "1234", peer_0_peer_1_id) - assert_raises_rpc_error(-1, "JSON value is not a string as expected", self.nodes[0].getblockfrompeer, 1234, peer_0_peer_1_id) - assert_raises_rpc_error(-1, "JSON value is not an integer as expected", self.nodes[0].getblockfrompeer, short_tip, "0") + assert_raises_rpc_error(-3, "Expected type string, got number", self.nodes[0].getblockfrompeer, 1234, peer_0_peer_1_id) + assert_raises_rpc_error(-3, "Expected type number, got string", self.nodes[0].getblockfrompeer, short_tip, "0") self.log.info("We must already have the header") assert_raises_rpc_error(-1, "Block header missing", self.nodes[0].getblockfrompeer, "00" * 32, 0)