Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UMessage: validator and unit test #158

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions include/up-cpp/datamodel/validator/UMessage.h
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,9 @@ using ValidationResult = std::tuple<bool, std::optional<Reason>>;
/// @brief Checks if UMessage is valid for sending a notification
///
/// In addition to all common attributes being valid, these checks must pass:
/// * Message type must be UMESSAGE_TYPE_PUBLISH
/// * Message source must pass uri::isValidTopic()
/// * Message sink must pass uri::isValidTopic()
/// * Message type must be UMESSAGE_TYPE_NOTIFICATION
/// * Message source must pass uri::isValidNotification()
/// * Message sink must pass uri::isValidNotification()
/// * Message must not set commstatus
/// * Message must not set reqid
/// * Message must not set permission_level
Expand Down
342 changes: 342 additions & 0 deletions src/datamodel/validator/UMessage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,345 @@
// SPDX-License-Identifier: Apache-2.0

#include "up-cpp/datamodel/validator/UMessage.h"

#include <google/protobuf/util/message_differencer.h>

#include "up-cpp/datamodel/validator/UUri.h"
#include "up-cpp/datamodel/validator/Uuid.h"

namespace uprotocol::datamodel::validator::message {

using namespace uprotocol::v1;
using namespace uprotocol::datamodel::validator;

std::string_view message(Reason reason) {
switch (reason) {
case Reason::BAD_ID:
return "The ID does not pass UUID validity checks";
case Reason::ID_EXPIRED:
return "The TTL, if present, indicates the ID has expired";
case Reason::PRIORITY_OUT_OF_RANGE:
return "The Priority, if set, is not within the allowable range";
Comment on lines +29 to +31
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't these errors only be returned when the field is present?

Suggested change
return "The TTL, if present, indicates the ID has expired";
case Reason::PRIORITY_OUT_OF_RANGE:
return "The Priority, if set, is not within the allowable range";
return "The TTL indicates the ID has expired";
case Reason::PRIORITY_OUT_OF_RANGE:
return "The Priority is not within the allowable range";

case Reason::PAYLOAD_FORMAT_OUT_OF_RANGE:
return "The Payload Format is not within the allowable range";
case Reason::WRONG_MESSAGE_TYPE:
return "The type set in the message is incorrect for the validated "
"mode";
case Reason::BAD_SOURCE_URI:
return "Source URI did not pass validity checks";
case Reason::BAD_SINK_URI:
return "Sink URI did not pass validity checks";
case Reason::INVALID_TTL:
return "TTL is set to an invalid value (e.g. 0)";
case Reason::DISALLOWED_FIELD_SET:
return "A field was set that is not allowed for the validated mode";
case Reason::REQID_MISMATCH:
return "The Request ID did not match the ID of the request message";
case Reason::PRIORITY_MISMATCH:
return "The Priority did not match the Priority of the request "
"message";
default:
return "Unknown reason.";
}
}

ValidationResult isValid(const v1::UMessage& umessage) {
{
auto [valid, reason] = isValidRpcRequest(umessage);
if (valid) {
return {true, std::nullopt};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: I'm pretty sure you can just do return {true, {}}, but I haven't tried it. It's not a big deal either way.

}
}

{
auto [valid, reason] = isValidRpcResponse(umessage);
if (valid) {
return {true, std::nullopt};
}
}

{
auto [valid, reason] = isValidPublish(umessage);
if (valid) {
return {true, std::nullopt};
}
}

return isValidNotification(umessage);
}

ValidationResult areCommonAttributesValid(const v1::UMessage& umessage) {
auto [valid, reason] = uuid::isUuid(umessage.attributes().id());
if (!valid) {
return {false, Reason::BAD_ID};
}

if (umessage.attributes().has_ttl() && (umessage.attributes().ttl() > 0)) {
auto [expired, reason] = uuid::isExpired(
umessage.attributes().id(),
std::chrono::milliseconds(umessage.attributes().ttl()));
if (expired) {
return {false, Reason::ID_EXPIRED};
}
}

if (!UPriority_IsValid(umessage.attributes().priority())) {
return {false, Reason::PRIORITY_OUT_OF_RANGE};
}

if (!UPayloadFormat_IsValid(umessage.attributes().payload_format())) {
return {false, Reason::PAYLOAD_FORMAT_OUT_OF_RANGE};
}
Comment on lines +95 to +101
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😮 neat! I assume this interface is provided by protobuf for checking that enums are within range? That might be useful in #146 and #161

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for suggestion. I have now used this in #161


return {true, std::nullopt};
}

ValidationResult isValidRpcRequest(const v1::UMessage& umessage) {
auto [valid, reason] = areCommonAttributesValid(umessage);
if (!valid) {
return {false, reason};
}

if (umessage.attributes().type() !=
v1::UMessageType::UMESSAGE_TYPE_REQUEST) {
return {false, Reason::WRONG_MESSAGE_TYPE};
}

if (!umessage.attributes().has_source()) {
return {false, Reason::BAD_SOURCE_URI};
}

if (!umessage.attributes().has_sink()) {
return {false, Reason::BAD_SINK_URI};
}

{
auto [valid, reason] =
uri::isValidRpcResponse(umessage.attributes().source());
if (!valid) {
return {false, Reason::BAD_SOURCE_URI};
}
}

{
auto [valid, reason] =
uri::isValidRpcMethod(umessage.attributes().sink());
if (!valid) {
return {false, Reason::BAD_SINK_URI};
}
}

if (umessage.attributes().priority() < UPRIORITY_CS4) {
return {false, Reason::PRIORITY_OUT_OF_RANGE};
}

if ((!umessage.attributes().has_ttl()) ||
(umessage.attributes().ttl() == 0)) {
return {false, Reason::INVALID_TTL};
}

if (umessage.attributes().has_commstatus()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_reqid()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

return {true, std::nullopt};
}

ValidationResult isValidRpcResponse(const v1::UMessage& umessage) {
auto [valid, reason] = areCommonAttributesValid(umessage);
if (!valid) {
return {false, reason};
}

if (umessage.attributes().type() !=
v1::UMessageType::UMESSAGE_TYPE_RESPONSE) {
return {false, Reason::WRONG_MESSAGE_TYPE};
}

if (!umessage.attributes().has_source()) {
return {false, Reason::BAD_SOURCE_URI};
}

if (!umessage.attributes().has_sink()) {
return {false, Reason::BAD_SINK_URI};
}

{
auto [valid, reason] =
uri::isValidRpcMethod(umessage.attributes().source());
if (!valid) {
return {false, Reason::BAD_SOURCE_URI};
}
}

{
auto [valid, reason] =
uri::isValidRpcResponse(umessage.attributes().sink());
if (!valid) {
return {false, Reason::BAD_SINK_URI};
}
}

if (!umessage.attributes().has_reqid()) {
return {false, Reason::REQID_MISMATCH};
}

{
auto [valid, reason] = uuid::isUuid(umessage.attributes().reqid());
if (!valid) {
return {false, Reason::REQID_MISMATCH};
}
}

if (umessage.attributes().has_ttl() && (umessage.attributes().ttl() > 0)) {
auto [expired, reason] = uuid::isExpired(
umessage.attributes().reqid(),
std::chrono::milliseconds(umessage.attributes().ttl()));
if (expired) {
return {false, Reason::ID_EXPIRED};
}
}

if (umessage.attributes().priority() < UPRIORITY_CS4) {
return {false, Reason::PRIORITY_OUT_OF_RANGE};
}

if (umessage.attributes().has_permission_level()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_token()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

return {true, std::nullopt};
}

ValidationResult isValidRpcResponseFor(const v1::UMessage& request,
const v1::UMessage& response) {
auto [valid, reason] = isValidRpcResponse(response);
if (!valid) {
return {false, reason};
}

if (!response.attributes().has_reqid()) {
return {false, Reason::REQID_MISMATCH};
}
Comment on lines +238 to +240
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this check necessary? I think isValidRpcResponse() already checks it.


if (!google::protobuf::util::MessageDifferencer::Equals(
response.attributes().reqid(), request.attributes().id())) {
return {false, Reason::REQID_MISMATCH};
}

if (request.attributes().priority() != response.attributes().priority()) {
return {false, Reason::PRIORITY_MISMATCH};
}

return {true, std::nullopt};
Comment on lines +247 to +251
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It just occurred to me that there should be two more checks: The source of the request should match the sink of the response and the sink of the request should match the source of the request. A URI_MISMATCH reason could be added for this scenario.

}

ValidationResult isValidPublish(const v1::UMessage& umessage) {
auto [valid, reason] = areCommonAttributesValid(umessage);
if (!valid) {
return {false, reason};
}

if (umessage.attributes().type() !=
v1::UMessageType::UMESSAGE_TYPE_PUBLISH) {
return {false, Reason::WRONG_MESSAGE_TYPE};
}

if (!umessage.attributes().has_source()) {
return {false, Reason::BAD_SOURCE_URI};
}

{
auto [valid, reason] =
uri::isValidPublishTopic(umessage.attributes().source());
if (!valid) {
return {false, Reason::BAD_SOURCE_URI};
}
}

if (umessage.attributes().has_sink()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}
gregmedd marked this conversation as resolved.
Show resolved Hide resolved

if (umessage.attributes().has_commstatus()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_reqid()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_permission_level()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_token()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

return {true, std::nullopt};
}

ValidationResult isValidNotification(const v1::UMessage& umessage) {
auto [valid, reason] = areCommonAttributesValid(umessage);
if (!valid) {
return {false, reason};
}

if (umessage.attributes().type() !=
v1::UMessageType::UMESSAGE_TYPE_NOTIFICATION) {
return {false, Reason::WRONG_MESSAGE_TYPE};
}

if (!umessage.attributes().has_source()) {
return {false, Reason::BAD_SOURCE_URI};
}

if (!umessage.attributes().has_sink()) {
return {false, Reason::BAD_SINK_URI};
}

{
auto [valid, reason] =
uri::isValidNotification(umessage.attributes().source());
if (!valid) {
return {false, Reason::BAD_SOURCE_URI};
}
}

{
auto [valid, reason] =
uri::isValidNotification(umessage.attributes().sink());
if (!valid) {
return {false, Reason::BAD_SINK_URI};
}
}

if (umessage.attributes().has_commstatus()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_reqid()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_permission_level()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

if (umessage.attributes().has_token()) {
return {false, Reason::DISALLOWED_FIELD_SET};
}

return {true, std::nullopt};
}

} // namespace uprotocol::datamodel::validator::message
Loading