Fix NLOHMANN_JSON_SERIALIZE_ENUM_STRICT's from_json message

from_json built its out_of_range.410 message with "..." + j.dump(). If the
unmatched value is (or contains) a string with invalid UTF-8, that dump()
itself throws type_error.316, so the caller got type_error.316 instead of
the documented out_of_range.410; such strings can reach get<Enum>()
unvalidated, e.g. from from_cbor()/from_msgpack(). With a custom string_t,
j.dump() returns that type, and "const char*" + string_t does not compile
unless the type happens to provide operator+, so the macro failed to
compile for such types.

Build the message with detail::concat(), which appends any type exposing
data()/size() and always yields a std::string, and dump with
error_handler_t::replace so building the message itself cannot throw.

Added regression tests: an invalid-UTF-8 case in the existing strict-enum
test in unit-conversions.cpp, and a strict-enum use with alt_string (the
custom string_t from unit-alt-string.cpp) to cover the compile failure.

Fixes #5667.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>

include/nlohmann/detail/macro_scope.hpp

@@ -336,7 +336,7 @@ void templated_json_throw(ExceptionType exception)
             return ej_pair.second == j;                                                         \
         });                                                                                     \
         if (it != std::end(m)) e = it->first;                                                   \
-        else templated_json_throw<nlohmann::detail::out_of_range>(nlohmann::detail::out_of_range::create(410,"enum value out of range for " #ENUM_TYPE ": " + j.dump(), &j)); \
+        else templated_json_throw<nlohmann::detail::out_of_range>(nlohmann::detail::out_of_range::create(410, nlohmann::detail::concat("enum value out of range for " #ENUM_TYPE ": ", j.dump(-1, ' ', false, nlohmann::detail::error_handler_t::replace)), &j)); \
     }
 
 // Ugly macros to avoid uglier copy-paste when specializing basic_json. They

include/nlohmann/json.hpp

@@ -1118,8 +1118,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
         // provides the latter (e.g., ones without a matching allocator-aware
         // fill constructor)
         dst.m_data.m_value.array = create<array_t>();
-        // only now that the array exists may dst stop being a null value
-        dst.m_data.m_type = value_t::array;
         dst.m_data.m_value.array->resize(src_array.size());
 
         auto dst_it = dst.m_data.m_value.array->begin();
@@ -1148,8 +1146,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 
         dst.m_data.m_value.object = create<object_t>(std::make_move_iterator(scratch.begin()),
                                     std::make_move_iterator(scratch.end()));
-        // only now that the object exists may dst stop being a null value
-        dst.m_data.m_type = value_t::object;
         scratch.clear();
 
         // pair every value of the copy with its counterpart in the original;
@@ -1212,6 +1208,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
             src_value = next.first;
             dst_value = next.second;
             worklist.pop_back();
+
+            // the value stops being a null value exactly here
+            dst_value->m_data.m_type = src_value->m_data.m_type;
         }
     }
 

single_include/nlohmann/json.hpp

@@ -2747,7 +2747,7 @@ void templated_json_throw(ExceptionType exception)
             return ej_pair.second == j;                                                         \
         });                                                                                     \
         if (it != std::end(m)) e = it->first;                                                   \
