Scarfski/json
1
0
Fork 0
mirror of https://github.com/nlohmann/json.git synced 2026-09-27 12:35:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix stack overflow when copying a deeply nested value (#5387) (#5389)

Browse Source 
* Bound the descent of the copy constructor

basic_json's copy constructor copied objects and arrays by handing the
container to its own copy constructor, which copy-constructs every element
and so reaches this constructor again, once per nesting level. A value
nested deeply enough exhausted the call stack and terminated the process
with a segmentation fault - no exception, nothing the caller could catch.
Parsing such a value works, as the parser is iterative, and so does
destroying one, as #1436 made destruction iterative.

Bound how far the copy descends rather than take the call stack away from
it. The first levels are copied exactly as they were - the containers copy
their own elements, which is by far the fastest way to fill them - and only
once the copy has descended 128 levels is the value below it finished
without the call stack, through an explicit worklist. Copying can therefore
no longer exhaust the stack, however deeply a value is nested, while a value
nested less deeply than the bound - all but a vanishing minority - is copied
by the very same code as before and pays only for one counter.

That counter lives in thread_local storage, as one shared between threads
would be raced. JSON_NO_THREAD_LOCAL switches it off for toolchains without
thread_local; copying then goes through the worklist right away, which
yields the same values but is measurably slower.

The deferred values are completed before the copy they belong to returns, so
a value copied while another copy is going on - by a custom base class, say -
is unaffected by the copy it is nested in.

operator= takes its argument by value, so copy assignment is fixed as well.

Copying is as fast as it was, within measurement noise (medians of 9
interleaved runs, clang -O3): -1.3% for an array of strings, +0.0% for a
flat object, +0.1% for a flat array of numbers, +0.3% for nested arrays,
+0.6% for nested objects and +1.2% for a twitter-like document. Copying a
three-key object costs about ten nanoseconds more, the counter. Deferring
every level instead, rather than only those below the bound, measured
between 3% and 9% slower depending on the shape of the value.

This fixes #5387 for the copy constructor. dump() is still recursive.

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

* Test the copy constructor's iterative path in CI

The copy constructor descends into 128 levels before it finishes a value
without the call stack, so the iterative path is otherwise only reached
by the few tests that nest deeper than that.

JSON_NO_THREAD_LOCAL switches the descent off, which sends every value
down that path. Running the whole test suite that way covers it with
every object type, string type, allocator, and base class the suite
already exercises. The new ci_test_no_thread_local target does that; the
macro had no build coverage at all before.

Copying a nested value also has to carry over what the element-wise copy
constructor would have copied: the parents that JSON_DIAGNOSTICS relies
on, and the positions that JSON_DIAGNOSTIC_POSITIONS reports. Both are
now checked on either side of the descent bound, for objects and arrays.
Neither was tested before, and dropping either one makes the new tests
fail.

Also quantify what JSON_NO_THREAD_LOCAL costs a copy instead of calling
it "measurably slower".

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

* Split the regression tests so that they keep linking

