Merge branch 'master' into fuzzing

Author: cktii

.github/workflows/sonarcube.yml

@@ -0,0 +1,40 @@
+name: Sonarcube Scan
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    types: [opened, synchronize, reopened]
+jobs:
+  build:
+    name: Build
+    runs-on: ubuntu-latest
+    env:
+      BUILD_WRAPPER_OUT_DIR: build_wrapper_output_directory # Directory where build-wrapper output will be placed
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0 # Shallow clones should be disabled for a better relevancy of analysis
+      - name: Install Build Wrapper
+        uses: SonarSource/sonarqube-scan-action/install-build-wrapper@v4
+
+      - name: Install Dependencies
+        run: |
+          sudo apt-get update
+          sudo apt-get install -y libzmq3-dev libsqlite3-dev
+
+      - name: Install googletest
+        uses: Bacondish2023/setup-googletest@v1
+
+      - name: Run Build Wrapper
+        run: |
+          mkdir build
+          cmake -S . -B build
+          build-wrapper-linux-x86-64 --out-dir ${{ env.BUILD_WRAPPER_OUT_DIR }} cmake --build build/ --config Release
+      - name: SonarQube Scan
+        uses: SonarSource/sonarqube-scan-action@v4
+        env:
+          SONAR_TOKEN: ${{ secrets.SONAR_TOKEN }} # Put the name of your token here
+        with:
+          args: >
+            --define sonar.cfamily.compile-commands="${{ env.BUILD_WRAPPER_OUT_DIR }}/compile_commands.json"

CMakeLists.txt

@@ -2,6 +2,9 @@ cmake_minimum_required(VERSION 3.16.3) # version on Ubuntu Focal
 
 project(behaviortree_cpp VERSION 4.6.2 LANGUAGES C CXX)
 
+# create compile_commands.json
+set(CMAKE_EXPORT_COMPILE_COMMANDS ON)
+
 #---- project configuration ----
 option(BTCPP_SHARED_LIBS "Build shared libraries" ON)
 option(BTCPP_BUILD_TOOLS "Build commandline tools" ON)

cmake/ament_build.cmake

@@ -12,6 +12,9 @@ endif()
 
 find_package(ament_index_cpp REQUIRED)
 
+set(BTCPP_EXTRA_INCLUDE_DIRS ${ZeroMQ_INCLUDE_DIRS}
+                             ${SQLite3_INCLUDE_DIRS})
+
 set( BTCPP_EXTRA_LIBRARIES
     $<BUILD_INTERFACE:ament_index_cpp::ament_index_cpp>
     $<BUILD_INTERFACE:${ZeroMQ_LIBRARIES}>
@@ -26,6 +29,7 @@ set( BTCPP_BIN_DESTINATION     bin )
 
 mark_as_advanced(
     BTCPP_EXTRA_LIBRARIES
+    BTCPP_EXTRA_INCLUDE_DIRS
     BTCPP_LIB_DESTINATION
     BTCPP_INCLUDE_DESTINATION
     BTCPP_BIN_DESTINATION )

include/behaviortree_cpp/bt_factory.h

@@ -109,8 +109,8 @@ class Tree
   Tree(const Tree&) = delete;
   Tree& operator=(const Tree&) = delete;
 
-  Tree(Tree&& other);
-  Tree& operator=(Tree&& other);
+  Tree(Tree&& other) = default;
+  Tree& operator=(Tree&& other) = default;
 
   void initialize();
 
@@ -149,7 +149,7 @@ class Tree
   [[nodiscard]] Blackboard::Ptr rootBlackboard();
 
   //Call the visitor for each node of the tree.
-  void applyVisitor(const std::function<void(const TreeNode*)>& visitor);
+  void applyVisitor(const std::function<void(const TreeNode*)>& visitor) const;
 
   //Call the visitor for each node of the tree.
   void applyVisitor(const std::function<void(TreeNode*)>& visitor);
@@ -215,8 +215,8 @@ class BehaviorTreeFactory
   BehaviorTreeFactory(const BehaviorTreeFactory& other) = delete;
   BehaviorTreeFactory& operator=(const BehaviorTreeFactory& other) = delete;
 
-  BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept;
-  BehaviorTreeFactory& operator=(BehaviorTreeFactory&& other) noexcept;
+  BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept = default;
+  BehaviorTreeFactory& operator=(BehaviorTreeFactory&& other) noexcept = default;
 
   /// Remove a registered ID.
   bool unregisterBuilder(const std::string& ID);

include/behaviortree_cpp/decorators/loop_node.h

@@ -60,7 +60,8 @@ class LoopNode : public DecoratorNode
       // special case: the port contains a string that was converted to SharedQueue<T>
       if(static_queue_)
       {
-        current_queue_ = static_queue_;
+        current_queue_ = std::make_shared<std::deque<T>>();
+        *current_queue_ = *static_queue_;
       }
     }
 

