Skip to content

Commit 141797f

Browse files
facontidavideclaude
andcommitted
Fix #883: JSON braces no longer treated as blackboard variables
Enhanced isBlackboardPointer() to validate that content between braces is a valid variable name (matching [a-zA-Z_@][a-zA-Z0-9_./]*) rather than just checking for surrounding braces. This prevents JSON strings like {"key": "value"} from being misidentified as blackboard references. Also centralized the check in xml_parsing.cpp to use TreeNode::isBlackboardPointer() instead of an inline brace check. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 2e72fb3 commit 141797f

3 files changed

Lines changed: 89 additions & 6 deletions

File tree

src/tree_node.cpp

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -383,11 +383,62 @@ bool TreeNode::isBlackboardPointer(StringView str, StringView* stripped_pointer)
383383
}
384384
const auto size = (last_index - front_index) + 1;
385385
auto valid = size >= 3 && str[front_index] == '{' && str[last_index] == '}';
386-
if(valid && stripped_pointer != nullptr)
386+
387+
if(!valid)
388+
{
389+
return false;
390+
}
391+
392+
// Extract content between braces
393+
StringView content(&str[front_index + 1], size - 2);
394+
395+
// Special case: {=} is valid (auto-remapping syntax)
396+
if(content == "=")
397+
{
398+
if(stripped_pointer != nullptr)
399+
{
400+
*stripped_pointer = content;
401+
}
402+
return true;
403+
}
404+
405+
// Validate that the content looks like a blackboard variable name:
406+
// must match [@]?[a-zA-Z_][a-zA-Z0-9_./]*
407+
// The optional '@' prefix refers to the root blackboard.
408+
// This rejects JSON strings like {"key": "value"}.
409+
if(content.empty())
410+
{
411+
return false;
412+
}
413+
size_t start = 0;
414+
if(content.front() == '@')
415+
{
416+
start = 1;
417+
if(start >= content.size())
418+
{
419+
return false;
420+
}
421+
}
422+
const char first_ch = content[start];
423+
if(!std::isalpha(static_cast<unsigned char>(first_ch)) && first_ch != '_')
424+
{
425+
return false;
426+
}
427+
for(size_t i = start + 1; i < content.size(); i++)
428+
{
429+
const char ch = content[i];
430+
if(!std::isalnum(static_cast<unsigned char>(ch)) && ch != '_' && ch != '.' &&
431+
ch != '/')
432+
{
433+
return false;
434+
}
435+
}
436+
437+
if(stripped_pointer != nullptr)
387438
{
388-
*stripped_pointer = StringView(&str[front_index + 1], size - 2);
439+
*stripped_pointer = content;
389440
}
390-
return valid;
441+
return true;
391442
}
392443

393444
StringView TreeNode::stripBlackboardPointer(StringView str)

src/xml_parsing.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -801,9 +801,7 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
801801
else
802802
{
803803
const auto& port_model = port_model_it->second;
804-
const bool is_blackboard = port_value.size() >= 3 &&
805-
port_value.front() == '{' &&
806-
port_value.back() == '}';
804+
const bool is_blackboard = TreeNode::isBlackboardPointer(port_value);
807805
// let's test already if conversion is possible
808806
if(!is_blackboard && port_model.converter() && port_model.isStronglyTyped())
809807
{

tests/gtest_ports.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "behaviortree_cpp/basic_types.h"
22
#include "behaviortree_cpp/bt_factory.h"
33
#include "behaviortree_cpp/json_export.h"
4+
#include "behaviortree_cpp/tree_node.h"
45
#include "behaviortree_cpp/xml_parsing.h"
56

67
#include <gtest/gtest.h>
@@ -815,3 +816,36 @@ TEST(PortTest, SubtreeStringLiteralToLoopDouble_Issue1065)
815816
EXPECT_DOUBLE_EQ(collected[1], 2.0);
816817
EXPECT_DOUBLE_EQ(collected[2], 3.0);
817818
}
819+
820+
// Issue #883: JSON braces should not be treated as blackboard pointers
821+
TEST(PortTest, IsBlackboardPointer_Issue883)
822+
{
823+
using BT::TreeNode;
824+
825+
// Valid blackboard pointers
826+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{my_var}"));
827+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{my.var}"));
828+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{my/var}"));
829+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{_private}"));
830+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{=}"));
831+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{a}"));
832+
EXPECT_TRUE(TreeNode::isBlackboardPointer(" {my_var} "));
833+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{@root_var}"));
834+
835+
// Invalid: JSON strings should NOT be treated as blackboard pointers
836+
EXPECT_FALSE(TreeNode::isBlackboardPointer(R"({"key": "value"})"));
837+
EXPECT_FALSE(TreeNode::isBlackboardPointer(R"({"config": {"nested": true}})"));
838+
839+
// Invalid: other non-variable content
840+
EXPECT_FALSE(TreeNode::isBlackboardPointer("{123}"));
841+
EXPECT_FALSE(TreeNode::isBlackboardPointer("{}"));
842+
EXPECT_FALSE(TreeNode::isBlackboardPointer("ab"));
843+
844+
// Verify stripped_pointer output
845+
BT::StringView stripped;
846+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{foo_bar}", &stripped));
847+
EXPECT_EQ(stripped, "foo_bar");
848+
849+
EXPECT_TRUE(TreeNode::isBlackboardPointer("{=}", &stripped));
850+
EXPECT_EQ(stripped, "=");
851+
}

0 commit comments

Comments
 (0)