More informative error messages for invalid txids

microsoft · Jul 12, 2024 · 8acc527 · 8acc527
1 parent 9c21ea3
commit 8acc527
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 5 deletions.
diff --git a/include/ccf/tx_id.h b/include/ccf/tx_id.h
@@ -42,8 +42,8 @@ namespace ccf
   // transaction executed by CCF.
   struct TxID
   {
-    View view;
-    SeqNo seqno;
+    View view = 0;
+    SeqNo seqno = 0;
 
     std::string to_str() const
     {
@@ -64,7 +64,7 @@ namespace ccf
         const auto view_sv = sv.substr(0, separator_idx);
         const auto [p, ec] =
           std::from_chars(view_sv.begin(), view_sv.end(), tx_id.view);
-        if (ec != std::errc() || p != view_sv.end())
+        if (ec != std::errc() || p != view_sv.end() || tx_id.view < 2)
         {
           return std::nullopt;
         }
@@ -74,7 +74,7 @@ namespace ccf
         const auto seqno_sv = sv.substr(separator_idx + 1);
         const auto [p, ec] =
           std::from_chars(seqno_sv.begin(), seqno_sv.end(), tx_id.seqno);
-        if (ec != std::errc() || p != seqno_sv.end())
+        if (ec != std::errc() || p != seqno_sv.end() || tx_id.seqno < 1)
         {
           return std::nullopt;
         }

diff --git a/tests/e2e_logging.py b/tests/e2e_logging.py
@@ -273,6 +273,23 @@ def send_corrupt_variations(content):
     return network
 
 
+@reqs.description("Test invalid transactions ids in /tx endpoint")
+def test_invalid_txids(network, args):
+    primary, _ = network.find_primary()
+
+    invalid = ["a.b", "-1.-1", "0.0", "1.1", "1.0", "0.1"]
+
+    with primary.client() as c:
+        for txid in invalid:
+            r = c.get(f"/tx?transaction_id={txid}")
+            assert r.status_code == http.HTTPStatus.BAD_REQUEST, r.status_code
+            assert (
+                r.body.json()["error"]["code"] == "InvalidQueryParameterValue"
+            ), r.body.json()
+
+    return network
+
+
 @reqs.description("Alternative protocols")
 @reqs.supports_methods("/log/private", "/log/public")
 @reqs.at_least_n_nodes(2)
@@ -1303,7 +1320,7 @@ def test_view_history(network, args):
             seqno_to_views = {}
             for seqno in range(1, commit_tx_id.seqno + 1):
                 views = []
-                for view in range(1, commit_tx_id.view + 1):
+                for view in range(2, commit_tx_id.view + 1):
                     r = c.get(f"/node/tx?transaction_id={view}.{seqno}", log_capture=[])
                     check(r)
                     status = TxStatus(r.body.json()["status"])
@@ -2056,6 +2073,7 @@ def run_parsing_errors(args):
 
         test_illegal(network, args)
         test_protocols(network, args)
+        test_invalid_txids(network, args)
 
 
 if __name__ == "__main__":