From b7f93a86ed338a16de1473ed4d899909b0f65595 Mon Sep 17 00:00:00 2001 From: Ariel Mendelzon Date: Fri, 5 Apr 2024 13:45:44 +1300 Subject: [PATCH] Additional brother elements validation on advanceBlockchain request payload - Added validation - Added and updated tests --- middleware/comm/protocol.py | 5 +++-- middleware/tests/comm/test_protocol.py | 28 ++++++++++++++++++++---- middleware/tests/ledger/test_protocol.py | 18 +++++++-------- 3 files changed, 36 insertions(+), 15 deletions(-) diff --git a/middleware/comm/protocol.py b/middleware/comm/protocol.py index df635613..41572cd9 100644 --- a/middleware/comm/protocol.py +++ b/middleware/comm/protocol.py @@ -203,9 +203,10 @@ def _validate_advance_blockchain(self, request): "different in length to Blocks field") return self.ERROR_CODE_INVALID_BROTHERS - # Validate brother elements are lists of strings + # Validate brother elements are lists of nonempty hex strings if not all(type(item) == list for item in request["brothers"]) or \ - not all(type(item) == str for brother_list in request["brothers"] + not all(type(item) == str and is_nonempty_hex_string(item) + for brother_list in request["brothers"] for item in brother_list): self.logger.info("Some of the brother list elements are not strings") return self.ERROR_CODE_INVALID_BROTHERS diff --git a/middleware/tests/comm/test_protocol.py b/middleware/tests/comm/test_protocol.py index fd9c7ed0..37724d30 100644 --- a/middleware/tests/comm/test_protocol.py +++ b/middleware/tests/comm/test_protocol.py @@ -1086,7 +1086,7 @@ def test_advance_blockchain_brothers_presence(self): "version": 4, "command": "advanceBlockchain", "blocks": ["ok", "another-ok", "yet-another-ok"], - "brothers": [["b11", "b12"], ["b21", 123], ["b31", "b32", "b33"]], + "brothers": [["bb11", "bb12"], ["bb21", 123], ["bb31", "bb32", "bb33"]], }), {"errorcode": -205}, ) @@ -1096,7 +1096,7 @@ def test_advance_blockchain_brothers_presence(self): "version": 4, "command": "advanceBlockchain", "blocks": ["ok", "another-ok", "yet-another-ok"], - "brothers": [["b11", "b12"], ["b21"]], + "brothers": [["bb11", "bb12"], ["bb21"]], }), {"errorcode": -205}, ) @@ -1106,7 +1106,27 @@ def test_advance_blockchain_brothers_presence(self): "version": 4, "command": "advanceBlockchain", "blocks": ["ok", "another-ok", "yet-another-ok"], - "brothers": [["b11", "b12"], ["b21"], ["b31", "b32", "b33"], []], + "brothers": [["bb11", "bb12"], ["bb21"], ["bb31", "bb32", "bb33"], []], + }), + {"errorcode": -205}, + ) + + self.assertEqual( + self.protocol.handle_request({ + "version": 4, + "command": "advanceBlockchain", + "blocks": ["ok"], + "brothers": [[""]], + }), + {"errorcode": -205}, + ) + + self.assertEqual( + self.protocol.handle_request({ + "version": 4, + "command": "advanceBlockchain", + "blocks": ["ok", "another-ok", "yet-another-ok"], + "brothers": [["bb11", "bb12"], ["bb21"], ["bb31", "bb32", "not-hex"]], }), {"errorcode": -205}, ) @@ -1117,7 +1137,7 @@ def test_advance_blockchain_notimplemented(self): "command": "advanceBlockchain", "version": 4, "blocks": ["fist-block", "second-block"], - "brothers": [["b11", "b12"], ["b21", "b22", "b23"]], + "brothers": [["bb11", "bb12"], ["bb21", "bb22", "bb23"]], }) def test_reset_advance_blockchain_notimplemented(self): diff --git a/middleware/tests/ledger/test_protocol.py b/middleware/tests/ledger/test_protocol.py index c9d09160..e37891ab 100644 --- a/middleware/tests/ledger/test_protocol.py +++ b/middleware/tests/ledger/test_protocol.py @@ -1211,14 +1211,14 @@ def test_advance_blockchain_mapping(self, _, response, expected_code): "version": 4, "command": "advanceBlockchain", "blocks": ["aabbcc", "ddeeff"], - "brothers": [["b11"], ["b21", "b22"]], + "brothers": [["bb11"], ["bb21", "bb22"]], }), ) self.assertEqual( [call( ["aabbcc", "ddeeff"], - [["b11"], ["b21", "b22"]], + [["bb11"], ["bb21", "bb22"]], )], self.dongle.advance_blockchain.call_args_list, ) @@ -1233,14 +1233,14 @@ def test_advance_blockchain_timeout(self): "version": 4, "command": "advanceBlockchain", "blocks": ["aabbcc", "ddeeff"], - "brothers": [["b11", "b12", "b13"], ["b21", "b22"]], + "brothers": [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], }), ) self.assertEqual( [call( ["aabbcc", "ddeeff"], - [["b11", "b12", "b13"], ["b21", "b22"]], + [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], )], self.dongle.advance_blockchain.call_args_list, ) @@ -1255,14 +1255,14 @@ def test_advance_blockchain_commerror_reconnection(self): "version": 4, "command": "advanceBlockchain", "blocks": ["aabbcc", "ddeeff"], - "brothers": [["b11", "b12", "b13"], ["b21", "b22"]], + "brothers": [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], }), ) self.assertEqual( [call( ["aabbcc", "ddeeff"], - [["b11", "b12", "b13"], ["b21", "b22"]], + [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], )], self.dongle.advance_blockchain.call_args_list, ) @@ -1277,7 +1277,7 @@ def test_advance_blockchain_commerror_reconnection(self): "version": 4, "command": "advanceBlockchain", "blocks": ["aabbcc", "ddeeff"], - "brothers": [["b11", "b12", "b13"], ["b21", "b22"]], + "brothers": [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], }), ) @@ -1292,14 +1292,14 @@ def test_advance_blockchain_exception(self): "version": 4, "command": "advanceBlockchain", "blocks": ["aabbcc", "ddeeff"], - "brothers": [["b11", "b12", "b13"], ["b21", "b22"]], + "brothers": [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], }), ) self.assertEqual( [call( ["aabbcc", "ddeeff"], - [["b11", "b12", "b13"], ["b21", "b22"]], + [["bb11", "bb12", "bb13"], ["bb21", "bb22"]], )], self.dongle.advance_blockchain.call_args_list, )