From f462e90edf93be958a197f40185085d56721819b Mon Sep 17 00:00:00 2001 From: Michel Hidalgo Date: Fri, 12 Jun 2020 13:17:52 -0300 Subject: [PATCH] Improve rcl init test coverage. Signed-off-by: Michel Hidalgo --- rcl/include/rcl/init.h | 1 + rcl/src/rcl/init.c | 6 +++--- rcl/src/rcl/security.c | 2 +- rcl/test/rcl/test_init.cpp | 29 ++++++++++++++++++++++++++++- 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/rcl/include/rcl/init.h b/rcl/include/rcl/init.h index 315bc4ba00..42b07b5f0c 100644 --- a/rcl/include/rcl/init.h +++ b/rcl/include/rcl/init.h @@ -69,6 +69,7 @@ extern "C" * \return `RCL_RET_OK` if initialization is successful, or * \return `RCL_RET_ALREADY_INIT` if rcl_init has already been called, or * \return `RCL_RET_INVALID_ARGUMENT` if any arguments are invalid, or + * \return `RCL_RET_INVALID_ROS_ARGS` if an invalid ROS argument is found, or * \return `RCL_RET_BAD_ALLOC` if allocating memory failed, or * \return `RCL_RET_ERROR` if an unspecified error occurs. */ diff --git a/rcl/src/rcl/init.c b/rcl/src/rcl/init.c index a18f675bb3..0cb92bb2f1 100644 --- a/rcl/src/rcl/init.c +++ b/rcl/src/rcl/init.c @@ -188,13 +188,13 @@ rcl_init( "Enclave name is not valid: '%s'. Invalid index: %zu", rcl_enclave_name_validation_result_string(validation_result), invalid_index); - fail_ret = RMW_RET_ERROR; + fail_ret = RCL_RET_ERROR; goto fail; } if (!context->impl->init_options.impl->rmw_init_options.enclave) { RCL_SET_ERROR_MSG("failed to set context name"); - fail_ret = RMW_RET_BAD_ALLOC; + fail_ret = RCL_RET_BAD_ALLOC; goto fail; } @@ -204,7 +204,7 @@ rcl_init( context->impl->init_options.impl->rmw_init_options.enclave, &context->impl->allocator, security_options); - if (RMW_RET_OK != ret) { + if (RCL_RET_OK != ret) { fail_ret = ret; goto fail; } diff --git a/rcl/src/rcl/security.c b/rcl/src/rcl/security.c index a128c6651c..e895fb0f79 100644 --- a/rcl/src/rcl/security.c +++ b/rcl/src/rcl/security.c @@ -42,7 +42,7 @@ rcl_get_security_options_from_environment( if (!use_security) { security_options->enforce_security = RMW_SECURITY_ENFORCEMENT_PERMISSIVE; - return RMW_RET_OK; + return RCL_RET_OK; } ret = rcl_get_enforcement_policy(&security_options->enforce_security); diff --git a/rcl/test/rcl/test_init.cpp b/rcl/test/rcl/test_init.cpp index da638f1e83..56807913e6 100644 --- a/rcl/test/rcl/test_init.cpp +++ b/rcl/test/rcl/test_init.cpp @@ -15,11 +15,13 @@ #include #include "rcl/rcl.h" - +#include "rcl/arguments.h" +#include "rcl/security.h" #include "./failing_allocator_functions.hpp" #include "osrf_testing_tools_cpp/memory_tools/memory_tools.hpp" #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "rcutils/env.h" #include "rcutils/format_string.h" #include "rcutils/snprintf.h" @@ -131,6 +133,31 @@ TEST_F(CLASSNAME(TestRCLFixture, RMW_IMPLEMENTATION), test_rcl_init_and_shutdown EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret); rcl_reset_error(); ASSERT_FALSE(rcl_context_is_valid(&context)); + // If an invalid ROS arg is given, init should fail. + const char * bad_remap_args[] = { + "some-arg", RCL_ROS_ARGS_FLAG, RCL_REMAP_FLAG, "name:="}; + ret = rcl_init(4, bad_remap_args, &init_options, &context); + EXPECT_EQ(RCL_RET_INVALID_ROS_ARGS, ret); + rcl_reset_error(); + ASSERT_FALSE(rcl_context_is_valid(&context)); + // If an invalid enclave is given, init should fail. + const char * bad_enclave_args[] = { + "some-arg", RCL_ROS_ARGS_FLAG, RCL_ENCLAVE_FLAG, "1foo"}; + ret = rcl_init(4, bad_enclave_args, &init_options, &context); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + ASSERT_FALSE(rcl_context_is_valid(&context)); + // If security keystore is invalid, init should fail. + ASSERT_TRUE(rcutils_set_env(ROS_SECURITY_ENABLE_VAR_NAME, "true")); + ASSERT_TRUE(rcutils_set_env(ROS_SECURITY_STRATEGY_VAR_NAME, "Enforce")); + ASSERT_TRUE(rcutils_set_env(ROS_SECURITY_KEYSTORE_VAR_NAME, "/not/a/real/secure/root")); + ret = rcl_init(0, nullptr, &init_options, &context); + EXPECT_EQ(RCL_RET_ERROR, ret); + rcl_reset_error(); + EXPECT_FALSE(rcl_context_is_valid(&context)); + ASSERT_TRUE(rcutils_set_env(ROS_SECURITY_KEYSTORE_VAR_NAME, "")); + ASSERT_TRUE(rcutils_set_env(ROS_SECURITY_STRATEGY_VAR_NAME, "")); + ASSERT_TRUE(rcutils_set_env(ROS_SECURITY_ENABLE_VAR_NAME, "")); // If either the allocate or deallocate function pointers are not set, it should be invalid arg. init_options.impl->allocator.allocate = nullptr; ret = rcl_init(0, nullptr, &init_options, &context);