From acf076a677c161643e64d8f83fd02624fd8c4bd3 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 6 Jul 2026 12:47:24 +0200 Subject: [PATCH] Fix ADL leak of nlohmann::detail through basic_json's default base class (#5238) --- .../detail/json_custom_base_class.hpp | 18 +++- single_include/nlohmann/json.hpp | 18 +++- tests/src/unit-regression2.cpp | 96 +++++++++++++++++++ 3 files changed, 126 insertions(+), 6 deletions(-) diff --git a/include/nlohmann/detail/json_custom_base_class.hpp b/include/nlohmann/detail/json_custom_base_class.hpp index a4b86f2b4..c1f1a5449 100644 --- a/include/nlohmann/detail/json_custom_base_class.hpp +++ b/include/nlohmann/detail/json_custom_base_class.hpp @@ -13,8 +13,6 @@ #include NLOHMANN_JSON_NAMESPACE_BEGIN -namespace detail -{ /*! @brief Default base class of the @ref basic_json class. @@ -25,13 +23,27 @@ of @ref basic_json do not require complex case distinctions @ref basic_json always has a base class. By default, this class is used because it is empty and thus has no effect on the behavior of @ref basic_json. + +@note This class intentionally lives in namespace @ref nlohmann rather than + @ref nlohmann::detail. Every @ref basic_json specialization derives from + it (via @ref detail::json_base_class) unless a custom base class is + supplied, which makes its namespace an associated namespace of + @ref basic_json for the purpose of argument-dependent lookup (ADL). If + it lived in `nlohmann::detail`, that namespace - and with it the + library's internal `to_json`/`from_json` overloads - would leak into + ADL for any unqualified `to_json`/`from_json` call a user makes + involving a @ref basic_json argument, silently shadowing the user's own + overloads in some cases. */ struct json_default_base {}; +namespace detail +{ + template using json_base_class = typename std::conditional < std::is_same::value, - json_default_base, + ::nlohmann::json_default_base, T >::type; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index d93bcc8bd..cf7cb31c3 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -15094,8 +15094,6 @@ NLOHMANN_JSON_NAMESPACE_END NLOHMANN_JSON_NAMESPACE_BEGIN -namespace detail -{ /*! @brief Default base class of the @ref basic_json class. @@ -15106,13 +15104,27 @@ of @ref basic_json do not require complex case distinctions @ref basic_json always has a base class. By default, this class is used because it is empty and thus has no effect on the behavior of @ref basic_json. + +@note This class intentionally lives in namespace @ref nlohmann rather than + @ref nlohmann::detail. Every @ref basic_json specialization derives from + it (via @ref detail::json_base_class) unless a custom base class is + supplied, which makes its namespace an associated namespace of + @ref basic_json for the purpose of argument-dependent lookup (ADL). If + it lived in `nlohmann::detail`, that namespace - and with it the + library's internal `to_json`/`from_json` overloads - would leak into + ADL for any unqualified `to_json`/`from_json` call a user makes + involving a @ref basic_json argument, silently shadowing the user's own + overloads in some cases. */ struct json_default_base {}; +namespace detail +{ + template using json_base_class = typename std::conditional < std::is_same::value, - json_default_base, + ::nlohmann::json_default_base, T >::type; diff --git a/tests/src/unit-regression2.cpp b/tests/src/unit-regression2.cpp index bf2d87767..3be8f9d5c 100644 --- a/tests/src/unit-regression2.cpp +++ b/tests/src/unit-regression2.cpp @@ -1358,4 +1358,100 @@ TEST_CASE("regression test #5122 - nlohmann::ordered_map move-assignment transfe CHECK(src.begin()->first == "after-move"); } +// Stand-in for a third-party library (e.g., Eigen as of 3.4, which added +// STL-compatible begin()/end() to its vector types), living in its own +// namespace with its own to_json overload for its vector type. +namespace issue_4320_eigen +{ +// "array-compatible" from the library's point of view (it has begin()/end()), +// but for which this (fake) third-party namespace provides its own to_json. +struct vector3 +{ + double v[3]; // NOLINT(cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays,cppcoreguidelines-use-default-member-init,modernize-use-default-member-init) + vector3(double x, double y, double z) : v{x, y, z} {} // NOLINT(hicpp-member-init,cppcoreguidelines-pro-type-member-init) + double x() const + { + return v[0]; + } + double y() const + { + return v[1]; + } + double z() const + { + return v[2]; + } + double* begin() + { + return v; + } + double* end() + { + return v + 3; + } + const double* begin() const + { + return v; + } + const double* end() const + { + return v + 3; + } +}; + +inline void to_json(json& j, const vector3& v) // NOLINT(misc-use-internal-linkage) +{ + j = {{"x", v.x()}, {"y", v.y()}, {"z", v.z()}}; +} +} // namespace issue_4320_eigen + +// The user's own namespace, using the (fake) Eigen type as an implementation +// detail behind a payload type that has nothing to do with vectors/arrays. +namespace issue_4320 +{ +// Publicly derives from issue_4320_eigen::vector3 but does *not* define its +// own to_json - it is only ever used as a temporary to reach the base +// class's to_json via ADL. +struct vector3_wrapper : issue_4320_eigen::vector3 +{ + using issue_4320_eigen::vector3::vector3; +}; + +struct payload +{ + double x, y, z; +}; + +inline vector3_wrapper to_eigen(const payload& p) // NOLINT(misc-use-internal-linkage) +{ + return {p.x, p.y, p.z}; +} + +inline void to_json(json& j, const payload& p) // NOLINT(misc-use-internal-linkage) +{ + // Unqualified call, passing a *derived* vector3_wrapper: relies on ADL + // finding issue_4320_eigen::to_json(json&, const vector3&) through the + // vector3 base class, via a derived-to-base conversion. Must NOT resolve + // to the library's own generic array-compatible to_json (an exact-match + // template for vector3_wrapper, since it also has begin()/end()), which + // would serialize this as [x, y, z] instead of {"x":x, "y":y, "z":z}. + to_json(j, to_eigen(p)); +} +} // namespace issue_4320 + +TEST_CASE("issue #4320 - custom base class must not leak nlohmann::detail into ADL") +{ + // Before the fix, basic_json unconditionally derived from a type living in + // nlohmann::detail (json_default_base), which made nlohmann::detail an + // associated namespace of every basic_json for ADL purposes. That leaked + // the library's internal generic-array to_json overload into unqualified + // to_json() calls made from user code, silently bypassing user-defined + // to_json overloads reached via a derived-to-base conversion. + const issue_4320::payload p{1.0, 2.0, 3.0}; + + json j; + to_json(j, p); + CHECK(j == json({{"x", 1.0}, {"y", 2.0}, {"z", 3.0}})); +} + DOCTEST_CLANG_SUPPRESS_WARNING_POP