-        else templated_json_throw<nlohmann::detail::out_of_range>(nlohmann::detail::out_of_range::create(410,"enum value out of range for " #ENUM_TYPE ": " + j.dump(), &j)); \
+        else templated_json_throw<nlohmann::detail::out_of_range>(nlohmann::detail::out_of_range::create(410, nlohmann::detail::concat("enum value out of range for " #ENUM_TYPE ": ", j.dump(-1, ' ', false, nlohmann::detail::error_handler_t::replace)), &j)); \
     }
 
 // Ugly macros to avoid uglier copy-paste when specializing basic_json. They
@@ -27199,8 +27199,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
         // provides the latter (e.g., ones without a matching allocator-aware
         // fill constructor)
         dst.m_data.m_value.array = create<array_t>();
-        // only now that the array exists may dst stop being a null value
-        dst.m_data.m_type = value_t::array;
         dst.m_data.m_value.array->resize(src_array.size());
 
         auto dst_it = dst.m_data.m_value.array->begin();
@@ -27229,8 +27227,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
 
         dst.m_data.m_value.object = create<object_t>(std::make_move_iterator(scratch.begin()),
                                     std::make_move_iterator(scratch.end()));
-        // only now that the object exists may dst stop being a null value
-        dst.m_data.m_type = value_t::object;
         scratch.clear();
 
         // pair every value of the copy with its counterpart in the original;
@@ -27293,6 +27289,9 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
             src_value = next.first;
             dst_value = next.second;
             worklist.pop_back();
+
+            // the value stops being a null value exactly here
+            dst_value->m_data.m_type = src_value->m_data.m_type;
         }
     }
 

tests/src/unit-allocator.cpp

@@ -276,133 +276,6 @@ TEST_CASE("controlled bad_alloc")
     }
 }
 
-namespace
-{
-// counts every allocation made on behalf of a basic_json value (of its own
-// object_t/array_t/string_t/binary_t or of its own type), and can be told to
-// fail one of them: the n-th call to allocate() throws std::bad_alloc instead
-// of allocating, whichever type it is allocating for
-std::size_t alloc_call_count = 0;
-long fail_at_alloc_call = -1; // -1: never fail
-
-template<class T>
-struct nth_alloc_fails_allocator : std::allocator<T>
-{
-    using std::allocator<T>::allocator;
-
-    T* allocate(std::size_t n)
-    {
-        const auto index = alloc_call_count++;
-        if (fail_at_alloc_call >= 0 && index == static_cast<std::size_t>(fail_at_alloc_call))
-        {
-            throw std::bad_alloc();
-        }
-        return std::allocator<T>::allocate(n);
-    }
-
-    template <class U>
-    struct rebind
-    {
-        using other = nth_alloc_fails_allocator<U>;
-    };
-};
-
-// builds a value nested more than 128 levels deep - the bound the copy
-// constructor descends into before it continues without the call stack - and
-// checks that a copy survives any single allocation of it failing: every
-// attempt either throws std::bad_alloc, without crashing or leaving the
-// source altered, or completes the copy
-template<class BasicJsonType>
-void check_deep_copy_survives_failing_allocation(bool nest_objects)
-{
-    CAPTURE(nest_objects);
-
-    fail_at_alloc_call = -1;
-
-    // [[[ ... [1] ... ]]], or the same nesting with objects, 130 levels deep
-    BasicJsonType src = 1;
-    for (std::size_t i = 0; i < 130; ++i)
-    {
-        if (nest_objects)
-        {
-            BasicJsonType wrapper = BasicJsonType::object();
-            wrapper["a"] = std::move(src);
-            src = std::move(wrapper);
-        }
-        else
-        {
-            src = BasicJsonType::array({std::move(src)});
-        }
-    }
-
-    const std::string original_dump = src.dump();
-
-    // first measure how many allocations an unhindered copy takes
-    alloc_call_count = 0;
-    {
-        // NOLINTNEXTLINE(performance-unnecessary-copy-initialization): the copy is what is measured
-        const BasicJsonType measure(src);
-    }
-    const std::size_t total_allocations = alloc_call_count;
-    REQUIRE(total_allocations > 0);
-    REQUIRE(src.dump() == original_dump);
-
-    // let the 0th, 1st, 2nd, ... allocation of the copy fail in turn; every
-    // such copy must throw std::bad_alloc rather than crash, and the source
-    // must come out exactly as it went in
-    for (std::size_t n = 0; n < total_allocations; ++n)
-    {
-        CAPTURE(n);
-        alloc_call_count = 0;
-        fail_at_alloc_call = static_cast<long>(n);
-
-        CHECK_THROWS_AS(BasicJsonType(src), std::bad_alloc&);
-
-        fail_at_alloc_call = -1;
-        CHECK(src.dump() == original_dump);
-    }
-
-    // once no allocation is made to fail, the copy itself must succeed
-    fail_at_alloc_call = -1;
-    const BasicJsonType copy(src);
-    CHECK(copy.dump() == original_dump);
-    CHECK(src.dump() == original_dump);
-}
-} // namespace
-
-TEST_CASE("copy of a deeply nested value survives a failing allocation (#5640)")
-{
-    SECTION("std::map-backed object_t")
-    {
-        using bad_alloc_json = nlohmann::basic_json<std::map,
-              std::vector,
-              std::string,
-              bool,
-              std::int64_t,
-              std::uint64_t,
-              double,
-              nth_alloc_fails_allocator>;
-
-        check_deep_copy_survives_failing_allocation<bad_alloc_json>(false);
-        check_deep_copy_survives_failing_allocation<bad_alloc_json>(true);
-    }
-
-    SECTION("ordered_map-backed object_t")
-    {
-        using bad_alloc_ordered_json = nlohmann::basic_json<nlohmann::ordered_map,
-              std::vector,
-              std::string,
-              bool,
-              std::int64_t,
-              std::uint64_t,
-              double,
-              nth_alloc_fails_allocator>;
-
-        check_deep_copy_survives_failing_allocation<bad_alloc_ordered_json>(false);
-        check_deep_copy_survives_failing_allocation<bad_alloc_ordered_json>(true);
-    }
-}
-
 namespace
 {
 // counts the allocations of pairs with a non-const first member: the object

tests/src/unit-alt-string.cpp

@@ -174,6 +174,15 @@ bool operator<(const char* op1, const alt_string& op2) noexcept
     return op1 < op2.str_impl;
 }
 
