diff --git a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/utils.hpp b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/utils.hpp index 6a501c71da..ccc1ca2fc3 100644 --- a/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/utils.hpp +++ b/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/utils.hpp @@ -15,8 +15,10 @@ #ifndef RMW_FASTRTPS_SHARED_CPP__UTILS_HPP_ #define RMW_FASTRTPS_SHARED_CPP__UTILS_HPP_ +#include #include #include +#include #include "fastdds/dds/topic/TopicDescription.hpp" #include "fastdds/dds/topic/TypeSupport.hpp" @@ -33,6 +35,59 @@ namespace rmw_fastrtps_shared_cpp { +/// Ensure a content filter parameter is a valid DDS SQL literal. +/// +/// Fast DDS requires content filter parameters to be parseable as DDS SQL +/// literals. Bare strings like "hello" must be wrapped in single quotes to +/// become valid string literals ('hello'), otherwise +/// DDSFilterParameter::set_value() fails with a parse error. +/// See https://github.com/eProsima/Fast-DDS/issues/4199 +inline std::string +ensure_dds_literal(const std::string & value) +{ + if (value.empty()) { + return "'" + value + "'"; + } + + // Already a quoted string + if ((value.front() == '\'' || value.front() == '`') && value.size() >= 2 && + value.back() == value.front()) + { + return value; + } + + // Boolean literals + if (value == "TRUE" || value == "FALSE") { + return value; + } + + // Numeric: optional leading sign, then digit or dot (covers int, float, hex) + const char * p = value.c_str(); + if (*p == '+' || *p == '-') { + ++p; + } + if (*p == '0' && (*(p + 1) == 'x' || *(p + 1) == 'X')) { + return value; // hex + } + if (std::isdigit(static_cast(*p)) || *p == '.') { + return value; // numeric + } + + // Not a recognized literal — wrap in single quotes + return "'" + value + "'"; +} + +/// Convert rmw content filter parameters, auto-quoting bare strings for Fast DDS. +inline std::vector +prepare_content_filter_parameters(const rmw_subscription_content_filter_options_t * options) +{ + std::vector expression_parameters; + for (size_t i = 0; i < options->expression_parameters.size; ++i) { + expression_parameters.push_back(ensure_dds_literal(options->expression_parameters.data[i])); + } + return expression_parameters; +} + /** * Convert a Fast DDS return code into the corresponding rmw_ret_t * \param[in] code The Fast DDS return code to be translated diff --git a/rmw_fastrtps_shared_cpp/src/rmw_subscription.cpp b/rmw_fastrtps_shared_cpp/src/rmw_subscription.cpp index 98e471563c..72dee4bede 100644 --- a/rmw_fastrtps_shared_cpp/src/rmw_subscription.cpp +++ b/rmw_fastrtps_shared_cpp/src/rmw_subscription.cpp @@ -120,10 +120,8 @@ __rmw_subscription_set_content_filter( "current subscriber has no content filter topic"); return RMW_RET_ERROR; } else if (filtered_topic && !filter_expression_empty) { - std::vector expression_parameters; - for (size_t i = 0; i < options->expression_parameters.size; ++i) { - expression_parameters.push_back(options->expression_parameters.data[i]); - } + std::vector expression_parameters = + rmw_fastrtps_shared_cpp::prepare_content_filter_parameters(options); eprosima::fastdds::dds::ReturnCode_t ret = filtered_topic->set_filter_expression(options->filter_expression, expression_parameters); diff --git a/rmw_fastrtps_shared_cpp/src/utils.cpp b/rmw_fastrtps_shared_cpp/src/utils.cpp index a781020107..674487c2c3 100644 --- a/rmw_fastrtps_shared_cpp/src/utils.cpp +++ b/rmw_fastrtps_shared_cpp/src/utils.cpp @@ -102,10 +102,7 @@ create_content_filtered_topic( const rmw_subscription_content_filter_options_t * options, eprosima::fastdds::dds::ContentFilteredTopic ** content_filtered_topic) { - std::vector expression_parameters; - for (size_t i = 0; i < options->expression_parameters.size; ++i) { - expression_parameters.push_back(options->expression_parameters.data[i]); - } + std::vector expression_parameters = prepare_content_filter_parameters(options); auto topic = dynamic_cast(topic_desc); static std::atomic cft_counter{0}; diff --git a/rmw_fastrtps_shared_cpp/test/CMakeLists.txt b/rmw_fastrtps_shared_cpp/test/CMakeLists.txt index 33c38b28ee..b859ae8138 100644 --- a/rmw_fastrtps_shared_cpp/test/CMakeLists.txt +++ b/rmw_fastrtps_shared_cpp/test/CMakeLists.txt @@ -54,3 +54,8 @@ ament_add_gtest(test_logging test_logging.cpp) if(TARGET test_logging) target_link_libraries(test_logging ${PROJECT_NAME} rmw::rmw) endif() + +ament_add_gtest(test_ensure_dds_literal test_ensure_dds_literal.cpp) +if(TARGET test_ensure_dds_literal) + target_link_libraries(test_ensure_dds_literal ${PROJECT_NAME} rmw::rmw) +endif() diff --git a/rmw_fastrtps_shared_cpp/test/test_ensure_dds_literal.cpp b/rmw_fastrtps_shared_cpp/test/test_ensure_dds_literal.cpp new file mode 100644 index 0000000000..929499605f --- /dev/null +++ b/rmw_fastrtps_shared_cpp/test/test_ensure_dds_literal.cpp @@ -0,0 +1,79 @@ +// Copyright 2026 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include + +#include "rmw_fastrtps_shared_cpp/utils.hpp" + +using rmw_fastrtps_shared_cpp::ensure_dds_literal; + +// Bare strings should be wrapped in single quotes +TEST(EnsureDdsLiteral, bare_string_gets_quoted) +{ + EXPECT_EQ("'hello'", ensure_dds_literal("hello")); + EXPECT_EQ("'RED'", ensure_dds_literal("RED")); + EXPECT_EQ("'some value'", ensure_dds_literal("some value")); +} + +// Already single-quoted strings should pass through unchanged +TEST(EnsureDdsLiteral, already_single_quoted) +{ + EXPECT_EQ("'hello'", ensure_dds_literal("'hello'")); + EXPECT_EQ("'with spaces'", ensure_dds_literal("'with spaces'")); +} + +// Backtick-quoted strings should pass through unchanged +TEST(EnsureDdsLiteral, already_backtick_quoted) +{ + EXPECT_EQ("`hello`", ensure_dds_literal("`hello`")); +} + +// Boolean literals should pass through unchanged +TEST(EnsureDdsLiteral, boolean_literals) +{ + EXPECT_EQ("TRUE", ensure_dds_literal("TRUE")); + EXPECT_EQ("FALSE", ensure_dds_literal("FALSE")); +} + +// Integer literals should pass through unchanged +TEST(EnsureDdsLiteral, integer_literals) +{ + EXPECT_EQ("42", ensure_dds_literal("42")); + EXPECT_EQ("0", ensure_dds_literal("0")); + EXPECT_EQ("-1", ensure_dds_literal("-1")); + EXPECT_EQ("+5", ensure_dds_literal("+5")); +} + +// Float literals should pass through unchanged +TEST(EnsureDdsLiteral, float_literals) +{ + EXPECT_EQ("3.14", ensure_dds_literal("3.14")); + EXPECT_EQ(".5", ensure_dds_literal(".5")); + EXPECT_EQ("-0.1", ensure_dds_literal("-0.1")); +} + +// Hex literals should pass through unchanged +TEST(EnsureDdsLiteral, hex_literals) +{ + EXPECT_EQ("0xFF", ensure_dds_literal("0xFF")); + EXPECT_EQ("0X1A", ensure_dds_literal("0X1A")); +} + +// Empty string should be quoted +TEST(EnsureDdsLiteral, empty_string) +{ + EXPECT_EQ("''", ensure_dds_literal("")); +}