Add oneof case bound check (#1502)

* Add check for Command variant case * Add check for Query variant case * Add related tests Signed-off-by: Kitsu <[email protected]>
hyperledger-iroha · Jul 25, 2018 · 560ad27 · 560ad27
1 parent bf22e01
commit 560ad27
Show file tree

Hide file tree

Showing 4 changed files with 55 additions and 6 deletions.
diff --git a/shared_model/validators/query_validator.hpp b/shared_model/validators/query_validator.hpp
@@ -20,7 +20,7 @@
 
 #include <boost/variant/static_visitor.hpp>
 
-#include "interfaces/queries/query.hpp"
+#include "backend/protobuf/queries/proto_query.hpp"
 #include "validators/answer.hpp"
 
 namespace shared_model {
@@ -174,9 +174,20 @@ namespace shared_model {
           answer.addReason(std::move(qry_reason));
         }
 
-        auto reason = boost::apply_visitor(query_field_validator_, qry.get());
-        if (not reason.second.empty()) {
+        auto qry_case = static_cast<const shared_model::proto::Query &>(qry)
+                            .getTransport()
+                            .payload()
+                            .query_case();
+        if (iroha::protocol::Query_Payload::QUERY_NOT_SET == qry_case) {
+          ReasonsGroupType reason;
+          reason.first = "Undefined";
+          reason.second.push_back("query is undefined");
           answer.addReason(std::move(reason));
+        } else {
+          auto reason = boost::apply_visitor(query_field_validator_, qry.get());
+          if (not reason.second.empty()) {
+            answer.addReason(std::move(reason));
+          }
         }
 
         return answer;

diff --git a/shared_model/validators/transaction_validator.hpp b/shared_model/validators/transaction_validator.hpp
@@ -21,6 +21,7 @@
 #include <boost/format.hpp>
 #include <boost/variant/static_visitor.hpp>
 
+#include "backend/protobuf/commands/proto_command.hpp"
 #include "backend/protobuf/permissions.hpp"
 #include "backend/protobuf/transaction.hpp"
 #include "validators/answer.hpp"
@@ -271,6 +272,17 @@ namespace shared_model {
         }
 
         for (const auto &command : tx.commands()) {
+          auto cmd_case =
+              static_cast<const shared_model::proto::Command &>(command)
+                  .getTransport()
+                  .command_case();
+          if (iroha::protocol::Command::COMMAND_NOT_SET == cmd_case) {
+            ReasonsGroupType reason;
+            reason.first = "Undefined";
+            reason.second.push_back("command is undefined");
+            answer.addReason(std::move(reason));
+            continue;
+          }
           auto reason = boost::apply_visitor(command_validator_, command.get());
           if (not reason.second.empty()) {
             answer.addReason(std::move(reason));

diff --git a/test/module/shared_model/validators/query_validator_test.cpp b/test/module/shared_model/validators/query_validator_test.cpp
@@ -66,6 +66,20 @@ TEST_F(QueryValidatorTest, StatelessValidTest) {
       });
 }
 
+/**
+ * @given Protobuf query object with unset query
+ * @when validate is called
+ * @then there is a error returned
+ */
+TEST_F(QueryValidatorTest, UnsetQuery) {
+  iroha::protocol::Query qry;
+  qry.mutable_payload()->mutable_meta()->set_created_time(created_time);
+  qry.mutable_payload()->mutable_meta()->set_creator_account_id(account_id);
+  qry.mutable_payload()->mutable_meta()->set_query_counter(counter);
+  auto answer = query_validator.validate(proto::Query(qry));
+  ASSERT_TRUE(answer.hasErrors());
+}
+
 /**
  * @given Protobuf query object
  * @when Query has no fields set, and each query type has no fields set

diff --git a/test/module/shared_model/validators/transaction_validator_test.cpp b/test/module/shared_model/validators/transaction_validator_test.cpp
@@ -39,6 +39,7 @@ class TransactionValidatorTest : public ValidatorsTest {
                   .getTransport();
     return tx;
   }
+  shared_model::validation::DefaultTransactionValidator transaction_validator;
 };
 
 /**
@@ -49,7 +50,6 @@ class TransactionValidatorTest : public ValidatorsTest {
 TEST_F(TransactionValidatorTest, EmptyTransactionTest) {
   auto tx = generateEmptyTransaction();
   tx.mutable_payload()->set_created_time(created_time);
-  shared_model::validation::DefaultTransactionValidator transaction_validator;
   auto result = proto::Transaction(iroha::protocol::Transaction(tx));
   auto answer = transaction_validator.validate(result);
   ASSERT_EQ(answer.getReasonsMap().size(), 1);
@@ -101,13 +101,26 @@ TEST_F(TransactionValidatorTest, StatelessValidTest) {
                    },
                    [] {});
 
-  shared_model::validation::DefaultTransactionValidator transaction_validator;
   auto result = proto::Transaction(iroha::protocol::Transaction(tx));
   auto answer = transaction_validator.validate(result);
 
   ASSERT_FALSE(answer.hasErrors()) << answer.reason();
 }
 
+/**
+ * @given Protobuf transaction object with unset command
+ * @when validate is called
+ * @then there is a error returned
+ */
+TEST_F(TransactionValidatorTest, UnsetCommand) {
+  iroha::protocol::Transaction tx = generateEmptyTransaction();
+  tx.mutable_payload()->set_creator_account_id(account_id);
+  tx.mutable_payload()->set_created_time(created_time);
+  auto answer = transaction_validator.validate(proto::Transaction(tx));
+  tx.mutable_payload()->add_commands();
+  ASSERT_TRUE(answer.hasErrors());
+}
+
 /**
  * @given transaction made of commands with invalid fields
  * @when commands validation is invoked
@@ -136,7 +149,6 @@ TEST_F(TransactionValidatorTest, StatelessInvalidTest) {
                    },
                    [] {});
 
-  shared_model::validation::DefaultTransactionValidator transaction_validator;
   auto result = proto::Transaction(iroha::protocol::Transaction(tx));
   auto answer = transaction_validator.validate(result);