Linking test-regression2 fails with "relocation truncated to fit:
IMAGE_REL_AMD64_REL32 against `.rdata'" once its object grows past what
the MinGW linker copes with, and the copy constructor's helpers push it
over: the object grows by 6.3%, from 4,654,128 to 4,944,920 bytes at -O0,
and develop links at the smaller of the two.

Building the tests optimized shrinks the object enough to link, but the
binaries clang 11.0.1 and clang 18.1.8 then produce crash before doctest
prints its first line - 39 of 102 tests on clang 18 - so the objects have
to become smaller rather than denser.

Moving the test cases that follow "regression tests 2" into a file of
their own brings that object to 4,687,888 bytes, which is 0.7% above the
size that links today rather than 6.3%. Both files still build for C++11,
C++17 and C++20, and run the same 9 test cases and 135 assertions as
before, now spread over two binaries.

New regression tests belong in unit-regression3.cpp from here on, which
is what CONTRIBUTING.md now says.

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

* Do not use thread_local storage with Clang targeting MinGW

Every test that copies a value segfaults there - 42 of 105 on clang
11.0.1, 39 of 102 on clang 18.1.8 - while the same tests pass with GCC
targeting MinGW, with Clang targeting MSVC, and with every other
toolchain the library is tested on. The counter that bounds the copy
constructor's descent is the library's first use of thread_local, so
that job had never exercised it before.

JSON_NO_THREAD_LOCAL already covers toolchains without thread_local
storage, and copying yields the same values with it, only more slowly.
Define it for this one automatically.

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

* Balance the warning suppression the split separated

unit-regression2.cpp opens a DOCTEST_CLANG_SUPPRESS_WARNING_PUSH block at
the top and closed it at the very bottom, which the split moved into
unit-regression3.cpp: one file was left with a push and no pop, the other
with a pop and no push, which clang reports as an error.

Give each file the pair it needs.

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

* Check both shapes without a C-style array

clang-tidy rejects the array the two shapes were iterated over
(cppcoreguidelines-avoid-c-arrays). The array only existed because astyle
reformats a range-for over a braced initializer list into something
unreadable; naming the two cases avoids both.

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

* Split the regression tests far enough to leave room

The first split left unit-regression2.cpp 0.7% below the size develop
links at, which the comparison change in the follow-up immediately used
up: the MinGW linker fails on test-regression2_cpp20 again, naming
copy_shallow and to_partial_ordering among the relocations it cannot fit.

Move the sections from "issue #2067" on, and the helper types they use,
so that the file stops being the one that decides whether the tests can
be linked at all. At -O0 and C++20, unit-regression2.cpp is now 2,964,944
bytes against develop's 4,708,248, and 3,070,568 bytes with the follow-up
applied - roughly a third smaller either way, rather than a fraction of a
percent larger.

The 135 assertions are the same ones as before, now spread over three
test cases in two files.

Also silence the clang-tidy findings the deep-nesting tests draw: the
copies they make are what is being tested, and the reserve() computation
gets its parentheses.

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

* Move the #4804 alias to the file that uses it

The split left the json_4804 alias behind in unit-regression2.cpp while
the test case that uses it went to unit-regression3.cpp, which does not
build for C++17 and C++20 as a result.

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

* Include <span> where the split moved its only use

The #2546 test case guards itself with __has_include(<span>), but the
include itself sat in unit-regression2.cpp's preamble and stayed behind,
so the section compiled without a declaration wherever the guard passed -
which nvhpc reported and libc++ builds do not, as they skip the section
altogether.

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

* Keep the descent bookkeeping in one place

Copying carried a depth count, a depth limit and a guard of its own, and
the comparison in the follow-up added a second set beside them. Neither
operation needs its own: they are never nested inside one another by the
library - copying a value does not compare one, and comparing two values
does not copy them - and where user code nests them anyway, sharing the
count only ends a descent sooner than it had to.

So there is now one nesting_depth(), one nesting_depth_limit() and one
nesting_depth_guard, which the follow-up uses instead of adding its own.
Inverting the test in copy_structured leaves the too-deep case and the
no-thread-local case as the same code.

The guard takes the count rather than looking it up, because the caller
has looked it up already to test it against the limit, and reaching
thread-local storage twice on the path that is taken almost every time is
worth avoiding.

The switch that copies the value of anything that is not an object or an
array was written twice - once in the copy constructor, once in
copy_shallow - so that adding a value_t meant editing both, and missing
one would have been silent. It is copy_leaf_value now, and inlined: both
callers have already sorted the containers out, and folding that test into
the switch is what keeps a value made mostly of numbers copying as fast as
it did.

Copying canada.json, citm_catalog.json and twitter.json is within 0.6% of
what it was before, measured as a paired ratio over 18 interleaved rounds
against a run-to-run spread of 0.3%.

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

* Check that an abandoned copy can still be destroyed

Copying a value without the call stack builds the copy from the top down,
and every value whose own copy has not been made yet stays a null value
until it is. That is what lets a copy be abandoned half-built: the
destructor finds nothing but complete values and null ones.

Nothing tested it. Failing an allocation part-way through a copy of a
deeply nested value does, with the allocator the file already has for
exactly this kind of test.

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

* Name the test's locals so Flawfinder stops matching them

The code scanning job reports CWE-362 - "check when opening files" - for
a test that opens no files: Flawfinder matched a local variable called
open. Rename it and its partner.

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

* Keep the descent guard's bookkeeping self-contained

nesting_depth_limit() and nesting_depth_guard were only used inside
the JSON_NO_THREAD_LOCAL-guarded branch of copy_structured(), but were
defined unconditionally. Move them inside the #ifndef, and have the
guard look up the depth and test it against the limit itself (via
okay()) instead of making the caller do it - the caller no longer
needs to touch nesting_depth() at all. Also shrink the thread-local
counter to std::uint8_t, matching what its own doc comment already
argued.

Addresses gregmarr's review comments on #5389.

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

* Make nesting_depth_guard usable regardless of JSON_NO_THREAD_LOCAL

nesting_depth_limit() and nesting_depth() stay behind #ifndef
JSON_NO_THREAD_LOCAL, since a descent cannot be bounded without a
per-thread count. But the guard itself now always exists, becoming a
no-op that is never okay() under that macro - the same way the bound
is already reached on every call without one. copy_structured() no
longer needs to know which case it is in.

This is what lets #5390 reuse the guard for comparison, which cannot
test JSON_NO_THREAD_LOCAL where the macro-based operators use it: the
guard now carries that distinction itself instead of requiring every
caller to.

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

* Silence VS2015's C4503 for the custom-base-class test

The deep-copy support added for #5387 lengthened the mangled name of
std::allocator_traits<...>::construct for the test's map type past
VS2015's limit, which /WX turns into a build failure even though the
name is only used for (now-truncated) debug info.

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

* Remove dead unused-parameter casts from copy_metadata()

@gregmarr asked whether the static_cast<void> pair in the
JSON_DIAGNOSTIC_POSITIONS-off branch was needed for an empty
json_base_class_t. It isn't: src and dst are already referenced
unconditionally by the base-class copy above, so no -Wunused-parameter
warning fires either way (checked with -Wall -Wextra
-Wunused-parameter, JSON_DIAGNOSTIC_POSITIONS 0 and 1).

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

* Fix CI: build custom array types without a fill constructor, re-amalgamate

copy_array_level() built the destination array with the fill
constructor array_t(count, value), which is not part of the array
container interface the library otherwise assumes (e.g. custom
ArrayTypes that only provide a default and an iterator-pair
constructor, as covered by unit-custom-array-type.cpp). Default-
construct the array and resize() it instead, matching how the rest
of the codebase already grows array_t.

Also re-run the amalgamation, which had fallen out of sync with
include/nlohmann/json.hpp.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Niels Lohmann
2026-09-24 17:01:15 +02:00
committed by GitHub
parent ed513715a8
commit 56b3ee566c
16 changed files with 1175 additions and 112 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
include/nlohmann/detail/macro_scope.hpp
View File

@@ -186,6 +186,15 @@
#define JSON_NO_UNIQUE_ADDRESS
#endif
// Clang targeting MinGW does not survive the thread_local storage the copy
// constructor uses to bound its descent: every test that copies a value
// segfaults with clang 11.0.1 and clang 18.1.8, while the same tests pass with
// GCC targeting MinGW and with every other toolchain the library is tested on.
// Copying works the same way without the counter, only more slowly.
#if !defined(JSON_NO_THREAD_LOCAL) && defined(__clang__) && defined(__MINGW32__)
#define JSON_NO_THREAD_LOCAL 1
#endif
// disable documentation warnings on clang
#if defined(__clang__)
#pragma clang diagnostic push

411
include/nlohmann/json.hpp
View File

@@ -28,14 +28,14 @@
#pragma GCC diagnostic ignored "-Wignored-attributes"
#endif
#include <algorithm> // all_of, find, for_each
#include <algorithm> // all_of, find, for_each, none_of
#include <cstddef> // nullptr_t, ptrdiff_t, size_t
#include <functional> // hash, less
#include <initializer_list> // initializer_list
#ifndef JSON_NO_IO
#include <iosfwd> // istream, ostream
#endif // JSON_NO_IO
#include <iterator> // random_access_iterator_tag
#include <iterator> // make_move_iterator, random_access_iterator_tag
#include <memory> // unique_ptr
#include <string> // string, stoi, to_string
#include <utility> // declval, forward, move, pair, swap
@@ -896,6 +896,352 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
return j;
}
#ifndef JSON_NO_THREAD_LOCAL
/// the number of levels an operation descends into before it finishes the
/// value below it without the call stack
static constexpr std::uint8_t nesting_depth_limit()
{
return 128;
}
/*!
@brief how many levels the operation going on in this thread has descended into
Copying a value and comparing two values share this count. The library never
nests one inside the other - copying a value does not compare one, and
comparing two values does not copy them - and where user code nests them
anyway, sharing the count only ends a descent sooner than it had to, which
costs a little speed and is never wrong.
A byte is enough: the count never exceeds the limit by more than the single
level that notices the limit has been reached.
*/
static std::uint8_t& nesting_depth() noexcept
{
static thread_local std::uint8_t depth = 0; // NOLINT(misc-use-internal-linkage)
return depth;
}
#endif
/*!
@brief counts one level of a bounded descent for as long as it runs, and
reports whether the descent was still within the limit when it began
Looks the count up and tests it against the limit itself, rather than
leaving that to the caller: either way it is reached exactly once, so
there is nothing to be gained by making the caller do it.
Does nothing and is never @ref okay without thread-local storage, where no
descent can be bounded at all: a caller that only descends while this says
it may always ends up finishing without the call stack, exactly as if every
value were nested past the limit.
*/
class nesting_depth_guard
{
public:
nesting_depth_guard() noexcept
#ifdef JSON_NO_THREAD_LOCAL
: m_okay(false)
#else
: m_okay(nesting_depth() < nesting_depth_limit())
#endif
{
#ifndef JSON_NO_THREAD_LOCAL
++nesting_depth();
#endif
}
~nesting_depth_guard()
{
#ifndef JSON_NO_THREAD_LOCAL
--nesting_depth();
#endif
}
nesting_depth_guard(const nesting_depth_guard&) = delete;
nesting_depth_guard& operator=(const nesting_depth_guard&) = delete;
nesting_depth_guard(nesting_depth_guard&&) = delete;
nesting_depth_guard& operator=(nesting_depth_guard&&) = delete;
bool okay() const noexcept
{
return m_okay;
}
private:
bool m_okay;
};
/// an entry of the iterative deep copy's worklist: a structured value and
/// the value that is to become its copy
using copy_worklist_t = std::vector<std::pair<const basic_json*, basic_json*>>;
/// scratch space to build the key skeleton of an object copy in one go
using copy_scratch_t = std::vector<std::pair<typename object_t::key_type, basic_json>>;
/// @brief copy everything of @a src into @a dst but its type and value
static void copy_metadata(const basic_json& src, basic_json& dst)
{
// a custom base class is only required to be copy-constructible and
// move-assignable, so the copy has to go through a temporary
static_cast<json_base_class_t&>(dst) = json_base_class_t(static_cast<const json_base_class_t&>(src));
#if JSON_DIAGNOSTIC_POSITIONS
dst.start_position = src.start_position;
dst.end_position = src.end_position;
#endif
}
/*!
@brief copy the value of @a src into @a dst, which must not be structured
Objects and arrays are left alone: creating those is the one thing the copy
constructor and @ref copy_shallow do differently from one another, and it is
the reason copying a value can descend at all.
*/
/// @note inlined on purpose: both callers have already told an object or an
/// array apart from the rest, and letting the compiler fold that test
/// into this switch is worth a few percent when copying a value made
/// mostly of numbers
JSON_HEDLEY_ALWAYS_INLINE
static void copy_leaf_value(const basic_json& src, basic_json& dst)
{
switch (src.m_data.m_type)
{
case value_t::string:
{
dst.m_data.m_value = *src.m_data.m_value.string;
break;
}
case value_t::binary:
{
dst.m_data.m_value = *src.m_data.m_value.binary;
break;
}
case value_t::boolean:
{
dst.m_data.m_value = src.m_data.m_value.boolean;
break;
}
case value_t::number_integer:
{
dst.m_data.m_value = src.m_data.m_value.number_integer;
break;
}
case value_t::number_unsigned:
{
dst.m_data.m_value = src.m_data.m_value.number_unsigned;
break;
}
case value_t::number_float:
{
dst.m_data.m_value = src.m_data.m_value.number_float;
break;
}
case value_t::object:
case value_t::array:
case value_t::null:
case value_t::discarded:
default:
break;
}
}
/*!
@brief copy everything of @a src into the null value @a dst but the children
Objects and arrays are not copied here; they are appended to @a worklist to
be created later by @ref copy_iteratively. Until that happens, @a dst remains
a null value, so that a partially built copy can be destroyed at any point
without ever violating the class invariants.
*/
static void copy_shallow(const basic_json& src, basic_json& dst, copy_worklist_t& worklist)
{
copy_metadata(src, dst);
if (src.m_data.m_type == value_t::object || src.m_data.m_type == value_t::array)
{
// defer: dst stays a null value until its container exists
worklist.emplace_back(&src, &dst);
return;
}
copy_leaf_value(src, dst);
// only now that the value exists may the type be set: had the creation
// of the value thrown, dst would have been left as a valid null value
dst.m_data.m_type = src.m_data.m_type;
}
/// @brief create the copy of the array @a src in @a dst
/// @note structured elements are appended to @a worklist instead
static void copy_array_level(const basic_json& src, basic_json& dst, copy_worklist_t& worklist)
{
const array_t& src_array = *src.m_data.m_value.array;
// create all elements up front: growing the array afterwards could
// invalidate the pointers that are handed to the worklist; resize()
// rather than the fill constructor, because not every array type
// provides the latter (e.g., ones without a matching allocator-aware
// fill constructor)
dst.m_data.m_value.array = create<array_t>();
dst.m_data.m_value.array->resize(src_array.size());
auto dst_it = dst.m_data.m_value.array->begin();
for (auto src_it = src_array.cbegin(); src_it != src_array.cend(); ++src_it, ++dst_it)
{
copy_shallow(*src_it, *dst_it, worklist);
}
}
/// @brief create the copy of the object @a src in @a dst
/// @note structured values are appended to @a worklist instead
static void copy_object_level(const basic_json& src, basic_json& dst,
copy_worklist_t& worklist, copy_scratch_t& scratch)
{
const object_t& src_object = *src.m_data.m_value.object;
// build the complete key skeleton and hand it to the object's range
// constructor: adding the keys one by one would be quadratic for object
// types that are backed by a vector, such as nlohmann::ordered_map
scratch.clear();
scratch.reserve(src_object.size());
for (const auto& element : src_object)
{
scratch.emplace_back(element.first, basic_json());
}
dst.m_data.m_value.object = create<object_t>(std::make_move_iterator(scratch.begin()),
std::make_move_iterator(scratch.end()));
scratch.clear();
// pair every value of the copy with its counterpart in the original;
// both are enumerated in the same order for every object type with a
// deterministic order, so the lookup is only needed for exotic ones
auto src_it = src_object.cbegin();
for (auto& element : *dst.m_data.m_value.object)
{
if (JSON_HEDLEY_LIKELY(src_it != src_object.cend() && src_it->first == element.first))
{
copy_shallow(src_it->second, element.second, worklist);
++src_it;
}
else
{
const auto found = src_object.find(element.first);
JSON_ASSERT(found != src_object.cend());
copy_shallow(found->second, element.second, worklist);
}
}
}
/*!
@brief deep-copy the object or array @a src into this value without recursing
The values whose copy has not been created yet are kept on an explicit
worklist rather than on the call stack. This is only reached for values
nested deeper than @ref nesting_depth_limit levels, which is why it copies
every container by hand instead of letting the container do it: the fast
ways of doing so would descend into the elements and defeat the purpose.
*/
void copy_iteratively(const basic_json& src)
{
copy_worklist_t worklist;
copy_scratch_t scratch;
const basic_json* src_value = &src;
basic_json* dst_value = this;
for (;;)
{
if (src_value->m_data.m_type == value_t::array)
{
copy_array_level(*src_value, *dst_value, worklist);
}
else
{
copy_object_level(*src_value, *dst_value, worklist, scratch);
}
// the container is complete and will not be modified again
dst_value->set_parents();
if (worklist.empty())
{
break;
}
const auto& next = worklist.back();
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;
}
}
/*!
@brief copy one level of the object or array @a src into this value
The container copies its own elements, which is the fastest way to fill it.
Every element that is structured itself comes back to @ref copy_structured.
*/
void copy_level(const basic_json& src)
{
if (m_data.m_type == value_t::object)
{
m_data.m_value = *src.m_data.m_value.object;
}
else
{
m_data.m_value = *src.m_data.m_value.array;
}
set_parents();
}
/*!
@brief deep-copy the object or array @a src into this value
Copying a container copies its elements, so a value nested deeply enough
used to exhaust the call stack. The descent is bounded here: the first
@ref nesting_depth_limit levels are copied by the containers themselves, just
as they always were, and anything below that is copied without the call
stack by @ref copy_iteratively. Copying a value can therefore no longer
exhaust the stack, however deeply it is nested, just like destroying one
cannot since #1436.
Nothing has to be scanned or built by hand to reach that: a value that is
not nested deeper than the limit - all but a vanishing minority - is copied
exactly as it was before, and this whole detour costs it one counter.
@sa https://github.com/nlohmann/json/issues/5387
*/
void copy_structured(const basic_json& src)
{
const nesting_depth_guard guard;
if (JSON_HEDLEY_LIKELY(guard.okay()))
{
copy_level(src);
return;
}
// Finish this value without descending any further. It is completed
// before this returns, so a copy made by a custom base class - or by
// anything else that runs while a copy is going on - is unaffected by
// the copy it is nested in.
copy_iteratively(src);
}
public:
//////////////////////////
// JSON parser callback //
@@ -1275,60 +1621,15 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
// check of passed value is valid
other.assert_invariant();
switch (m_data.m_type)
if (m_data.m_type == value_t::object || m_data.m_type == value_t::array)
{
case value_t::object:
{
m_data.m_value = *other.m_data.m_value.object;
break;
}
case value_t::array:
{
m_data.m_value = *other.m_data.m_value.array;
break;
}
case value_t::string:
{
m_data.m_value = *other.m_data.m_value.string;
break;
}
case value_t::boolean:
{
m_data.m_value = other.m_data.m_value.boolean;
break;
}
case value_t::number_integer:
{
m_data.m_value = other.m_data.m_value.number_integer;
break;
}
case value_t::number_unsigned:
{
m_data.m_value = other.m_data.m_value.number_unsigned;
break;
}
case value_t::number_float:
{
m_data.m_value = other.m_data.m_value.number_float;
break;
}
case value_t::binary:
{
m_data.m_value = *other.m_data.m_value.binary;
break;
}
case value_t::null:
case value_t::discarded:
default:
break;
// copying the container directly would call this constructor again
// for every element, once per nesting level
copy_structured(other);
}
else
{
copy_leaf_value(other, *this);
}
set_parents();