+enum class alt_color { red, green };
+
+// NOLINTNEXTLINE(misc-use-internal-linkage,misc-const-correctness,cppcoreguidelines-avoid-c-arrays,hicpp-avoid-c-arrays,modernize-avoid-c-arrays) - false positive
+NLOHMANN_JSON_SERIALIZE_ENUM_STRICT(alt_color,
+{
+    {alt_color::red, "red"},
+    {alt_color::green, "green"},
+})
+
 TEST_CASE("alternative string type")
 {
     SECTION("binary formats")
@@ -374,4 +383,19 @@ TEST_CASE("alternative string type")
         const auto j2 = j.flatten();
         CHECK(j2.dump() == R"({"/foo/0":"bar","/foo/1":"baz"})");
     }
+
+    SECTION("strict enum")
+    {
+        // regression test for #5667: NLOHMANN_JSON_SERIALIZE_ENUM_STRICT's from_json
+        // built its exception message with "..." + j.dump(), which does not compile
+        // when j.dump() returns a custom string_t (here alt_string) instead of
+        // std::string
+        alt_json doc;
+        doc = "red";
+        CHECK(doc.get<alt_color>() == alt_color::red);
+
+        alt_json _;
+        doc = "blue";
+        CHECK_THROWS_WITH_AS(_ = doc.get<alt_color>(), "[json.exception.out_of_range.410] enum value out of range for alt_color: \"blue\"", alt_json::out_of_range&);
+    }
 }

tests/src/unit-conversions.cpp

@@ -1740,6 +1740,12 @@ TEST_CASE("Strict JSON to enum mapping")
 
         // conversion of unmapped enum -> exception thrown
         CHECK_THROWS_WITH_AS(json(strict_cards::andere), "[json.exception.out_of_range.410] enum value out of range for strict_cards", json::out_of_range&);
+
+        // invalid UTF-8 -> out_of_range.410, not the type_error.316 thrown while building the
+        // message (regression test for #5667); such strings can reach get<Enum>() unvalidated,
+        // e.g. from from_cbor()/from_msgpack() (#5529)
+        const json j_invalid_utf8 = "\xFF";
+        CHECK_THROWS_WITH_AS(_ = j_invalid_utf8.get<strict_cards>(), "[json.exception.out_of_range.410] enum value out of range for strict_cards: \"\xEF\xBF\xBD\"", json::out_of_range&);
     }
 
     SECTION("traditional enum")