include/behaviortree_cpp/json_export.h

@@ -114,12 +114,12 @@ inline Expected<T> JsonExporter::fromJson(const nlohmann::json& source) const
   auto res = fromJson(source);
   if(!res)
   {
-    return nonstd::expected_lite::make_unexpected(res.error());
+    return nonstd::make_unexpected(res.error());
   }
   auto casted = res->first.tryCast<T>();
   if(!casted)
   {
-    return nonstd::expected_lite::make_unexpected(casted.error());
+    return nonstd::make_unexpected(casted.error());
   }
   return *casted;
 }

include/behaviortree_cpp/scripting/operators.hpp

@@ -129,6 +129,11 @@ struct ExprUnaryArithmetic : ExprBase
         case negate:
           return Any(-rv);
         case complement:
+          if(rv > static_cast<double>(std::numeric_limits<int64_t>::max()) ||
+             rv < static_cast<double>(std::numeric_limits<int64_t>::min()))
+          {
+            throw RuntimeError("Number out of range for bitwise operation");
+          }
           return Any(static_cast<double>(~static_cast<int64_t>(rv)));
         case logical_not:
           return Any(static_cast<double>(!static_cast<bool>(rv)));

include/behaviortree_cpp/tree_node.h

@@ -598,7 +598,8 @@ inline Result TreeNode::setOutput(const std::string& key, const T& value)
 
   if constexpr(std::is_same_v<BT::Any, T>)
   {
-    if(config().manifest->ports.at(key).type() != typeid(BT::Any))
+    auto port_type = config().manifest->ports.at(key).type();
+    if(port_type != typeid(BT::Any) && port_type != typeid(BT::AnyTypeAllowed))
     {
       throw LogicError("setOutput<Any> is not allowed, unless the port "
                        "was declared using OutputPort<Any>");

include/behaviortree_cpp/utils/convert_impl.hpp

@@ -13,6 +13,7 @@
 #pragma once
 
 #include <type_traits>
+#include <cmath>
 #include "simple_string.hpp"
 
 #undef max
@@ -88,9 +89,42 @@ inline void checkLowerLimit(const From& from)
 template <typename From, typename To>
 inline void checkTruncation(const From& from)
 {
-  if(from != static_cast<From>(static_cast<To>(from)))
+  // Handle integer to floating point
+  if constexpr(std::is_integral_v<From> && std::is_floating_point_v<To>)
   {
-    throw std::runtime_error("Floating point truncated");
+    // Check if value can be represented exactly in the target type
+    constexpr auto max_exact = (1LL << std::numeric_limits<double>::digits) - 1;
+    if(from > max_exact || from < -max_exact)
+    {
+      throw std::runtime_error("Loss of precision when converting a large integer number "
+                               "to floating point:" +
+                               std::to_string(from));
+    }
+  }
+  // Handle floating point to integer
+  else if constexpr(std::is_floating_point_v<From> && std::is_integral_v<To>)
+  {
+    if(from > static_cast<From>(std::numeric_limits<To>::max()) ||
+       from < static_cast<From>(std::numeric_limits<To>::lowest()) ||
+       from != std::nearbyint(from))
+    {
+      throw std::runtime_error("Invalid floating point to integer conversion");
+    }
+  }
+  // Handle other conversions
+  else
+  {
+    if(from > static_cast<From>(std::numeric_limits<To>::max()) ||
+       from < static_cast<From>(std::numeric_limits<To>::lowest()))
+    {
+      throw std::runtime_error("Value outside numeric limits");
+    }
+    To as_target = static_cast<To>(from);
+    From back_to_source = static_cast<From>(as_target);
+    if(from != back_to_source)
+    {
+      throw std::runtime_error("Value truncated in conversion");
+    }
   }
 }
 

include/behaviortree_cpp/utils/safe_any.hpp

@@ -245,7 +245,34 @@ class Any
 template <typename SRC, typename TO>
 inline bool ValidCast(const SRC& val)
 {
-  return (val == static_cast<SRC>(static_cast<TO>(val)));
+  // First check numeric limits
+  if constexpr(std::is_arithmetic_v<SRC> && std::is_arithmetic_v<TO>)
+  {
+    // Handle conversion to floating point
+    if constexpr(std::is_floating_point_v<TO>)
+    {
+      if constexpr(std::is_integral_v<SRC>)
+      {
+        // For integral to float, check if we can represent the value exactly
+        TO as_float = static_cast<TO>(val);
+        SRC back_conv = static_cast<SRC>(as_float);
+        return back_conv == val;
+      }
+    }
+    // Handle conversion to integral
+    else if constexpr(std::is_integral_v<TO>)
+    {
+      if(val > static_cast<SRC>(std::numeric_limits<TO>::max()) ||
+         val < static_cast<SRC>(std::numeric_limits<TO>::lowest()))
+      {
+        return false;
+      }
+    }
+  }
+
+  TO as_target = static_cast<TO>(val);
+  SRC back_to_source = static_cast<SRC>(as_target);
+  return val == back_to_source;
 }
 
 template <typename T>

sonar-project.properties

@@ -0,0 +1,13 @@
+sonar.projectKey=BehaviorTree_BehaviorTree.CPP
+sonar.organization=behaviortree
+
+# This is the name and version displayed in the SonarCloud UI.
+#sonar.projectName=BehaviorTree.CPP
+#sonar.projectVersion=1.0
+
+
+# Path is relative to the sonar-project.properties file. Replace "\" by "/" on Windows.
+#sonar.sources=.
+
+# Encoding of the source code. Default is default system encoding
+#sonar.sourceEncoding=UTF-8

src/blackboard.cpp

@@ -143,6 +143,10 @@ void Blackboard::createEntry(const std::string& key, const TypeInfo& info)
 {
   if(StartWith(key, '@'))
   {
+    if(key.find('@', 1) != std::string::npos)
+    {
+      throw LogicError("Character '@' used multiple times in the key");
+    }
     rootBlackboard()->createEntryImpl(key.substr(1, key.size() - 1), info);
   }
   else

src/bt_factory.cpp

@@ -107,17 +107,6 @@ BehaviorTreeFactory::BehaviorTreeFactory() : _p(new PImpl)
   _p->scripting_enums = std::make_shared<std::unordered_map<std::string, int>>();
 }
 
-BehaviorTreeFactory::BehaviorTreeFactory(BehaviorTreeFactory&& other) noexcept
-{
-  this->_p = std::move(other._p);
-}
-
-BehaviorTreeFactory& BehaviorTreeFactory::operator=(BehaviorTreeFactory&& other) noexcept
-{
-  this->_p = std::move(other._p);
-  return *this;
-}
-
 BehaviorTreeFactory::~BehaviorTreeFactory()
 {}
 
@@ -531,22 +520,9 @@ BehaviorTreeFactory::substitutionRules() const
   return _p->substitution_rules;
 }
 
-Tree& Tree::operator=(Tree&& other)
-{
-  subtrees = std::move(other.subtrees);
-  manifests = std::move(other.manifests);
-  wake_up_ = other.wake_up_;
-  return *this;
-}
-
 Tree::Tree()
 {}
 
-Tree::Tree(Tree&& other)
-{
-  (*this) = std::move(other);
-}
-
 void Tree::initialize()
 {
   wake_up_ = std::make_shared<WakeUpSignal>();
@@ -621,7 +597,7 @@ Blackboard::Ptr Tree::rootBlackboard()
   return {};
 }
 
-void Tree::applyVisitor(const std::function<void(const TreeNode*)>& visitor)
+void Tree::applyVisitor(const std::function<void(const TreeNode*)>& visitor) const
 {
   BT::applyRecursiveVisitor(static_cast<const TreeNode*>(rootNode()), visitor);
 }

src/xml_parsing.cpp

@@ -242,6 +242,10 @@ void XMLParser::PImpl::loadDocImpl(XMLDocument* doc, bool add_includes)
   }
 
   const XMLElement* xml_root = doc->RootElement();
