Skip to content

Version conflict with embedded nlohmann::json can cause undefined behavior #959

Open
@Omar-Elmofty

Description

@Omar-Elmofty

Currently, BehaviorTree.CPP has the nlohmann::json v.3.11.3 header embedded in the source code under include/behaviortree_cpp/contrib. This means that whenever you're including headers from behaviortree_cpp, you'll be bring this version of nlohmann::json in your codebase.

The problem is that if the codebase already uses a different version of nlohmann::json (v3.8.0 in my case), you can run into scenarios where, depending on the order of the header files, either of the versions will get used. That's because the header guards across version for nlohmann::json are the same.

Here is an example to illustrate:

#include <behaviortree_cpp/blackboard.h> # includes nlohman::json v3.11.3
#include <nlohmann/json.hpp> # v3.8.0 provided by my codebase

nlohman::json my_json_object; // this will be version v3.11.3 since it was included first

and if we swap the order of the headers in the above example, the version of my_json_object will be v3.8.0

This can cause some unintended consequences where a json object of one version could be converted to another version of a json object.

This is an example on how to reproduce the issue

// First translation unit
#include <behaviortree_cpp/blackboard.h> #  v3.11.3
#include <nlohmann/json.hpp> # v3.8.0 

nlohman::json my_json_object; // v3.11.3

// convert from nlohmann::json to std::any
std::any my_object_converted = my_json_object
// pass a pointer of my_object_converted to the second translation unit


// Second translation unit
#include <nlohmann/json.hpp> # v3.8.0 
#include <behaviortree_cpp/blackboard.h> #  v3.11.3

// converting back from std::any to nlohman::json of version v3.8.0
// This will cause undefined behavior (if the versions are not backward-compatible)
nlohman::json my_json_object = std::any_cast<nlohmann::json>(my_object_converted);

Here is a real example where this issue has manifested itself.
After bumping BT.CPP to v4.6.2, we get this check where there is a type mismatch between the declared type of the port (which uses one version of nlohmann::json) and the actual type of the data (which uses a different version of nlohmann::json). On v.4.5.2 we didn't have this failure, which meant that we were silently converting between the 2 versions of nlohmann::json.

Uncaught exception: Blackboard::set(global_replan_params): once declared, the type of a port shall not change. Previously declared type [std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, nlohmann::json_abi_v3_11_3::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void>, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, nlohmann::json_abi_v3_11_3::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::json_abi_v3_11_3::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> >, void> > > >], current type [std::map<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, nlohmann::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> > >, std::less<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, nlohmann::basic_json<std::map, std::vector, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, bool, long, unsigned long, double, std::allocator, nlohmann::adl_serializer, std::vector<unsigned char, std::allocator<unsigned char> > > > > >]

Can we add a CMake option to opt out of the version of nlohmann::json embeddded inside BT.CPP and use the version provided by the user's code base? Same applies to other headers in include/behaviortree_cpp/contrib

FYI @jasonimercer @iwanders

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions