cleanup and fix validation

Author: facontidavide

.pre-commit-config.yaml

@@ -61,15 +61,3 @@ repos:
         -   tomli
         args:
             [--toml=./pyproject.toml]
-
-  # Include What You Use (requires: apt install iwyu, and compile_commands.json)
-  - repo: local
-    hooks:
-      - id: iwyu
-        name: include-what-you-use
-        entry: iwyu_tool -p build
-        language: system
-        files: \.(cpp|hpp|h)$
-        exclude: ^3rdparty/|^include/behaviortree_cpp/contrib/
-        pass_filenames: false
-        stages: [manual]

include/behaviortree_cpp/action_node.h

@@ -19,7 +19,6 @@
 #include <atomic>
 #include <future>
 #include <mutex>
-#include <thread>
 
 namespace BT
 {

include/behaviortree_cpp/basic_types.h

@@ -348,6 +348,11 @@ struct Timestamp
 
 [[nodiscard]] bool IsReservedAttribute(StringView str);
 
+/// Returns the first forbidden character found in the name, or '\0' if valid.
+/// Forbidden characters include: space, tab, newline, CR, < > & " ' / \ : * ? | .
+/// and control characters (ASCII 0-31, 127). UTF-8 multibyte sequences are allowed.
+[[nodiscard]] char findForbiddenChar(StringView name);
+
 class TypeInfo
 {
 public:

src/basic_types.cpp

@@ -447,10 +447,16 @@ bool IsAllowedPortName(StringView str)
     return false;
   }
   const char first_char = str.data()[0];
+  // Port name cannot start with a digit
   if(std::isalpha(static_cast<unsigned char>(first_char)) == 0)
   {
     return false;
   }
+  // Check for forbidden characters
+  if(findForbiddenChar(str) != '\0')
+  {
+    return false;
+  }
   return !IsReservedAttribute(str);
 }
 
@@ -473,6 +479,36 @@ bool IsReservedAttribute(StringView str)
   return str == "name" || str == "ID" || str == "_autoremap";
 }
 
+char findForbiddenChar(StringView name)
+{
+  // Forbidden characters that break XML serialization or cause filesystem issues
+  static constexpr std::array<char, 16> kForbiddenChars = {
+    ' ', '\t', '\n', '\r', '<', '>', '&', '"', '\'', '/', '\\', ':', '*', '?', '|', '.'
+  };
+
+  for(const char c : name)
+  {
+    const auto uc = static_cast<unsigned char>(c);
+    // Allow UTF-8 multibyte sequences (high bit set)
+    if(uc >= 0x80)
+    {
+      continue;
+    }
+    // Block control characters (ASCII 0-31 and 127)
+    if(uc < 32 || uc == 127)
+    {
+      return c;
+    }
+    // Check forbidden character list
+    if(std::find(kForbiddenChars.begin(), kForbiddenChars.end(), c) !=
+       kForbiddenChars.end())
+    {
+      return c;
+    }
+  }
+  return '\0';
+}
+
 Any convertFromJSON(StringView json_text, std::type_index type)
 {
   const nlohmann::json json = nlohmann::json::parse(json_text);

src/behavior_tree.cpp

@@ -12,8 +12,6 @@
 
 #include "behaviortree_cpp/behavior_tree.h"
 
-#include <cstring>
-
 namespace BT
 {
 void applyRecursiveVisitor(const TreeNode* node,

src/xml_parsing.cpp

@@ -309,6 +309,8 @@ void BT::XMLParser::PImpl::loadSubtreeModel(const XMLElement* xml_root)
           {
             throw RuntimeError("Missing attribute [name] in port (SubTree model)");
           }
+          // Validate port name
+          validatePortName(name, port_node->GetLineNum());
           if(auto default_value = port_node->Attribute("default"))
           {
             port.setDefaultValue(default_value);
@@ -550,6 +552,11 @@ void VerifyXML(const std::string& xml_text,
       {
         ThrowError(line_number, "The tag <BehaviorTree> must have the attribute [ID]");
       }
+      // Validate BehaviorTree ID as a model name
+      if(!ID.empty())
+      {
+        validateModelName(ID, line_number);
+      }
       if(registered_nodes.count(ID) != 0)
       {
         ThrowError(line_number, "The attribute [ID] of tag <BehaviorTree> must not use "
@@ -568,6 +575,8 @@ void VerifyXML(const std::string& xml_text,
         ThrowError(line_number,
                    "<SubTree> with ID '" + ID + "' should not have any child");
       }
+      // Validate SubTree ID as a model name
+      validateModelName(ID, line_number);
       if(registered_nodes.count(ID) != 0)
       {
         ThrowError(line_number, "The attribute [ID] of tag <SubTree> must not use the "
@@ -578,6 +587,11 @@ void VerifyXML(const std::string& xml_text,
     {
       // use ID for builtin node types, otherwise use the element name
       const auto lookup_name = is_builtin ? ID : name;
+      // Validate model name for custom node types (non-builtin element names)
+      if(!is_builtin)
+      {
+        validateModelName(name, line_number);
+      }
       const auto search = registered_nodes.find(lookup_name);
       const bool found = (search != registered_nodes.end());
       if(!found)
@@ -743,6 +757,12 @@ TreeNode::Ptr XMLParser::PImpl::createNodeFromXML(const XMLElement* element,
   const char* attr_name = element->Attribute("name");
   const std::string instance_name = (attr_name != nullptr) ? attr_name : type_ID;
 
+  // Validate instance name if explicitly provided
+  if(attr_name != nullptr)
+  {
+    validateInstanceName(instance_name, element->GetLineNum());
+  }
+
   const TreeNodeManifest* manifest = nullptr;
 
   auto manifest_it = factory->manifests().find(type_ID);