Skip to content

Commit d1098a6

Browse files
fix SimpleString equality for content with embedded NUL bytes (#1230)
strncmp stops at the first NUL byte, so two different strings of equal length that agree up to an embedded NUL compared equal, while operator< ordered the same pair with memcmp. Embedded NUL bytes reach a blackboard string through ImportBlackboardFromJSON, where a JSON escape decodes to a real NUL, and the scripting == and != operators compare through SimpleString.
1 parent 6a3b337 commit d1098a6

2 files changed

Lines changed: 30 additions & 2 deletions

File tree

‎include/behaviortree_cpp/utils/simple_string.hpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,13 +110,16 @@ class SimpleString
110110
bool operator==(const SimpleString& other) const
111111
{
112112
const size_t N = size();
113-
return other.size() == N && std::strncmp(data(), other.data(), N) == 0;
113+
// memcmp, not strncmp: the content may hold embedded NUL bytes, and strncmp
114+
// stops at the first one, reporting two different strings of equal length as
115+
// equal. It is also what operator< below compares with.
116+
return other.size() == N && std::memcmp(data(), other.data(), N) == 0;
114117
}
115118

116119
bool operator!=(const SimpleString& other) const
117120
{
118121
const size_t N = size();
119-
return other.size() != N || std::strncmp(data(), other.data(), N) != 0;
122+
return other.size() != N || std::memcmp(data(), other.data(), N) != 0;
120123
}
121124

122125
bool operator<=(const SimpleString& other) const

‎tests/gtest_simple_string.cpp‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -377,6 +377,31 @@ TEST(SimpleStringTest, EmptyStringComparison)
377377
EXPECT_TRUE(nonEmpty >= empty1);
378378
}
379379

380+
// Test comparison of strings that hold embedded NUL bytes
381+
TEST(SimpleStringTest, ComparisonWithEmbeddedNull)
382+
{
383+
const SimpleString s1(std::string("a\0b", 3));
384+
const SimpleString s2(std::string("a\0c", 3));
385+
const SimpleString s3(std::string("a\0b", 3));
386+
387+
EXPECT_FALSE(s1 == s2);
388+
EXPECT_TRUE(s1 != s2);
389+
EXPECT_TRUE(s1 < s2);
390+
EXPECT_FALSE(s1 > s2);
391+
392+
EXPECT_TRUE(s1 == s3);
393+
EXPECT_FALSE(s1 != s3);
394+
395+
// same thing with the heap representation
396+
const SimpleString l1(std::string("0123456789abcdef\0b", 18));
397+
const SimpleString l2(std::string("0123456789abcdef\0c", 18));
398+
399+
EXPECT_FALSE(l1.isSOO());
400+
EXPECT_FALSE(l1 == l2);
401+
EXPECT_TRUE(l1 != l2);
402+
EXPECT_TRUE(l1 < l2);
403+
}
404+
380405
// Test that SimpleString size is as expected (16 bytes)
381406
TEST(SimpleStringTest, SizeOfSimpleString)
382407
{

0 commit comments

Comments
 (0)