From 560ad27c1025214d22e7977b317cc50bf7b44ed1 Mon Sep 17 00:00:00 2001 From: Kitsu Date: Tue, 3 Jul 2018 12:38:13 +0300 Subject: [PATCH] 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 --- shared_model/validators/query_validator.hpp | 17 ++++++++++++++--- .../validators/transaction_validator.hpp | 12 ++++++++++++ .../validators/query_validator_test.cpp | 14 ++++++++++++++ .../validators/transaction_validator_test.cpp | 18 +++++++++++++++--- 4 files changed, 55 insertions(+), 6 deletions(-) diff --git a/shared_model/validators/query_validator.hpp b/shared_model/validators/query_validator.hpp index 05e183fb3b..c306cef604 100644 --- a/shared_model/validators/query_validator.hpp +++ b/shared_model/validators/query_validator.hpp @@ -20,7 +20,7 @@ #include -#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(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 index 6e1dcb07d2..522d904643 100644 --- a/shared_model/validators/transaction_validator.hpp +++ b/shared_model/validators/transaction_validator.hpp @@ -21,6 +21,7 @@ #include #include +#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(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 index 772712921e..af5f31fb0f 100644 --- 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 index b421bafb7b..6e37db4aa1 100644 --- 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);