Skip to content

Commit

Permalink
Enforce -r/--remap flags. (#491)
Browse files Browse the repository at this point in the history 
* Enforce -r/--remap flags.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>

* Cast size_t to int explicitly.

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
  • Loading branch information
@hidmic
hidmic authored Sep 3, 2019
1 parent 6f98943 commit 2740a43
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 141 deletions.
30 changes: 21 additions & 9 deletions rcl/include/rcl/arguments.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,26 +57,38 @@ rcl_get_zero_initialized_arguments(void);

/// Parse command line arguments into a structure usable by code.
/**
* If an argument does not appear to be a valid ROS argument then it is skipped
* and parsing continues with the next argument in `argv`.
*
* \sa rcl_get_zero_initialized_arguments()
* \sa rcl_arguments_get_count_unparsed()
* \sa rcl_arguments_get_unparsed()
*
* ROS arguments are expected to be scoped by a leading `--ros-args` flag and a trailing double
* dash token `--` which may be elided if no non-ROS arguments follow after the last `--ros-args`.
*
* Remap rule parsing is supported via `-r/--remap` flags e.g. `--remap from:=to` or `-r from:=to`.
* Successfully parsed remap rules are stored in the order they were given in `argv`.
* If given arguments `{"__ns:=/foo", "__ns:=/bar"}` then the namespace used by nodes in this
* process will be `/foo` and not `/bar`.
*
* \sa rcl_remap_topic_name()
* \sa rcl_remap_service_name()
* \sa rcl_remap_node_name()
* \sa rcl_remap_node_namespace()
*
* Parameter override rule parsing is supported via `-p/--param` flags e.g. `--param name:=value`
* or `-p name:=value`.
*
* The default log level will be parsed as `__log_level:=level`, where `level` is a name
* representing one of the log levels in the `RCUTILS_LOG_SEVERITY` enum, e.g. `info`, `debug`,
* `warn`, not case sensitive.
* If multiple of these rules are found, the last one parsed will be used.
*
* \sa rcl_remap_topic_name()
* \sa rcl_remap_service_name()
* \sa rcl_remap_node_name()
* \sa rcl_remap_node_namespace()
* If an argument does not appear to be a known ROS argument, then it is skipped and left unparsed.
*
* \sa rcl_arguments_get_count_unparsed_ros()
* \sa rcl_arguments_get_unparsed_ros()
*
* All arguments found outside a `--ros-args ... --` scope are skipped and left unparsed.
*
* \sa rcl_arguments_get_count_unparsed()
* \sa rcl_arguments_get_unparsed()
*
* <hr>
* Attribute | Adherence
Expand Down
12 changes: 0 additions & 12 deletions rcl/src/rcl/arguments.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -364,18 +364,6 @@ rcl_parse_arguments(
rcl_get_error_string().str);
rcl_reset_error();

// Attempt to parse argument as remap rule
rcl_remap_t * rule = &(args_impl->remap_rules[args_impl->num_remap_rules]);
*rule = rcl_get_zero_initialized_remap();
if (RCL_RET_OK == _rcl_parse_remap_rule(argv[i], allocator, rule)) {
++(args_impl->num_remap_rules);
continue;
}
RCUTILS_LOG_DEBUG_NAMED(ROS_PACKAGE_NAME,
"Couldn't parse arg %d (%s) as remap rule. Error: %s", i, argv[i],
rcl_get_error_string().str);
rcl_reset_error();

// Attempt to parse argument as log level configuration
int log_level;
if (RCL_RET_OK == _rcl_parse_log_level_rule(argv[i], allocator, &log_level)) {
Expand Down
141 changes: 74 additions & 67 deletions rcl/test/rcl/test_arguments.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,9 @@ class CLASSNAME (TestArgumentsFixture, RMW_IMPLEMENTATION) : public ::testing::T
} while (0)

bool
are_valid_ros_args(int argc, std::vector<const char *> argv)
are_known_ros_args(std::vector<const char *> argv)
{
const int argc = static_cast<int>(argv.size());
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
rcl_ret_t ret = rcl_parse_arguments(
argc, argv.data(), rcl_get_default_allocator(), &parsed_args);
Expand All @@ -117,74 +118,80 @@ are_valid_ros_args(int argc, std::vector<const char *> argv)
return is_valid;
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_valid_vs_invalid_args) {
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "__node:=node_name"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "old_name:__node:=node_name"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "old_name:__node:=nodename123"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "__node:=nodename123"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "__ns:=/foo/bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "__ns:=/"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "_:=kq"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "nodename:__ns:=/foobar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "foo:=bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "~/foo:=~/bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "/foo/bar:=bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "foo:=/bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "/foo123:=/bar123"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "node:/foo123:=/bar123"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "rostopic:=/foo/bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "rosservice:=baz"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "rostopic://rostopic:=rosservice"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "rostopic:///rosservice:=rostopic"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-r", "rostopic:///foo/bar:=baz"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-p", "foo:=bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-p", "~/foo:=~/bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-p", "/foo/bar:=bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-p", "foo:=/bar"}));
EXPECT_TRUE(are_valid_ros_args(3, {"--ros-args", "-p", "/foo123:=/bar123"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__params:=file_name.yaml"}));

EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", ":="}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "foo:="}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", ":=bar"}));

EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-p", ":="}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-p", "foo:="}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-p", ":=bar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "__ns:="}));

EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "__node:="}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "__node:=/foo/bar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "__ns:=foo"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", ":__node:=nodename"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "~:__node:=nodename"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "}foo:=/bar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "f oo:=/bar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "foo:=/b ar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "f{oo:=/bar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "foo:=/b}ar"}));

EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-p", "}foo:=/bar"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-p", "f oo:=/bar"}));

EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "rostopic://:=rosservice"}));
EXPECT_FALSE(are_valid_ros_args(3, {"--ros-args", "-r", "rostopic::=rosservice"}));
EXPECT_FALSE(are_valid_ros_args(2, {"--ros-args", "__param:=file_name.yaml"}));
TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), check_known_vs_unknown_args) {
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "__node:=node_name"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "old_name:__node:=node_name"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "old_name:__node:=nodename123"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "__node:=nodename123"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "__ns:=/foo/bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "__ns:=/"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "_:=kq"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "nodename:__ns:=/foobar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "foo:=bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "~/foo:=~/bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "/foo/bar:=bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "foo:=/bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "/foo123:=/bar123"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "node:/foo123:=/bar123"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rostopic:=/foo/bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rosservice:=baz"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rostopic://rostopic:=rosservice"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rostopic:///rosservice:=rostopic"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-r", "rostopic:///foo/bar:=baz"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "foo:=bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "~/foo:=~/bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "/foo/bar:=bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "foo:=/bar"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "-p", "/foo123:=/bar123"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__params:=file_name.yaml"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "--remap"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", ":="}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "foo:="}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", ":=bar"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-p"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "--param"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-p", ":="}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-p", "foo:="}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-p", ":=bar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "__ns:="}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "__node:="}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "__node:=/foo/bar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "__ns:=foo"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", ":__node:=nodename"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "~:__node:=nodename"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "}foo:=/bar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "f oo:=/bar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "foo:=/b ar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "f{oo:=/bar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "foo:=/b}ar"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-p", "}foo:=/bar"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-p", "f oo:=/bar"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "rostopic://:=rosservice"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "-r", "rostopic::=rosservice"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "__param:=file_name.yaml"}));

// Setting logger level
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=UNSET"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=DEBUG"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=INFO"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=WARN"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=ERROR"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=FATAL"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=debug"}));
EXPECT_TRUE(are_valid_ros_args(2, {"--ros-args", "__log_level:=Info"}));

EXPECT_FALSE(are_valid_ros_args(2, {"--ros-args", "__log:=foo"}));
EXPECT_FALSE(are_valid_ros_args(2, {"--ros-args", "__loglevel:=foo"}));
EXPECT_FALSE(are_valid_ros_args(2, {"--ros-args", "__log_level:="}));
EXPECT_FALSE(are_valid_ros_args(2, {"--ros-args", "__log_level:=foo"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=UNSET"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=DEBUG"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=INFO"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=WARN"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=ERROR"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=FATAL"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=debug"}));
EXPECT_TRUE(are_known_ros_args({"--ros-args", "__log_level:=Info"}));

EXPECT_FALSE(are_known_ros_args({"--ros-args", "__log:=foo"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "__loglevel:=foo"}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "__log_level:="}));
EXPECT_FALSE(are_known_ros_args({"--ros-args", "__log_level:=foo"}));
}

TEST_F(CLASSNAME(TestArgumentsFixture, RMW_IMPLEMENTATION), test_no_args) {
Expand Down
Loading

0 comments on commit 2740a43

Please sign in to comment.