Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Omar-Elmofty opened this issue Apr 8, 2025 · 0 comments

Comments

@Omar-Elmofty
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant