From 590b622d481d6ec350a42c047bd5fe23baec4a13 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Mon, 2 Nov 2026 18:25:30 +0100 Subject: [PATCH] parse(): propagate std::bad_alloc instead of crashing on a half-constructed member (#5991) * Create the DOM parser's member value before placing it (#5985) json_sax_dom_parser::set_member() constructed the new value in place over the null member. basic_json(value_t) sets the type before the json_value allocates, so a std::bad_alloc left a member typed object/array with a null pointer, and the destructor crashed instead of propagating the exception. Construct the value first and move it over the member (the move constructor is noexcept). Signed-off-by: Niels Lohmann * Fix CI for the parse() bad_alloc sweep test (#5985) - sweep_allocator: provide max_size(), construct() and destroy(), which GCC 4.8's std::map calls on the allocator directly. - Declare the non-throwing test lambdas noexcept (-Werror=noexcept on recent GCC). - Give the sax_parse lambda an explicit return type; Clang 3.4 crashes on the deduced one in C++11 mode. - Skip the parser callback sweep with iterator debugging (MSVC debug builds, clang-cl), where containers allocate debug proxies inside noexcept move constructors and terminate instead of throwing, as in the other allocator tests. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01N5NasAgbDGG28MqXjQvLqp Signed-off-by: Claude * Construct a scalar member in place; only a throwing value goes through a temporary Creating the value first and moving it over the member is needed only when creating it can throw (a string, a binary value or a container). A scalar or null is again constructed in place, without the extra move and destroy per member (and, with JSON_DIAGNOSTICS, without the extra set_parents()). Signed-off-by: Niels Lohmann * Spell the nothrow trait without && and fix the construct_member comments The dispatch in set_member tests std::is_nothrow_constructible, not whether the value is a scalar. Say so in the comments, and drop the && from the trait argument (equivalent for a forwarding reference) so that astyle no longer inserts spaces around the template brackets. Signed-off-by: Niels Lohmann * Say "for example" in the construct_member comments The overloads are selected by std::is_nothrow_constructible, not by the kind of value: a custom type that cannot throw takes the first one. Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann Signed-off-by: Claude Co-authored-by: Claude --- include/nlohmann/detail/input/json_sax.hpp | 22 ++- single_include/nlohmann/json.hpp | 22 ++- tests/src/unit-allocator.cpp | 172 ++++++++++++++++++++- 3 files changed, 213 insertions(+), 3 deletions(-) diff --git a/include/nlohmann/detail/input/json_sax.hpp b/include/nlohmann/detail/input/json_sax.hpp index 43af2616a..80850ab92 100644 --- a/include/nlohmann/detail/input/json_sax.hpp +++ b/include/nlohmann/detail/input/json_sax.hpp @@ -1303,7 +1303,7 @@ class json_sax_dom_parser : public json_sax_dom_error_state { // a new member is null and holds nothing: construct the value // over it instead of destroying and swapping - ::new (static_cast(&member)) BasicJsonType(std::forward(v)); + construct_member(member, std::forward(v), std::is_nothrow_constructible {}); } else { @@ -1312,6 +1312,26 @@ class json_sax_dom_parser : public json_sax_dom_error_state } } + /// constructing the value cannot throw (for example a number or a boolean): construct it in place + template + JSON_HEDLEY_ALWAYS_INLINE + static void construct_member(BasicJsonType& member, Value&& v, std::true_type /*nothrow*/) + { + ::new (static_cast(&member)) BasicJsonType(std::forward(v)); + } + + /// constructing the value can throw (for example a container or a string), and a + /// constructor that was interrupted must not have touched the member (#5985): + /// create the value first, then move it over the member (the move constructor is + /// noexcept) + template + JSON_HEDLEY_ALWAYS_INLINE + static void construct_member(BasicJsonType& member, Value&& v, std::false_type /*may throw*/) + { + BasicJsonType value(std::forward(v)); + ::new (static_cast(&member)) BasicJsonType(std::move(value)); + } + /// a custom base class may care about the lifetime of the value template static void set_member(BasicJsonType& member, Value&& v, std::false_type /*custom base*/) diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 1d7df6d99..dee9797b4 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -16937,7 +16937,7 @@ class json_sax_dom_parser : public json_sax_dom_error_state { // a new member is null and holds nothing: construct the value // over it instead of destroying and swapping - ::new (static_cast(&member)) BasicJsonType(std::forward(v)); + construct_member(member, std::forward(v), std::is_nothrow_constructible {}); } else { @@ -16946,6 +16946,26 @@ class json_sax_dom_parser : public json_sax_dom_error_state } } + /// constructing the value cannot throw (for example a number or a boolean): construct it in place + template + JSON_HEDLEY_ALWAYS_INLINE + static void construct_member(BasicJsonType& member, Value&& v, std::true_type /*nothrow*/) + { + ::new (static_cast(&member)) BasicJsonType(std::forward(v)); + } + + /// constructing the value can throw (for example a container or a string), and a + /// constructor that was interrupted must not have touched the member (#5985): + /// create the value first, then move it over the member (the move constructor is + /// noexcept) + template + JSON_HEDLEY_ALWAYS_INLINE + static void construct_member(BasicJsonType& member, Value&& v, std::false_type /*may throw*/) + { + BasicJsonType value(std::forward(v)); + ::new (static_cast(&member)) BasicJsonType(std::move(value)); + } + /// a custom base class may care about the lifetime of the value template static void set_member(BasicJsonType& member, Value&& v, std::false_type /*custom base*/) diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index 02b145aae..e1dee5509 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -13,6 +13,7 @@ using nlohmann::json; #include +#include #include #if JSON_HAS_RANGES #include @@ -791,7 +792,7 @@ TEST_CASE("a failed allocation leaves the value unchanged") #if JSON_HAS_RANGES && !defined(__MINGW32__) const std::vector numbers = {1, 2}; next_construct_fails = true; - CHECK_THROWS_AS(nlohmann::to_json(j, numbers | std::views::filter([](int /*unused*/) + CHECK_THROWS_AS(nlohmann::to_json(j, numbers | std::views::filter([](int /*unused*/) noexcept { return true; })), std::bad_alloc&); @@ -1024,3 +1025,172 @@ TEST_CASE("insert() provides its documented exception guarantees (#5986)") } } #endif + +#if !defined(JSON_NOEXCEPTION) +namespace +{ +// the allocation numbered sweep_fail_at (counted from 1) throws; 0 never fails +std::size_t sweep_allocations = 0; +std::size_t sweep_fail_at = 0; +std::ptrdiff_t sweep_live = 0; + +template +struct sweep_allocator +{ + using value_type = T; + + sweep_allocator() = default; + template sweep_allocator(const sweep_allocator& /*unused*/) noexcept {} + + T* allocate(std::size_t n) + { + if (sweep_fail_at != 0 && ++sweep_allocations == sweep_fail_at) + { + throw std::bad_alloc(); + } + ++sweep_live; + return std::allocator().allocate(n); + } + + void deallocate(T* p, std::size_t n) noexcept + { + --sweep_live; + std::allocator().deallocate(p, n); + } + + // GCC 4.8's std::map calls these on the allocator directly instead of + // going through std::allocator_traits + std::size_t max_size() const noexcept + { + return (std::numeric_limits::max)() / sizeof(T); + } + + template + void construct(U* p, Args&& ... args) noexcept(noexcept(::new (static_cast(p)) U(std::forward(args)...))) + { + ::new (static_cast(p)) U(std::forward(args)...); + } + + template + void destroy(U* p) noexcept + { + static_cast(p); // unused for trivially destructible types + p->~U(); + } + + template + struct rebind + { + using other = sweep_allocator; + }; + + bool operator==(const sweep_allocator& /*unused*/) const noexcept + { + return true; + } + bool operator!=(const sweep_allocator& /*unused*/) const noexcept + { + return false; + } +}; + +// run make() with the allocation numbered k failing, for k = 1, 2, ...: every +// failure must be a std::bad_alloc and must not leave anything behind, and +// the first k without a failure must yield the value made without failures +template +void sweep_allocations_of(const Make& make) +{ + sweep_fail_at = 0; + const BasicJsonType expected = make(); + const std::ptrdiff_t live_before = sweep_live; + + std::size_t failures = 0; + for (std::size_t k = 1; k < 10000; ++k) + { + sweep_allocations = 0; + sweep_fail_at = k; + try + { + const BasicJsonType result = make(); + sweep_fail_at = 0; + CHECK(result == expected); + break; + } + catch (const std::bad_alloc&) + { + sweep_fail_at = 0; + ++failures; + } + CHECK(sweep_live == live_before); + } + sweep_fail_at = 0; + CHECK(failures > 10); + CHECK(sweep_live == live_before); +} +} // namespace + +TEST_CASE("parse() survives a failing allocation at every point (#5985)") +{ + // strings beyond the small string buffer, nested objects and arrays + const std::string text = R"({"a":"a long string that does not fit into the small string buffer1","b":[1,2,[3,{"k":"another string that is longer than any small string buffer"}]],"c":{"x":1,"y":{"z":[]},"w":"yet another string that does not fit into a small buffer"},"d":1,"b":[[],{}]})"; + + using sweep_json = nlohmann::json::with_allocator_t; + using sweep_ordered_json = nlohmann::ordered_json::with_allocator_t; + + SECTION("json") + { + sweep_allocations_of([&] + { + return sweep_json::parse(text); + }); + } + + SECTION("ordered_json") + { + sweep_allocations_of([&] + { + return sweep_ordered_json::parse(text); + }); + } + + // With iterator debugging (MSVC STL debug builds, also used by clang-cl), + // containers allocate a debug proxy through the allocator inside their + // noexcept move constructors, so failing that allocation terminates the + // program instead of throwing std::bad_alloc. Nothing to check there. +#if !(defined(_ITERATOR_DEBUG_LEVEL) && _ITERATOR_DEBUG_LEVEL > 0) + SECTION("parser callback") + { + const auto keep_all = [](int /*depth*/, sweep_json::parse_event_t /*event*/, sweep_json& /*parsed*/) noexcept + { + return true; + }; + sweep_allocations_of([&] + { + return sweep_json::parse(text, keep_all); + }); + + const auto drop_d = [](int /*depth*/, sweep_json::parse_event_t event, sweep_json & parsed) + { + return event != sweep_json::parse_event_t::key || parsed != "d"; + }; + sweep_allocations_of([&] + { + return sweep_json::parse(text, drop_d); + }); + } +#endif + + SECTION("sax_parse into an existing value") + { + // a failed parse leaves the target in a state that is safe to destroy + using sax_t = nlohmann::detail::json_sax_dom_parser>; + sweep_allocations_of([&]() -> sweep_json + { + sweep_json value; + sax_t sax(value, false); + static_cast(sweep_json::sax_parse(text, &sax)); + return value; + }); + } +} +#endif