Skip to content

Fix stack overflow converting deep values between specializations - #5723

Merged
nlohmann merged 5 commits into
developfrom
claude/convert-specialization-iterative-5650
Sep 30, 2026
Merged

nlohmann merged 5 commits into
developfrom
claude/convert-specialization-iterative-5650

Conversation

@nlohmann

Copy link
Copy Markdown
Owner

Summary

Constructing a basic_json from another specialization (nlohmann::ordered_json o = j;, the reverse direction, or j.get<nlohmann::ordered_json>()) converted every container with its range constructor, which calls the converting constructor for each element. The stack grew with every nesting level, so a value nested some 30,000 levels deep crashed with a stack overflow. Parsing, copying, and dump() handle such a value fine.

The conversion now bounds its descent the way the copy constructor does since #5387. The first 128 levels are converted exactly as before. Below that, the value is finished with an explicit stack.

Changes

  • The converting constructor basic_json(const BasicJsonType&) now handles leaves with convert_leaf(), which holds the leaf cases of the old switch and is shared by both paths, and handles objects and arrays with convert_structured().
  • convert_structured() takes a nesting_depth_guard. Within the bound it calls JSONSerializer<other_object_t/other_array_t>::to_json() as before, so shallow conversions run the same code as before. Past the bound, or always with JSON_NO_THREAD_LOCAL (the same as for copying), it calls convert_iteratively().
  • convert_iteratively() converts post-order with an explicit stack. Each pending container holds the source container and its position. The converted elements go into shared scratch vectors: basic_json for arrays, and std::pair<key_type, basic_json> for objects, with the key converted the way the range constructor converts it. Once a container is complete, convert_level() creates it from move iterators with create<array_t> / create<object_t> and hands it to its parent. I did not pair elements by position, as copy_object_level() does, for two reasons. std::map and ordered_map can enumerate the same members in different orders. Building from a range also keeps the range constructor's handling of keys that become equal on conversion.
  • A container gets its type only after it has been created, so a throwing allocation never leaves an array or object with a null pointer (the trap from Deep copy of a value nested more than 128 levels crashes when an allocation fails #5640). Every built container gets set_parents(). Under JSON_DIAGNOSTIC_POSITIONS, every element gets its source's positions.
  • Side fix: converting a null value lost its positions under JSON_DIAGNOSTIC_POSITIONS, on both paths. The old constructor ran *this = nullptr, which swapped in the positions of the temporary. The value is already null at that point, so the assignment is gone.
  • Updated the comment on nesting_depth(): conversion now uses the same thread-local count as copying and comparing.

Stack and speed (Apple clang, arm64):

  • Stack use: at -O2, converting a 100,000-level array or object uses about 32-38 KB of stack in total. Before, it used about 240-290 bytes per level (240-288 KB at 1,000 levels) and overflowed the 8 MB stack at 30,000-40,000 levels. Under ASan the conversion now needs about 190 KB at 100,000 levels.
  • Shallow values: at 128 levels the stack use is byte-for-byte the same as on develop. A round trip json → ordered_json → json of a wide 20,000-element document takes the same time within noise (7.6 ms before and after).
  • Deep values: converting a 100,000-level value takes about 9 ms, about the same as copying it.

Tests

  • unit-large_json.cpp, new section "issue Converting between basic_json specializations (e.g. json to ordered_json) overflows the stack on deep input #5650": nested arrays, nested objects, and mixed nesting at 100,000 levels, for json → ordered_json, ordered_json → json, and get<ordered_json>(). Each result is checked by comparing its dump() with the source text; all objects have a single member, so both object types list members in the same order. The section also has:
    • all depths from 1 to 300 in both directions, around the bound;
    • a value with every value type (binary with and without subtype, discarded, empty containers, objects whose members the two types list in different orders), placed 200 levels deep so the iterative path converts it. Its result must equal the same value converted on its own (recursive path), by dump() and by ==.
      Against develop's headers this section crashes with an ASan stack overflow in the json → ordered_json conversion.
  • unit-diagnostics.cpp: after a 300-level mixed conversion, the JSON Pointer in a diagnostic still reaches the innermost value (parents set by the iterative path).
  • unit-diagnostic-positions.cpp: start and end positions of every value on the path, and of its sibling, at depths 1, 127, 128, 129, and 300. The innermost value is null. Against develop this fails at depth 1 (the null lost its positions). With the positions copy removed from the iterative path, it fails at level 129.
  • unit-allocator.cpp: a new countdown_allocator fails the 1st, 2nd, 3rd, … construction while a 300-level value is converted to a specialization that uses it, until the conversion succeeds. Every failure must reach the caller as std::bad_alloc. Checked by mutation: setting the type before create<array_t>() makes this test fail on assert_invariant().
  • Ran unit-large_json, unit-allocator, unit-diagnostics, and unit-diagnostic-positions with clang and -fsanitize=address,undefined in these configurations: C++11, C++17, -DJSON_DIAGNOSTICS=1, -DJSON_DIAGNOSTIC_POSITIONS=1, and -DJSON_NO_THREAD_LOCAL. The last one runs every structured conversion through the iterative path. With -DJSON_NO_THREAD_LOCAL, unit-ordered_json, unit-ordered_json2, unit-udt, unit-regression2, unit-comparison, and unit-constructor1 also pass. The four changed test files produce no warnings with clang -Weverything (CI exclusions, C++11 and C++20) or with GCC 16 and the CI flags (including -Weffc++).

Public API

No breaking changes. The signature of the converting constructor is unchanged, and all new members are private. Two behavior changes:

  • Values nested deeper than 128 levels now convert instead of crashing.
  • Under JSON_DIAGNOSTIC_POSITIONS, a converted null keeps its positions.

For levels past the bound, and for all levels with JSON_NO_THREAD_LOCAL, the containers are built directly instead of through JSONSerializer<other_array_t> / JSONSerializer<other_object_t>. Leaves still go through JSONSerializer. This only matters for a custom JSONSerializer that specializes the other specialization's container types. Copying already takes the same approach past the bound.

Fixes #5650


This PR was written by Claude Code on behalf of @nlohmann.

🤖 Generated with Claude Code

Constructing a basic_json from another specialization (json to
ordered_json or back, also via get<ordered_json>()) converted every
container with its range constructor, which calls the converting
constructor for each element. The call stack therefore grew with every
nesting level, and a value nested some 30,000 levels deep overflowed it.

The conversion now bounds its descent the way the copy constructor does
since #5387: the first 128 levels are converted exactly as before, and
below that convert_iteratively() finishes the value with an explicit
stack. It builds each container bottom-up from its converted elements
with the container's range constructor, so member order and keys that
become equal are handled as before, and it gives a value its type only
once its container exists, so an exception leaves nothing behind that
cannot be destroyed. Parents (JSON_DIAGNOSTICS) and positions
(JSON_DIAGNOSTIC_POSITIONS) are set for every value.

Converting a null value no longer resets its positions: the constructor
assigned null to a value that already was null, which swapped in the
positions of the temporary.

Fixes #5650.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann nlohmann added the review needed It would be great if someone could review the proposed changes. label Sep 30, 2026
Comment thread include/nlohmann/json.hpp Outdated
Comment thread include/nlohmann/json.hpp Outdated
Review feedback on #5723 (gregmarr): clarify in comments that the
converting constructor has already copied the positions of val, which
the null case keeps like every other case, and that next must be a
reference into pending so that ++next advances the stored iterator.

Comments only; no code change.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Conflicts:
- tests/src/unit-diagnostics.cpp: kept both new sections (#5650 from the
  PR, #5668 from develop)

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
The comment still named nesting_depth_limit, which #5637 removed on
develop in favor of detail::recursion_depth_limit().

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Comment thread include/nlohmann/json.hpp Outdated
…ull comment

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann nlohmann added 🚀 ready to merge Ready to merge - just waiting for CI to complete. and removed review needed It would be great if someone could review the proposed changes. labels Sep 30, 2026
@nlohmann nlohmann added this to the Release 3.13.0 milestone Sep 30, 2026
@nlohmann
nlohmann merged commit 7d7055e into develop Sep 30, 2026
4 of 161 checks passed
@nlohmann
nlohmann deleted the claude/convert-specialization-iterative-5650 branch September 30, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🚀 ready to merge Ready to merge - just waiting for CI to complete.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Converting between basic_json specializations (e.g. json to ordered_json) overflows the stack on deep input

2 participants