From bad86c204db17f2538a537366edf74b4d79645bd Mon Sep 17 00:00:00 2001 From: ivanpauno Date: Fri, 30 Aug 2019 16:58:38 -0300 Subject: [PATCH] Added name and namespace validation. Corrected tests Signed-off-by: ivanpauno --- rcl/src/rcl/graph.c | 55 ++++++++++++++++++- rcl/test/rcl/test_graph.cpp | 104 +++++++++++++++++------------------- 2 files changed, 102 insertions(+), 57 deletions(-) diff --git a/rcl/src/rcl/graph.c b/rcl/src/rcl/graph.c index c43722775..637fad092 100644 --- a/rcl/src/rcl/graph.c +++ b/rcl/src/rcl/graph.c @@ -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, @@ -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), @@ -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, @@ -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), @@ -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), diff --git a/rcl/test/rcl/test_graph.cpp b/rcl/test/rcl/test_graph.cpp index 2a6bd1e76..99c23a30d 100644 --- a/rcl/test/rcl/test_graph.cpp +++ b/rcl/test/rcl/test_graph.cpp @@ -274,8 +274,8 @@ TEST_F( rcl_allocator_t zero_allocator = static_cast( 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( @@ -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); @@ -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); @@ -361,8 +359,8 @@ TEST_F( rcl_allocator_t zero_allocator = static_cast( 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( @@ -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); @@ -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); @@ -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( @@ -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); @@ -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); @@ -529,7 +523,7 @@ TEST_F( rcl_allocator_t zero_allocator = static_cast( 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( @@ -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( @@ -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(