From 3bfbc6a390ff28858576b5f12227a24d37040795 Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Fri, 9 Oct 2026 16:29:11 +0200 Subject: [PATCH] Do not leave a half-assigned container when set_moved() throws assign() rewrote the kind, length and extent of a slot before set_moved() registered its new element sequence. set_moved() can throw (std::bad_alloc from reserving the bookkeeping vectors) for a slot that is not moved yet, which left a container without the moved flag that showed its old children. The bookkeeping is now reserved first (reserve_moved()), so that nothing after the first write to the slot can throw. block_of() reserves before it marks anything for the same reason. Signed-off-by: Niels Lohmann --- include/nlohmann/detail/view/edit.hpp | 10 +++++++ include/nlohmann/detail/view/edit_storage.hpp | 27 +++++++++++++------ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/include/nlohmann/detail/view/edit.hpp b/include/nlohmann/detail/view/edit.hpp index 6636f96e3..9f72d5049 100644 --- a/include/nlohmann/detail/view/edit.hpp +++ b/include/nlohmann/detail/view/edit.hpp @@ -380,10 +380,20 @@ class editor const node* const r = e.region; const std::uint32_t extent = is_container(*slot) ? slot->next : 1; const bool was_moved = (slot->flags & node_flags::moved) != 0; + // Everything that can throw happens before the slot changes: a slot + // that is a container without the moved flag would show its old + // elements. reserve_moved() makes the set_moved() below, which sets + // the flag, safe; the entry of `regions` exists already (encode() + // added it), so that the assignment at the end does not allocate. + if (!was_moved) + { + reserve_moved(m_doc); + } slot->kind = r->kind; slot->extra = 0; slot->len = r->len; slot->next = extent; + // (set_moved() adds the moved flag to a slot that does not have it yet) slot->flags = was_moved ? static_cast(node_flags::moved | node_flags::is_new) : std::uint8_t{0}; set_moved(m_doc, slot, e.region, 0); edit_state_of(m_doc).regions[e.region] = slot; diff --git a/include/nlohmann/detail/view/edit_storage.hpp b/include/nlohmann/detail/view/edit_storage.hpp index 372bb3049..eb5926c2d 100644 --- a/include/nlohmann/detail/view/edit_storage.hpp +++ b/include/nlohmann/detail/view/edit_storage.hpp @@ -101,16 +101,12 @@ inline std::size_t moved_capacity(const document_data& d, const node* n) noexcep return d.edits->moved_cap[n->off]; } -/// let container n take its elements from `seq` (header node first) -inline void set_moved(document_data& d, node* n, node* seq, std::size_t cap) +/// Make room for one more moved container. This is the part of set_moved() +/// that can throw: a caller that changes a node before it calls set_moved() +/// calls this first, so that a failure leaves the node as it was. +inline void reserve_moved(document_data& d) { document_data::edit_state& e = edit_state_of(d); - if ((n->flags & node_flags::moved) != 0) - { - e.moved[n->off] = seq; - e.moved_cap[n->off] = cap; - return; - } if (e.moved.size() >= 0xFFFFFFFFu) { throw_out_of_range(416, "more than 4294967295 edited arrays and objects are not supported by json_document"); // LCOV_EXCL_LINE @@ -121,6 +117,20 @@ inline void set_moved(document_data& d, node* n, node* seq, std::size_t cap) e.moved.reserve((2 * e.moved.size()) + 16); e.moved_cap.reserve((2 * e.moved.size()) + 16); } +} + +/// let container n take its elements from `seq` (header node first); cannot +/// throw if n is moved already or reserve_moved() was called +inline void set_moved(document_data& d, node* n, node* seq, std::size_t cap) +{ + document_data::edit_state& e = edit_state_of(d); + if ((n->flags & node_flags::moved) != 0) + { + e.moved[n->off] = seq; + e.moved_cap[n->off] = cap; + return; + } + reserve_moved(d); e.moved.push_back(seq); e.moved_cap.push_back(cap); n->off = static_cast(e.moved.size() - 1); @@ -146,6 +156,7 @@ inline node* block_of(document_data& d, node* n, std::size_t extra) set_moved(d, n, nh, cap); return nh; } + reserve_moved(d); // (so that set_moved() below cannot throw) const bool object = n->kind == static_cast(value_t::object); const std::size_t used = 1 + (static_cast(n->len) * (object ? 2 : 1)); const std::size_t cap = used + extra;