Additional brother elements validation on advanceBlockchain request p…

…ayload - Added validation - Added and updated tests
rsksmart · Apr 5, 2024 · 302589d · 302589d
1 parent aea9013
commit 302589d
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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,
         )