Add oneof case bound check #1502
Conversation
Signed-off-by: Kitsu <[email protected]>
Signed-off-by: Kitsu <[email protected]>
l4l
added
needs-review
| static_cast<const shared_model::proto::Command &>(command) | ||
| .getTransport() | ||
| .command_case(); | ||
| if (cmd_begin > cmd_case || cmd_case >= cmd_end) { |
There was a problem hiding this comment.
Same as in query, getTransport().has_command() is simpler and covers the desired case.
| .getTransport() | ||
| .payload() | ||
| .query_case(); | ||
| if (qry_begin > qry_case || qry_case >= qry_end) { |
There was a problem hiding this comment.
payload().has_query() can be used instead, since it's simpler and covers the case.
Signed-off-by: Kitsu <[email protected]>
Signed-off-by: Kitsu <[email protected]>
|
@kamilsa @lebdron commit 66220fc build status: UNSTABLE. build URL: https://jenkins.soramitsu.co.jp/job/iroha/job/iroha-hyperledger/job/PR-1502/4/ |
Signed-off-by: Kitsu <[email protected]>
|
@kamilsa @lebdron commit 8572628 build status: SUCCESS. build URL: https://jenkins.soramitsu.co.jp/job/iroha/job/iroha-hyperledger/job/PR-1502/5/ |
| */ | ||
| TEST_F(QueryValidatorTest, UnsetQuery) { | ||
| iroha::protocol::Query qry; | ||
| auto *meta = new iroha::protocol::QueryPayloadMeta(); |
There was a problem hiding this comment.
Use mutable_meta instead to avoid new call.
| 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)); | ||
| ASSERT_TRUE(answer.hasErrors()); |
There was a problem hiding this comment.
Answer here is Transaction: [[Transaction should contain at least one command ]], which is not the described case.
There should be an additional call tx.mutable_payload()->add_commands();
Signed-off-by: Kitsu <[email protected]>
| 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()->clear_add_asset_quantity(); |
There was a problem hiding this comment.
After discussion, it seems that clear_add_asset_quantity() is redundant, since add_commands() does not set add_asset_quantity by default, and compiler won't probably optimize any calls. So clear_add_asset_quantity() call can be removed.
Signed-off-by: Kitsu <[email protected]>
|
@kamilsa @lebdron commit 7b40ac4 build status: FAILURE. build URL: https://jenkins.soramitsu.co.jp/job/iroha/job/iroha-hyperledger/job/PR-1502/6/ |
|
@kamilsa @lebdron commit a32cabd build status: FAILURE. build URL: https://jenkins.soramitsu.co.jp/job/iroha/job/iroha-hyperledger/job/PR-1502/7/ |
* Add check for Command variant case * Add check for Query variant case * Add related tests Signed-off-by: Kitsu <[email protected]>
* Add check for Command variant case * Add check for Query variant case * Add related tests Signed-off-by: Kitsu <[email protected]>
* Add check for Command variant case * Add check for Query variant case * Add related tests Signed-off-by: Kitsu <[email protected]>
Description of the Change
Command/Query cases aren't properly validated
Benefits
Lesser bugs
Possible Drawbacks
Validators now require protobuf backend objects but should be fixed with IR-1040