+  if(!xml_root)
+  {
+    throw RuntimeError("Invalid XML: missing root element");
+  }
 
   auto format = xml_root->Attribute("BTCPP_format");
   if(!format)
@@ -548,8 +552,13 @@ void VerifyXML(const std::string& xml_text,
           for(auto child = node->FirstChildElement(); child != nullptr;
               child = child->NextSiblingElement())
           {
-            const std::string child_name = node->FirstChildElement()->Name();
+            const std::string child_name = child->Name();
             const auto child_search = registered_nodes.find(child_name);
+            if(child_search == registered_nodes.end())
+            {
+              ThrowError(child->GetLineNum(),
+                         std::string("Unknown node type: ") + child_name);
+            }
             const auto child_type = child_search->second;
             if(child_type == NodeType::CONTROL &&
                ((child_name == "ThreadedAction") ||

tests/gtest_factory.cpp

@@ -104,6 +104,13 @@ static const char* xml_text_subtree_part2 = R"(
 
 // clang-format on
 
+TEST(BehaviorTreeFactory, NotRegisteredNode)
+{
+  BehaviorTreeFactory factory;
+  ASSERT_ANY_THROW(factory.createTreeFromText(xml_text));
+  ASSERT_ANY_THROW(std::make_shared<BT::Tree>(factory.createTreeFromText(xml_text)));
+}
+
 TEST(BehaviorTreeFactory, XMLParsingOrder)
 {
   BehaviorTreeFactory factory;