Skip to content

Commit 3fa3096

Browse files
Auto-quote bare string parameters for DDS content filter expressions
Fast DDS requires content filter expression parameters to be valid DDS SQL literals. Bare strings like "hello" fail to parse in DDSFilterParameter::set_value() because the DDSSQLFilter grammar expects string literals to be single-quoted ('hello'). This causes content filter parameter substitution (%0, %1, ...) to silently fail for string-typed fields when users pass unquoted values, which is the natural thing to do from a CLI or application perspective. Add ensure_dds_literal() that detects bare strings (not already a number, boolean, or quoted string) and wraps them in single quotes before passing to Fast DDS. Applied at both content filter creation and dynamic filter update call sites. References: eProsima/Fast-DDS#4199
1 parent 41514e5 commit 3fa3096

5 files changed

Lines changed: 142 additions & 8 deletions

File tree

rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/utils.hpp

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
#ifndef RMW_FASTRTPS_SHARED_CPP__UTILS_HPP_
1616
#define RMW_FASTRTPS_SHARED_CPP__UTILS_HPP_
1717

18+
#include <cctype>
1819
#include <mutex>
1920
#include <string>
21+
#include <vector>
2022

2123
#include "fastdds/dds/topic/TopicDescription.hpp"
2224
#include "fastdds/dds/topic/TypeSupport.hpp"
@@ -33,6 +35,59 @@
3335
namespace rmw_fastrtps_shared_cpp
3436
{
3537

38+
/// Ensure a content filter parameter is a valid DDS SQL literal.
39+
///
40+
/// Fast DDS requires content filter parameters to be parseable as DDS SQL
41+
/// literals. Bare strings like "hello" must be wrapped in single quotes to
42+
/// become valid string literals ('hello'), otherwise
43+
/// DDSFilterParameter::set_value() fails with a parse error.
44+
/// See https://github.com/eProsima/Fast-DDS/issues/4199
45+
inline std::string
46+
ensure_dds_literal(const std::string & value)
47+
{
48+
if (value.empty()) {
49+
return "'" + value + "'";
50+
}
51+
52+
// Already a quoted string
53+
if ((value.front() == '\'' || value.front() == '`') && value.size() >= 2 &&
54+
value.back() == value.front())
55+
{
56+
return value;
57+
}
58+
59+
// Boolean literals
60+
if (value == "TRUE" || value == "FALSE") {
61+
return value;
62+
}
63+
64+
// Numeric: optional leading sign, then digit or dot (covers int, float, hex)
65+
const char * p = value.c_str();
66+
if (*p == '+' || *p == '-') {
67+
++p;
68+
}
69+
if (*p == '0' && (*(p + 1) == 'x' || *(p + 1) == 'X')) {
70+
return value; // hex
71+
}
72+
if (std::isdigit(static_cast<unsigned char>(*p)) || *p == '.') {
73+
return value; // numeric
74+
}
75+
76+
// Not a recognized literal — wrap in single quotes
77+
return "'" + value + "'";
78+
}
79+
80+
/// Convert rmw content filter parameters, auto-quoting bare strings for Fast DDS.
81+
inline std::vector<std::string>
82+
prepare_content_filter_parameters(const rmw_subscription_content_filter_options_t * options)
83+
{
84+
std::vector<std::string> expression_parameters;
85+
for (size_t i = 0; i < options->expression_parameters.size; ++i) {
86+
expression_parameters.push_back(ensure_dds_literal(options->expression_parameters.data[i]));
87+
}
88+
return expression_parameters;
89+
}
90+
3691
/**
3792
* Convert a Fast DDS return code into the corresponding rmw_ret_t
3893
* \param[in] code The Fast DDS return code to be translated

rmw_fastrtps_shared_cpp/src/rmw_subscription.cpp

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,10 +120,8 @@ __rmw_subscription_set_content_filter(
120120
"current subscriber has no content filter topic");
121121
return RMW_RET_ERROR;
122122
} else if (filtered_topic && !filter_expression_empty) {
123-
std::vector<std::string> expression_parameters;
124-
for (size_t i = 0; i < options->expression_parameters.size; ++i) {
125-
expression_parameters.push_back(options->expression_parameters.data[i]);
126-
}
123+
std::vector<std::string> expression_parameters =
124+
rmw_fastrtps_shared_cpp::prepare_content_filter_parameters(options);
127125

128126
eprosima::fastdds::dds::ReturnCode_t ret =
129127
filtered_topic->set_filter_expression(options->filter_expression, expression_parameters);

rmw_fastrtps_shared_cpp/src/utils.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,10 +102,7 @@ create_content_filtered_topic(
102102
const rmw_subscription_content_filter_options_t * options,
103103
eprosima::fastdds::dds::ContentFilteredTopic ** content_filtered_topic)
104104
{
105-
std::vector<std::string> expression_parameters;
106-
for (size_t i = 0; i < options->expression_parameters.size; ++i) {
107-
expression_parameters.push_back(options->expression_parameters.data[i]);
108-
}
105+
std::vector<std::string> expression_parameters = prepare_content_filter_parameters(options);
109106

110107
auto topic = dynamic_cast<eprosima::fastdds::dds::Topic *>(topic_desc);
111108
static std::atomic<uint32_t> cft_counter{0};

rmw_fastrtps_shared_cpp/test/CMakeLists.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,3 +54,8 @@ ament_add_gtest(test_logging test_logging.cpp)
5454
if(TARGET test_logging)
5555
target_link_libraries(test_logging ${PROJECT_NAME} rmw::rmw)
5656
endif()
57+
58+
ament_add_gtest(test_ensure_dds_literal test_ensure_dds_literal.cpp)
59+
if(TARGET test_ensure_dds_literal)
60+
target_link_libraries(test_ensure_dds_literal ${PROJECT_NAME} rmw::rmw)
61+
endif()
Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
// Copyright 2026 Open Source Robotics Foundation, Inc.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
#include <gtest/gtest.h>
16+
17+
#include <string>
18+
19+
#include "rmw_fastrtps_shared_cpp/utils.hpp"
20+
21+
using rmw_fastrtps_shared_cpp::ensure_dds_literal;
22+
23+
// Bare strings should be wrapped in single quotes
24+
TEST(EnsureDdsLiteral, bare_string_gets_quoted)
25+
{
26+
EXPECT_EQ("'hello'", ensure_dds_literal("hello"));
27+
EXPECT_EQ("'RED'", ensure_dds_literal("RED"));
28+
EXPECT_EQ("'some value'", ensure_dds_literal("some value"));
29+
}
30+
31+
// Already single-quoted strings should pass through unchanged
32+
TEST(EnsureDdsLiteral, already_single_quoted)
33+
{
34+
EXPECT_EQ("'hello'", ensure_dds_literal("'hello'"));
35+
EXPECT_EQ("'with spaces'", ensure_dds_literal("'with spaces'"));
36+
}
37+
38+
// Backtick-quoted strings should pass through unchanged
39+
TEST(EnsureDdsLiteral, already_backtick_quoted)
40+
{
41+
EXPECT_EQ("`hello`", ensure_dds_literal("`hello`"));
42+
}
43+
44+
// Boolean literals should pass through unchanged
45+
TEST(EnsureDdsLiteral, boolean_literals)
46+
{
47+
EXPECT_EQ("TRUE", ensure_dds_literal("TRUE"));
48+
EXPECT_EQ("FALSE", ensure_dds_literal("FALSE"));
49+
}
50+
51+
// Integer literals should pass through unchanged
52+
TEST(EnsureDdsLiteral, integer_literals)
53+
{
54+
EXPECT_EQ("42", ensure_dds_literal("42"));
55+
EXPECT_EQ("0", ensure_dds_literal("0"));
56+
EXPECT_EQ("-1", ensure_dds_literal("-1"));
57+
EXPECT_EQ("+5", ensure_dds_literal("+5"));
58+
}
59+
60+
// Float literals should pass through unchanged
61+
TEST(EnsureDdsLiteral, float_literals)
62+
{
63+
EXPECT_EQ("3.14", ensure_dds_literal("3.14"));
64+
EXPECT_EQ(".5", ensure_dds_literal(".5"));
65+
EXPECT_EQ("-0.1", ensure_dds_literal("-0.1"));
66+
}
67+
68+
// Hex literals should pass through unchanged
69+
TEST(EnsureDdsLiteral, hex_literals)
70+
{
71+
EXPECT_EQ("0xFF", ensure_dds_literal("0xFF"));
72+
EXPECT_EQ("0X1A", ensure_dds_literal("0X1A"));
73+
}
74+
75+
// Empty string should be quoted
76+
TEST(EnsureDdsLiteral, empty_string)
77+
{
78+
EXPECT_EQ("''", ensure_dds_literal(""));
79+
}

0 commit comments

Comments
 (0)