From c82bd8b39a0246d78e9d689b452b49769f147b91 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Sun, 1 Nov 2026 21:08:09 +0100 Subject: [PATCH] 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 --- include/nlohmann/detail/input/json_sax.hpp | 10 +- single_include/nlohmann/json.hpp | 10 +- tests/src/unit-allocator.cpp | 143 +++++++++++++++++++++ 3 files changed, 157 insertions(+), 6 deletions(-) diff --git a/include/nlohmann/detail/input/json_sax.hpp b/include/nlohmann/detail/input/json_sax.hpp index 0ceec61b7..42b6f1eb9 100644 --- a/include/nlohmann/detail/input/json_sax.hpp +++ b/include/nlohmann/detail/input/json_sax.hpp @@ -1301,9 +1301,13 @@ class json_sax_dom_parser : public json_sax_dom_error_state { if (JSON_HEDLEY_LIKELY(member.is_null())) { - // 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)); + // a new member is null and holds nothing: move the value over it + // instead of destroying and swapping. The value is created first, + // because creating a container or a string can throw, and a + // constructor that was interrupted must not have touched the + // member (#5985); the move constructor is noexcept + BasicJsonType value(std::forward(v)); + ::new (static_cast(&member)) BasicJsonType(std::move(value)); } else { diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 1fc214c5f..0c48f765f 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -16946,9 +16946,13 @@ class json_sax_dom_parser : public json_sax_dom_error_state { if (JSON_HEDLEY_LIKELY(member.is_null())) { - // 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)); + // a new member is null and holds nothing: move the value over it + // instead of destroying and swapping. The value is created first, + // because creating a container or a string can throw, and a + // constructor that was interrupted must not have touched the + // member (#5985); the move constructor is noexcept + BasicJsonType value(std::forward(v)); + ::new (static_cast(&member)) BasicJsonType(std::move(value)); } else { diff --git a/tests/src/unit-allocator.cpp b/tests/src/unit-allocator.cpp index 4062870d5..96d13b1d3 100644 --- a/tests/src/unit-allocator.cpp +++ b/tests/src/unit-allocator.cpp @@ -804,3 +804,146 @@ TEST_CASE("a failed allocation leaves the value unchanged") #endif } #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); + } + + 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); + }); + } + + SECTION("parser callback") + { + const auto keep_all = [](int /*depth*/, sweep_json::parse_event_t /*event*/, sweep_json& /*parsed*/) + { + 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); + }); + } + + 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 value; + sax_t sax(value, false); + static_cast(sweep_json::sax_parse(text, &sax)); + return value; + }); + } +} +#endif