Skip to content

Commit

Permalink
Added name and namespace validation. Corrected tests
Browse files Browse the repository at this point in the history 
Signed-off-by: ivanpauno <ivanpauno@ekumenlabs.com>
  • Loading branch information
@ivanpauno
ivanpauno committed Aug 30, 2019
1 parent b78ea81 commit bad86c2
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 57 deletions.
55 changes: 53 additions & 2 deletions rcl/src/rcl/graph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,46 @@ extern "C"
#include "rmw/get_service_names_and_types.h"
#include "rmw/get_topic_names_and_types.h"
#include "rmw/names_and_types.h"
#include "rmw/error_handling.h"
#include "rmw/rmw.h"
#include "rmw/validate_namespace.h"
#include "rmw/validate_node_name.h"

#include "./common.h"

rcl_ret_t
__validate_node_name_and_namespace(
const char * node_name,
const char * node_namespace)
{
int validation_result = 0;
rmw_ret_t rmw_ret = rmw_validate_namespace(node_namespace, &validation_result, NULL);

if (rmw_ret != RMW_RET_OK) {
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
if (validation_result != RMW_NAMESPACE_VALID) {
const char * msg = rmw_namespace_validation_result_string(validation_result);
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("%s, result: %d", msg, validation_result);
return RCL_RET_NODE_INVALID_NAMESPACE;
}

validation_result = 0;
rmw_ret = rmw_validate_node_name(node_name, &validation_result, NULL);
if (rmw_ret != RMW_RET_OK) {
RCL_SET_ERROR_MSG(rmw_get_error_string().str);
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
if (validation_result != RMW_NODE_NAME_VALID) {
const char * msg = rmw_node_name_validation_result_string(validation_result);
RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("%s, result: %d", msg, validation_result);
return RCL_RET_NODE_INVALID_NAME;
}

return RCL_RET_OK;
}

rcl_ret_t
rcl_get_publisher_names_and_types_by_node(
const rcl_node_t * node,
Expand All @@ -51,11 +87,14 @@ rcl_get_publisher_names_and_types_by_node(
if (strlen(node_namespace) > 0) {
valid_namespace = node_namespace;
}
rmw_ret_t rmw_ret;
rmw_ret = rmw_names_and_types_check_zero(topic_names_and_types);
rmw_ret_t rmw_ret = rmw_names_and_types_check_zero(topic_names_and_types);
if (rmw_ret != RMW_RET_OK) {
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
rcl_ret_t rcl_ret = __validate_node_name_and_namespace(node_name, valid_namespace);
if (RCL_RET_OK != rcl_ret) {
return rcl_ret;
}
rcutils_allocator_t rcutils_allocator = *allocator;
rmw_ret = rmw_get_publisher_names_and_types_by_node(
rcl_node_get_rmw_handle(node),
Expand Down Expand Up @@ -95,6 +134,10 @@ rcl_get_subscriber_names_and_types_by_node(
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
rcutils_allocator_t rcutils_allocator = *allocator;
rcl_ret_t rcl_ret = __validate_node_name_and_namespace(node_name, valid_namespace);
if (RCL_RET_OK != rcl_ret) {
return rcl_ret;
}
rmw_ret = rmw_get_subscriber_names_and_types_by_node(
rcl_node_get_rmw_handle(node),
&rcutils_allocator,
Expand Down Expand Up @@ -131,6 +174,10 @@ rcl_get_service_names_and_types_by_node(
if (rmw_ret != RMW_RET_OK) {
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
rcl_ret_t rcl_ret = __validate_node_name_and_namespace(node_name, valid_namespace);
if (RCL_RET_OK != rcl_ret) {
return rcl_ret;
}
rcutils_allocator_t rcutils_allocator = *allocator;
rmw_ret = rmw_get_service_names_and_types_by_node(
rcl_node_get_rmw_handle(node),
Expand Down Expand Up @@ -167,6 +214,10 @@ rcl_get_client_names_and_types_by_node(
if (rmw_ret != RMW_RET_OK) {
return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
}
rcl_ret_t rcl_ret = __validate_node_name_and_namespace(node_name, valid_namespace);
if (RCL_RET_OK != rcl_ret) {
return rcl_ret;
}
rcutils_allocator_t rcutils_allocator = *allocator;
rmw_ret = rmw_get_client_names_and_types_by_node(
rcl_node_get_rmw_handle(node),
Expand Down
104 changes: 49 additions & 55 deletions rcl/test/rcl/test_graph.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,8 @@ TEST_F(
rcl_allocator_t zero_allocator = static_cast<rcl_allocator_t>(
rcutils_get_zero_initialized_allocator());
rcl_node_t zero_node = rcl_get_zero_initialized_node();
const char * unknown_node_name = "/test_rcl_get_publisher_names_and_types_by_node";
// const char * unknown_node_ns = "/test/namespace";
const char * unknown_node_name = "test_rcl_get_publisher_names_and_types_by_node";
const char * unknown_node_ns = "/test/namespace/unknown";
rcl_names_and_types_t nat = rcl_get_zero_initialized_names_and_types();
// invalid node
ret = rcl_get_publisher_names_and_types_by_node(
Expand Down Expand Up @@ -311,17 +311,16 @@ TEST_F(
// test valid strings with invalid node names
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, "", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, "_InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
rcl_reset_error();
// TODO(jacobperron): This succeeds, but should fail due to invalid namespace
// ret = rcl_get_publisher_names_and_types_by_node(
// this->node_ptr, &allocator, false, this->test_graph_node_name, "_!invalidNs", &nat);
// EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
// rcl_reset_error();
this->node_ptr, &allocator, false, "_!InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, "_!invalidNs", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAMESPACE, ret) << rcl_get_error_string().str;
rcl_reset_error();
// invalid names and types
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, "", nullptr);
Expand All @@ -330,14 +329,13 @@ TEST_F(
// unknown node name
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, unknown_node_name, "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str;
rcl_reset_error();
// unknown node namespace
// TODO(jacobperron): This succeeds, but should fail due to invalid namespace
// ret = rcl_get_publisher_names_and_types_by_node(
// this->node_ptr, &allocator, false, this->test_graph_node_name, unknown_node_ns, &nat);
// EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
// rcl_reset_error();
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, unknown_node_ns, &nat);
EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str;
rcl_reset_error();
// valid call
ret = rcl_get_publisher_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, "", &nat);
Expand All @@ -361,8 +359,8 @@ TEST_F(
rcl_allocator_t zero_allocator = static_cast<rcl_allocator_t>(
rcutils_get_zero_initialized_allocator());
rcl_node_t zero_node = rcl_get_zero_initialized_node();
const char * unknown_node_name = "/test_rcl_get_subscriber_names_and_types_by_node";
// const char * unknown_node_ns = "/test/namespace";
const char * unknown_node_name = "test_rcl_get_subscriber_names_and_types_by_node";
const char * unknown_node_ns = "/test/namespace/unknown";
rcl_names_and_types_t nat = rcl_get_zero_initialized_names_and_types();
// invalid node
ret = rcl_get_subscriber_names_and_types_by_node(
Expand Down Expand Up @@ -398,17 +396,16 @@ TEST_F(
// test valid strings with invalid node names
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, "", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, "_!InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, "_InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
rcl_reset_error();
// TODO(jacobperron): This succeeds, but should fail due to invalid namespace
// ret = rcl_get_subscriber_names_and_types_by_node(
// this->node_ptr, &allocator, false, this->test_graph_node_name, "_!invalidNs", &nat);
// EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
// rcl_reset_error();
this->node_ptr, &allocator, false, this->test_graph_node_name, "_!invalidNs", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAMESPACE, ret) << rcl_get_error_string().str;
rcl_reset_error();
// invalid names and types
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, "", nullptr);
Expand All @@ -417,14 +414,13 @@ TEST_F(
// unknown node name
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, unknown_node_name, "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str;
rcl_reset_error();
// unknown node namespace
// TODO(jacobperron): This succeeds, but should fail due to invalid namespace
// ret = rcl_get_subscriber_names_and_types_by_node(
// this->node_ptr, &allocator, false, this->test_graph_node_name, unknown_node_ns, &nat);
// EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
// rcl_reset_error();
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, unknown_node_ns, &nat);
EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str;
rcl_reset_error();
// valid call
ret = rcl_get_subscriber_names_and_types_by_node(
this->node_ptr, &allocator, false, this->test_graph_node_name, "", &nat);
Expand All @@ -446,7 +442,7 @@ TEST_F(
rcutils_get_zero_initialized_allocator());
rcl_node_t zero_node = rcl_get_zero_initialized_node();
const char * unknown_node_name = "/test_rcl_get_service_names_and_types_by_node";
// const char * unknown_node_ns = "/test/namespace";
const char * unknown_node_ns = "/test/namespace/unknown";
rcl_names_and_types_t nat = rcl_get_zero_initialized_names_and_types();
// invalid node
ret = rcl_get_service_names_and_types_by_node(
Expand Down Expand Up @@ -482,17 +478,16 @@ TEST_F(
// test valid strings with invalid node names
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, "", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, "_!InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, "_InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
rcl_reset_error();
// TODO(jacobperron): This succeeds, but should fail due to invalid namespace
// ret = rcl_get_service_names_and_types_by_node(
// this->node_ptr, &allocator, false, this->test_graph_node_name, "_!invalidNs", &nat);
// EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
// rcl_reset_error();
this->node_ptr, &allocator, this->test_graph_node_name, "_!invalidNs", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAMESPACE, ret) << rcl_get_error_string().str;
rcl_reset_error();
// invalid names and types
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, this->test_graph_node_name, "", nullptr);
Expand All @@ -501,14 +496,13 @@ TEST_F(
// unknown node name
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, unknown_node_name, "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
// unknown node namespace
// TODO(jacobperron): This succeeds, but should fail due to invalid namespace
// ret = rcl_get_service_names_and_types_by_node(
// this->node_ptr, &allocator, this->test_graph_node_name, unknown_node_ns, &nat);
// EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
// rcl_reset_error();
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, this->test_graph_node_name, unknown_node_ns, &nat);
EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str;
rcl_reset_error();
// valid call
ret = rcl_get_service_names_and_types_by_node(
this->node_ptr, &allocator, this->test_graph_node_name, "", &nat);
Expand All @@ -529,7 +523,7 @@ TEST_F(
rcl_allocator_t zero_allocator = static_cast<rcl_allocator_t>(
rcutils_get_zero_initialized_allocator());
rcl_node_t zero_node = rcl_get_zero_initialized_node();
const char * unknown_node_name = "/test_rcl_get_client_names_and_types_by_node";
const char * unknown_node_name = "test_rcl_get_client_names_and_types_by_node";
rcl_names_and_types_t nat = rcl_get_zero_initialized_names_and_types();
// invalid node
ret = rcl_get_client_names_and_types_by_node(
Expand Down Expand Up @@ -565,11 +559,11 @@ TEST_F(
// test valid strings with invalid node names
ret = rcl_get_client_names_and_types_by_node(
this->node_ptr, &allocator, "", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
ret = rcl_get_client_names_and_types_by_node(
this->node_ptr, &allocator, "_InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
this->node_ptr, &allocator, "_!InvalidNodeName", "", &nat);
EXPECT_EQ(RCL_RET_NODE_INVALID_NAME, ret) << rcl_get_error_string().str;
rcl_reset_error();
// invalid names and types
ret = rcl_get_client_names_and_types_by_node(
Expand All @@ -579,7 +573,7 @@ TEST_F(
// unknown node name
ret = rcl_get_client_names_and_types_by_node(
this->node_ptr, &allocator, unknown_node_name, "", &nat);
EXPECT_EQ(RCL_RET_ERROR, ret) << rcl_get_error_string().str;
EXPECT_EQ(RCL_RET_NODE_NAME_NON_EXISTENT, ret) << rcl_get_error_string().str;
rcl_reset_error();
// valid call
ret = rcl_get_client_names_and_types_by_node(
Expand Down

0 comments on commit bad86c2

Please sign in to comment.