Improve rcl init test coverage.

Signed-off-by: Michel Hidalgo <[email protected]>
ros2 · Jun 12, 2020 · f462e90 · f462e90
1 parent 7723308
commit f462e90
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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
@@ -15,11 +15,13 @@
 #include <gtest/gtest.h>
 
 #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);