Stop binary writers overflowing the stack on deep values by ayush-singh-0601 · Pull Request #5518 · nlohmann/json

ayush-singh-0601 · 2026-09-10T16:08:52Z

to_cbor, to_msgpack, and to_ubjson recurse once per nesting level. Parsing is iterative, so a value this library will happily build can still SIGSEGV when you serialize it.

This keeps the current recursive write for the first 128 levels (normal documents) and finishes anything deeper on a heap stack. Encoding is the same.

BSON is not in this PR. Its writer also walks the tree a second time to compute document lengths, so that belongs with the size-pass work.

The changes are described in detail, both the what and why.
If applicable, an existing issue is referenced.
The Code coverage remained at 100%. A test case for every new line of code.
If applicable, the documentation is updated.
The source code is amalgamated by running make amalgamate.

Closes #5392

Issue link added by @nlohmann; written by Claude Code.

nlohmann

Please update from the develop branch.

nlohmann · 2026-09-22T19:45:26Z

Please rebase to the latest develop branch as I had to fix some tests.

to_cbor, to_msgpack, and to_ubjson recurse once per nesting level. The parser is iterative, so a value the library accepts can crash on the way back out. Keep the existing recursive path for the first 128 levels and finish anything deeper on a heap stack. Output is unchanged. BSON is left alone because its extra size walk is a separate change. Rebased onto the value-type output sink. The heap frames now initialize every member, which is what -Weffc++ was rejecting. See nlohmann#5392. Signed-off-by: ayush-singh-0601 <singhayush062006@gmail.com>

ayush-singh-0601 · 2026-09-23T10:52:33Z

Rebased onto the latest develop.

The output-sink rewrite landed while this was open, so the branch no longer applied. The gcc job was also failing -Weffc++ because the heap frames never initialized their iterators.

That is fixed. The depth limit sits on the new writer. Anything under 128 levels still uses the same recursive write, and anything deeper finishes on a heap stack. The bytes match what the recursive writer produced. I checked CBOR, MessagePack, UBJSON, and BJData against current develop, including values that cross the limit, and a 100000-deep array serializes instead of blowing the stack. BSON is still left out of this change.

Ready for another look.

nlohmann

Thanks for picking this up and for keeping the encoding identical. It follows the same bounded-descent pattern dump() uses, which makes it easy to review. My request from 2026-09-12 to update from develop is done: the branch is on top of #5286.

I verified it locally before reviewing:

Byte-identical output. I compared against develop on 1,420 random values, including many built to nest 126, 127, 128, 129, 130, 200 and 300 levels deep so they cross the switch-over, with siblings on both sides of it. That covers json and ordered_json, plus JData ndarray objects. Every writer and mode matched: CBOR and MessagePack (vector-returning and adapter overloads), UBJSON and BJData draft 2/3 in all four use_size/use_type combinations. That's 28,400 outputs in C++11 and C++20.
Deep values. All four writers now handle 1,000,000 levels of nested arrays and of nested objects on a 1 MiB stack, where develop crashes at 100,000. Reading the output back gives the same depth and leaf.
unit-large_json.cpp passes in C++11 and C++17.
Speed. On an ordinary 3,000-object document it is consistently 1-4% slower (median of 41 runs, repeated three times), from the extra depth argument. That's fine with me; I'm only noting it.

One thing blocks it: every clang job fails with -Wtautological-type-limit-compare at binary_writer.hpp:939 and :970. The new write_cbor_array_prefix/write_cbor_object_prefix take a plain std::size_t, so N <= numeric_limits<uint64_t>::max() is always true on a 64-bit target. The code these replace declared const auto N = j.m_data.m_value.array->size();, a type that depends on BasicJsonType, and clang doesn't diagnose tautological comparisons on dependent types. Taking the containers' own size_type restores that; see the two suggestions. I checked it locally: with them, the -Weverything set from cmake/clang_flags.cmake compiles to_cbor cleanly again.

BSON is not part of this PR, and that's fine: I'll cover it in a separate PR, since its writer also needs the recursive document-size pass reworked (#5308).

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

nlohmann · 2026-09-23T19:48:18Z

+        return 128;
+    }
+
+    void write_cbor_array_prefix(const std::size_t N)


std::size_t makes the uint64_t comparison below (line 939) tautological on 64-bit targets, which fails every clang job. The container's own size_type is dependent on BasicJsonType, so clang doesn't diagnose it (that is why the replaced const auto N = ... code was fine):

Suggested change

void write_cbor_array_prefix(const std::size_t N)

void write_cbor_array_prefix(const typename BasicJsonType::array_t::size_type N)

nlohmann · 2026-09-23T19:48:18Z

+        // LCOV_EXCL_STOP
+    }
+
+    void write_cbor_object_prefix(const std::size_t N)


Same here, for line 970:

Suggested change

void write_cbor_object_prefix(const std::size_t N)

void write_cbor_object_prefix(const typename BasicJsonType::object_t::size_type N)

write_cbor_array_prefix and write_cbor_object_prefix took std::size_t, so the uint64_t range check is always true on 64-bit targets and clang rejects the build. The container size_type depends on BasicJsonType, which is what the inlined writer used. Signed-off-by: ayush-singh-0601 <singhayush062006@gmail.com>

gregmarr

I wrote these comments yesterday but didn't submit the review. Hopefully they're not out of date.

gregmarr · 2026-09-23T14:20:57Z

    /*!
-    @param[in] j  JSON value to serialize
+    @param[in] j      JSON value to serialize
+    @param[in] depth  nesting level of @a j, counted from the value passed to @ref write_cbor


Does this @ref need clarification, since this function IS write_cbor? Is this referring to basic_json::write_cbor? Same comment for other functions where depth was added.

gregmarr · 2026-09-23T14:23:11Z

  private:
+    /// the number of levels the recursive writers descend into before handing
+    /// over to the iterative writers below
+    static constexpr std::size_t binary_write_depth_limit()


Should this reuse the existing recursion limit rather than adding a new one?

gregmarr · 2026-09-23T14:24:44Z

+            {}
+
+            const BasicJsonType* value;
+            bool started;


Should this be bool started = false; instead of manually setting it in the constructor?

gregmarr · 2026-09-23T14:27:52Z

+            {}
+
+            const BasicJsonType* value;
+            bool started;


nlohmann · 2026-09-27T12:12:21Z

@ayush-singh-0601 Arw you willing to continue working on this?

ayush-singh-0601 · 2026-09-27T12:42:25Z

@nlohmann yes

nlohmann

Thanks for staying on this! Here is a second pass. @gregmarr's comments (the @refs, reusing detail::recursion_depth_limit(), and default member initializers) still apply, so I haven't repeated them below.

Rebase first. The branch conflicts with develop again, and several merges touched the same code:

#5583 added write_cbor_head(major_type, argument), which makes the two CBOR prefix helpers here redundant. It also makes my size_type suggestions from last time moot.
#5584 routes every MessagePack length through to_msgpack_length(size, j), which throws out_of_range.412 above UINT32_MAX. The iterative path has to throw the same way.
#5555 replaced the per-container bjdx vector with is_bjdata_excluded_type_marker().
#5553 has merged, so BSON no longer recurses. You can drop the BSON paragraph from the description, or say it's covered by #5553.

CI. Only ci_clang_tidy really failed. It's the use-default-member-init error Greg pointed out, but it also fires for the four bools in the UBJSON frame (see inline). The other red jobs were cancelled by fail-fast.

The inline comments are mostly about simplifying the iterative writers. The main idea: push only containers, and write their headers when you push them. write_bson_document does the same on develop, and it removes most of the frame state. The tests can also be stronger now that the readers and == no longer recurse.

Please also write Closes #5392 in the description, so the issue closes on merge.

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

nlohmann · 2026-09-27T12:47:27Z

+        return 128;
+    }
+
+    void write_cbor_array_prefix(const typename BasicJsonType::array_t::size_type N)


After rebasing, write_cbor_head(0x80, size) and write_cbor_head(0xA0, size) (from #5583) do exactly what these two helpers do. Please delete both helpers and call write_cbor_head from the recursive and the iterative writer. That also makes my earlier size_type suggestions unnecessary.

nlohmann · 2026-09-27T12:47:27Z

+        }
+    }
+
+    void write_msgpack_array_prefix(const std::size_t N)


With #5584, develop computes MessagePack lengths as const auto N = to_msgpack_length(size, j);, which throws out_of_range.412 above UINT32_MAX. These helpers should do the same, so that both paths share one header writer and a value past the bound throws just like one above it. They will need j for the diagnostics.

(For now, this if/else if chain has no final else, so a longer container would get no header at all. develop had the same bug until #5584.)

nlohmann · 2026-09-27T12:47:27Z

+                const BasicJsonType& j = *stack.back().value;
+                if (!j.is_array() && !j.is_object())
+                {
+                    write_cbor(j);


A different frame design might resolve Greg's started comment: push only containers, and write each container's header when you push it. When the loop takes the next child, it writes a scalar right away (write_cbor(*child)) or writes the child's header and pushes a frame for it. The started flag and the scalar branch at the top of the loop both go away. Leaves no longer cost a push and a pop each, and a frame only needs its value and iterators. bson_frame on develop works this way. It also sets its iterator in the constructor, so the frame never needs default-constructed iterators.

nlohmann · 2026-09-27T12:47:27Z

+
+    @sa @ref write_cbor_iterative
+    */
+    void write_msgpack_iterative(const BasicJsonType& root)


This is the CBOR loop again: apart from the comments, the two loops differ only in the names write_cbor* and write_msgpack*. I'm fine with keeping two copies, since they are short. But if you restructure the frames as suggested above, consider one walker, parameterized by the scalar, header and key writers, for CBOR and MessagePack. Your call.

nlohmann · 2026-09-27T12:47:27Z

+            bool add_prefix;
+            bool prefix_required;
+            bool started;
+            bool write_closer;


clang-tidy also flags prefix_required, started, write_closer and is_object here (lines 1301-1304). Greg's "as above" only covered line 1122.

Some of these fields don't need to be stored at all:

write_closer is always !use_count, and that is fixed for the whole call.

is_object is value->is_object().

add_prefix is only read before the header is written. If headers are written at push time (see the CBOR comment), the parent's prefix_required can be passed straight to write_ubjson_start_* for the child, and the frame no longer needs to store it.

That leaves value, prefix_required, and the iterator. write_ubjson_start_* could then return prefix_required instead of taking it as an out parameter.

nlohmann · 2026-09-27T12:47:27Z

+                    continue;
+                }
+
+                if (j.is_object() && use_bjdata && j.m_data.m_value.object->size() == 3 &&


This condition copies the ndarray check in write_ubjson's object case. If one copy changes and the other doesn't, the two paths will write different bytes for a value at depth 127 and at depth 129, and the existing tests wouldn't notice. Please move the check into a small helper, e.g. static bool is_bjdata_ndarray(const BasicJsonType&), and call it from both places.

nlohmann · 2026-09-27T12:47:27Z

+
+TEST_CASE("issue #5392 - binary writers on deeply nested values")
+{
+    // 200 is past the point where the writers stop recursing, and still


This comment is out of date. Since the reader series (#5502-#5508), from_* no longer recurses, and since #5390, operator== doesn't either. The limit of 200 can stay for speed, but please update the comment.

nlohmann · 2026-09-27T12:47:27Z

+
+        std::vector<std::uint8_t> packed;
+        CHECK_NOTHROW(packed = json::to_cbor(j));
+        CHECK(packed.size() > depth);


packed.size() > depth shows that nothing crashed, not that the output is right. Now that the readers and == handle any depth, this can be CHECK(json::from_cbor(packed) == j); (and likewise for the other formats).

A round trip still can't catch the two paths choosing different but valid encodings, and byte-identical output is what this PR promises. Nested one-element arrays make that cheap to check exactly:

// every level adds one 0x81 (CBOR) / 0x91 (MessagePack) header std::vector<std::uint8_t> expected(depth, 0x81); expected.push_back(0x00); CHECK(json::to_cbor(j) == expected);

For UBJSON the expected output is depth × '[', then the leaf, then depth × ']'. Checking a depth from the 120-140 loop this way covers the switch-over byte for byte.

nlohmann · 2026-09-27T15:43:41Z

OSS-Fuzz found another instance that this PR fixes: issue 566583014 (parse_cbor_fuzzer, testcase 5559710531780608, stack overflow in binary_writer::write_cbor). The input is 25,766 bytes of nested CBOR headers, and it still crashes on develop at f422b75. ClusterFuzz groups it with testcases 6388496731013120 (CBOR writer) and 4818268117073920 (MessagePack writer). All three should verify once this lands.

A regression test doesn't need the fuzzer bytes. This reproduces it:

std::vector<std::uint8_t> v(200000, 0x81); // 200k nested one-element arrays
v.push_back(0xf6);                         // null
const auto j = json::from_cbor(v);
CHECK(json::to_cbor(j) == v);              // no crash, byte-identical

(Comment written by Claude Code on behalf of @nlohmann.)

write_bjdata_ndarray() built a 12-entry std::map<string_t, CharType> on every call just to translate the _ArrayType_ name to a dtype marker (the only reason binary_writer.hpp included <map>), then mapped dtype to C++ type twice more: once as a switch for the range-check pass and once as a separate if/else chain for the write pass, with nothing checking that the two agreed. The caller also ran three at() lookups, and the callee called value.at(key) about ten more times for the same three members. Replace the map with bjdata_ndarray_type_marker(), a plain string comparison chain (a C++11 constexpr function cannot contain a switch, so this mirrors binary_reader's own static table style). Replace the switch/if-chain pair with one write_bjdata_ndarray_elements() that switches on dtype once and calls a per-type helper - write_bjdata_ndarray_element<T>() for the eight integer dtypes and write_bjdata_ndarray_float_element() for 'd' - with a dry_run flag selecting the range check or the actual write, so the two passes can no longer disagree on the type. _ArrayType_, _ArraySize_ and _ArrayData_ are now looked up once into references, and the four header marker bytes ('[', '$', '#') are written through to_char_type() like the rest of the UBJSON/BJData writer. The 'd' (single-precision) rule is left exactly as before, since #5707 is expected to change it separately. Verified byte-for-byte identical output before/after for every dtype (including the Draft 2/Draft 3 'byte' fallback and the use_count/ use_type combinations) via a standalone probe, plus round-tripping through from_bjdata(). Overlaps #5707, which is expected to touch the 'd' dtype case, and #5518, which is expected to move the write_bjdata_ndarray() call site. #5710 item 4 Signed-off-by: Niels Lohmann <mail@nlohmann.me>

@pre

* Fix stale and missing comments in binary_writer The doc block of write_number() ended up above the byte_swap() helpers added in #5286, about 80 lines from the function. It was also a plain comment that Doxygen skips, said "write a number to output input", and left BON8 out of the big-endian formats. Move it back onto write_number() as a /*! block and fix the text. write_bson() documented "@pre j.type() == value_t::object", but it throws type_error.317 for every other type, and to_bson() relies on that. Document the exception instead. Explain why the CBOR binary subtype is always written with a 0xD8..0xDB head and never in the one-byte tag form: binary_reader with cbor_tag_handler_t::store only keeps those heads as a subtype, so switching to write_cbor_head() would break round trips for subtypes 0..23. Also fix the grammar of the to_char_type comment. Comments only; no change in behavior, API or ABI. Part of #5710 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Merge the duplicated UBJSON/BJData integer marker ladders write_number_with_ubjson_prefix() (unsigned and signed overloads) and ubjson_prefix() (number_integer and number_unsigned cases) each picked the UBJSON/BJData integer marker (i, U, I, u, l, m, L, M, H) with their own independent if/else ladder, and the values beyond 64 bits were handled by a second, tag-dispatched pair of ladders. An optimized container announces the marker of its first element via ubjson_prefix() and then writes every element through write_number_with_ubjson_prefix(), so the two had to be kept in lockstep by hand across four call sites. Replace all of that with one ubjson_integer_prefix() built on value_in_range_of<T>, and one write_ubjson_integer_payload() that writes the value (or, for 'H', the decimal digits) for a given marker. write_number_with_ubjson_prefix() and ubjson_prefix() keep their signatures and now just call these two helpers. Behavior, the public API and the ABI are unchanged. Verified with a new regression test covering scalars and $-optimized arrays/objects at every int8/uint8/int16/uint16/int32/uint32/int64/uint64 boundary for to_ubjson/to_bjdata (both use_size/use_type settings), and by diffing to_ubjson/to_bjdata output before and after over the json_test_data corpus (bit-identical). Part of #5710 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Remove dead get_char parameters in binary_reader The non-recursive rewrite of the binary readers (#5505, #5506, #5507) left parse_cbor_internal()'s and parse_ubjson_internal()'s get_char parameters dead: parse_cbor_internal() has one caller and it always passes true, and parse_ubjson_internal() has one caller and it always uses the true default. Both parameters, and the @PARAM docs describing the "reuse the last character" mode they used to select, no longer correspond to anything. Drop both parameters, initialise fetch/prefix unconditionally, and update the two call sites in sax_parse(). parse_cbor_value()'s and get_ubjson_string()'s own get_char parameters are unrelated and are left alone; both still have a false caller. Also delete a stray `@return whether a valid MessagePack value was passed to the SAX parser` doxygen block that sits directly above parse_msgpack_value()'s real doc comment, a leftover of the same rewrite. Behavior, the public API and the ABI are unchanged; these are private members of detail::binary_reader. Verified by compiling with -Wunused-parameter and running unit-cbor, unit-ubjson, unit-bjdata and unit-msgpack (offline, against the stubbed test_data.hpp). Part of #5711 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Share the IEEE half-precision decoder between CBOR and BJData binary_reader had two ~45-line copies of the IEEE 754 half-precision decoder: CBOR's case 0xF9 and BJData's case 'h'. Once formatting is normalised, the two blocks were identical except for the byte order used to assemble the 16-bit half (CBOR is big endian, BJData is little endian). Any future change to half-float decoding had to be made and kept in sync in both places. Add one get_half_float(format, little_endian) helper that does the two get()/unexpect_eof() reads, assembles the half in the requested byte order, decodes it per RFC 8949 Appendix D, and calls sax->number_float. Both cases now just call it with their byte order; the BJData case keeps its bjdata-only guard. Behavior, the public API and the ABI are unchanged. Verified with a scratch probe comparing the old and new decoders bit-for-bit (NaN by isnan()) over all 65536 wire byte pairs, in both formats, and by running unit-cbor and unit-bjdata (offline, against the stubbed test_data.hpp). Part of #5711 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Deduplicate the MessagePack unsigned-integer writer ladder The number_integer (non-negative branch) and number_unsigned cases in write_msgpack() each held their own copy of the fixint/uint8/16/32/64 ladder, kept in lockstep only by a comment ("we used the code from the value_t::number_unsigned case here"). Both copies mixed union members: the signed copy compared number_unsigned but wrote number_integer, and vice versa. Extract write_msgpack_unsigned(std::uint64_t), mirroring how write_cbor_head() already avoids the same duplication for CBOR, and call it from both cases. Each case now reads only its own active union member. Output bytes are unchanged for the default 64-bit number types. #5710 item 3 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Unify float marker selection and fix the long double compile error Four formats picked between a float32 and float64 marker through four different helper styles: dummy-argument overloads for CBOR and MessagePack, an std::is_same template for BON8, and a runtime if-chain on input_format_t for write_compact_float(). With number_float_t set to long double, to_cbor, to_msgpack and to_ubjson failed inside the library with "call to 'get_cbor_float_prefix' is ambiguous", while to_bson kept working because write_bson_double() takes a plain double. Change write_compact_float() to take the two marker bytes directly (each of its three callers already knows them at compile time) instead of an input_format_t it only forwarded, and delete the now-unused get_cbor_float_prefix(), get_msgpack_float_prefix(), get_bon8_float_prefix() and get_compact_float_prefix() helpers. Turn the two get_ubjson_float_prefix() overloads into one template. Both write_compact_float() and get_ubjson_float_prefix() now report an unsupported number_float_t with a static_assert naming the requirement, rather than an ambiguous-overload error; the assert lives in the function body, not the class scope, so to_bson with long double is unaffected. Verified with a probe basic_json<..., long double>: to_bson still compiles and round-trips, while to_cbor/to_msgpack/to_ubjson now fail to compile with the new static_assert message. This changes the text of an existing compile error for users with an unsupported number_float_t (documented as a public-API-visible change in #5710). #5710 item 1 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Deduplicate the BJData ndarray writer's dtype dispatch and drop <map> write_bjdata_ndarray() built a 12-entry std::map<string_t, CharType> on every call just to translate the _ArrayType_ name to a dtype marker (the only reason binary_writer.hpp included <map>), then mapped dtype to C++ type twice more: once as a switch for the range-check pass and once as a separate if/else chain for the write pass, with nothing checking that the two agreed. The caller also ran three at() lookups, and the callee called value.at(key) about ten more times for the same three members. Replace the map with bjdata_ndarray_type_marker(), a plain string comparison chain (a C++11 constexpr function cannot contain a switch, so this mirrors binary_reader's own static table style). Replace the switch/if-chain pair with one write_bjdata_ndarray_elements() that switches on dtype once and calls a per-type helper - write_bjdata_ndarray_element<T>() for the eight integer dtypes and write_bjdata_ndarray_float_element() for 'd' - with a dry_run flag selecting the range check or the actual write, so the two passes can no longer disagree on the type. _ArrayType_, _ArraySize_ and _ArrayData_ are now looked up once into references, and the four header marker bytes ('[', '$', '#') are written through to_char_type() like the rest of the UBJSON/BJData writer. The 'd' (single-precision) rule is left exactly as before, since #5707 is expected to change it separately. Verified byte-for-byte identical output before/after for every dtype (including the Draft 2/Draft 3 'byte' fallback and the use_count/ use_type combinations) via a standalone probe, plus round-tripping through from_bjdata(). Overlaps #5707, which is expected to touch the 'd' dtype case, and #5518, which is expected to move the write_bjdata_ndarray() call site. #5710 item 4 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Assert that write_bson_document() consumes every calc_bson_sizes() entry calc_bson_sizes() and write_bson_document() are a hand-synchronized pair of passes over the same object/array tree, introduced by #5553: the size pass appends to nested_sizes in visiting order, and the write pass consumes the table by position with nested_sizes[next_size++]. Nothing checked that the write pass consumed the whole table. If a future change touched only one of the two passes - for example to skip or reject an entry - every later size prefix in the document would be silently wrong. Add JSON_ASSERT(next_size == nested_sizes.size()) where write_bson_document() returns, so such a future drift between the two passes is caught immediately (JSON_ASSERT expands to nothing in release builds using assert(), and the fuzzers/tests already build with it enabled). The two passes agree today, so this changes nothing observable; it only guards against the risk described in #5710 item 5. Extracting a shared stepper for the two passes (the second half of the proposed change) is left for a follow-up: it only saves ~30 lines and the issue asks for it only if the result reads clearly, which needs more room to get right than a mechanical cleanup pass allows. #5710 item 5 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Make the BJData lookup tables static functions instead of members binary_reader held bjd_optimized_type_markers and bjd_types_map as non-static const members (12 string_t objects for the type-name table), built and destroyed on every from_cbor/from_msgpack/from_bson/ from_ubjson/from_bon8/from_bjdata call even though only from_bjdata ever reads them. They also needed the #define/decltype/#undef workaround from #3637 and two NOLINTNEXTLINE suppressions, and binary_writer already carries the same two lists in another form (is_bjdata_excluded_type_marker() and a local std::map in write_bjdata_ndarray(), the latter removed by the item-4 commit), so the excluded-marker lists could drift apart. Replace bjd_optimized_type_markers with static constexpr is_bjd_excluded_optimized_type(char_int_type), using the same ||-chain as binary_writer's is_bjdata_excluded_type_marker(). Replace bjd_types_map with a non-constexpr static bjd_type_name(char_int_type) switch returning nullptr for an unknown marker (a C++11 constexpr function cannot contain a switch). Delete both JSON_BINARY_READER_MAKE_* macros, the bjd_type pair alias, the NOLINTNEXTLINE suppressions, detail::make_array() (no longer used anywhere), and the now-unused <algorithm> and <array> includes. Update the two call sites (the ND-array excluded-type check and the _ArrayType_ lookup) accordingly, and replace unit-bjdata.cpp's "LUT arrays are sorted" section, which only checked the two tables' internal ordering, with a check of all 12 type names and all 8 excluded markers against both new functions. #5711 item 1 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Read CBOR's 1/2/4/8-byte argument through one helper parse_cbor_internal() hand-wrote the same "read a 1/2/4/8-byte big-endian unsigned integer" ladder four times over: - twice for tag numbers 0xD8-0xDB, once in the tag_handler::ignore branch and once, nearly identically, in the ::store branch (~90 lines to read one integer); - twice more for container lengths, once for array heads 0x98-0x9B and once for map heads 0xB8-0xBB, where the 1/2-byte forms called enter_array()/enter_object() directly and the 4/8-byte forms additionally went through get_cbor_container_size(). Add get_cbor_argument(std::uint64_t&), reading the width selected by current & 0x1F via the same get_number() calls as before (so EOF is reported exactly as before), and route all four sites through it: - 0xD8-0xDB now read the argument once per branch instead of switching on `current` a second time; behavior split cleanly from embedded tags 0xC0-0xD7 (tag value in the head, no argument to read), which is now its own case block that no longer has to fall into the ::store switch's "default" case to reach the same tag_pending = true; return true; outcome. - 0x98-0x9B and 0xB8-0xBB collapse into one case block each, always going through get_cbor_container_size() (harmless for 1/2-byte lengths, which already always fit). Verified byte-for-byte identical behavior before/after with a standalone probe covering embedded and multi-byte tags under all three tag_handler_t settings, a tag over a byte string (subtype path), truncated tag/length arguments of every width, and array/map lengths of every width, including the out_of_range.408 "excessive size" case: same exceptions, same messages, same chars_read, same successful results. Left the string/byte-string length ladders in get_cbor_string()/ get_cbor_binary() untouched, as noted in #5711 item 2, since #5325 is expected to touch them separately. Overlaps #5601 (adds a branch right above the embedded-tag case) and #5607 (touches the integer cases 0x18-0x1B, which share this ladder's shape in separate hunks). #5711 item 2 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Add leave_container() to match enter_container() Every container is opened through enter_container(), whose docs promise that a check placed there runs before every start event. The close side had no equivalent: the same "container_stack.pop_back(); dispatch to end_object() or end_array()" sequence was written out separately in BSON, CBOR, MessagePack, UBJSON/BJData and BON8, each copying the pattern of keeping an is_object flag around the pop_back() that would otherwise invalidate a reference to it. A check needed on close would have had to be added in five places, and a sixth copy could go unnoticed. Add leave_container() next to enter_container(), doing the same pop-then-dispatch, and replace the five sites with it. Each site keeps its own surrounding logic (BSON's check_bson_document_size() call before popping, MessagePack's is_object copy used again below, UBJSON/BJData's remaining-container handling after popping, BON8's top used again below); only the repeated pop/dispatch line pair is now shared. Verified all six binary-format unit suites and unit-regression2's deep-nesting tests (dependent count/reuse count and the bjdata ndarray depth cases) still pass, compiled with -Wall -Wextra and ASan/UBSan. Overlaps #5601, which is expected to add a sixth close site in its own skip loop; that site can route through leave_container() too once it lands. #5711 item 4 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Stop passing the input format to sax_parse() when the reader already has it binary_reader's constructor stores the format in the input_format member, and sax_parse(format, sax_, strict, tag_handler) took the same value again purely to dispatch on it. Every in-tree caller passed the same value both times (all 16 from_cbor/from_msgpack/from_ubjson/ from_bjdata/from_bon8/from_bson call sites in json.hpp, and the three public basic_json::sax_parse() overloads), so nothing was broken today, but a caller of the detail class directly (only reachable via JSON_PRIVATE_UNLESS_TESTED, as unit-bjdata.cpp already does) could pass a mismatched pair - say bjdata to the constructor and ubjson to sax_parse - and dispatch on one format while applying the other format's rules; the default-constructed input_format_t::json reader would additionally hit JSON_ASSERT(false) in exception_message() on its first error. Add sax_parse(json_sax_t*, bool, cbor_tag_handler_t) forwarding to the existing overload with the stored input_format, and switch every caller to it: the 16 from_*() sites (keeping their `// cppcheck-suppress[accessMoved]` comments) and the three basic_json::sax_parse() overloads, all of which already had the format available from their own `format` parameter. The four-argument overload is kept for anyone still calling it, now with JSON_ASSERT(format == input_format) so a mismatch fails immediately in a debug build (assert-enabled binaries, including the fuzzers and test suite) instead of misbehaving; verified with a probe that constructs a reader for one format and calls the explicit overload with another, which aborts on that assertion as expected. Removing or asserting against the constructor's input_format_t::json default, which would affect direct detail users, is left as a separate decision per #5711 item 5. Overlaps #5601, which is expected to add an AllowRecovery template parameter to sax_parse() and touch these same call sites in json.hpp. #5711 item 5 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Deduplicate UBJSON/BJData signed-count handling, drop dead ndarray checks get_ubjson_size_value()'s 'i'/'I'/'l'/'L' cases each read a differently sized signed integer and then repeated the same "reject negative with error 113" check; only 'L' additionally checked value_in_range_of for the out_of_range.408 case. Any change to that error path had to be made four times. Add get_ubjson_signed_count<SignedType>(std::size_t&), doing the read, the negative check and the range check once, and route all four markers through it. The range check is a no-op for 'i'/'I'/'l' (their values always fit std::size_t) and only live for 'L' on a 32-bit std::size_t target, matching today's behavior exactly. In the ndarray dimension-product loop, the preceding loop already returns early on any zero dimension and result starts at 1, so `i > 0` in the pre-multiplication overflow check was always true, and `result == 0` in the post-multiplication check could not be reached either: two positive factors whose product does not overflow (as the pre-check already guarantees) cannot be zero. Drop the dead `i > 0 &&` and narrow the post-check to `result == npos`, the one case the pre-check cannot rule out (an exact, non-overflowing match with the sentinel reserved for unknown-size containers), with a comment explaining why. Verified byte-for-byte identical behavior before/after with a standalone probe covering negative counts for every marker, a matching positive count, and ndarray inputs, plus the full unit-ubjson and unit-bjdata suites (same assertion counts as before this change). Overlaps #5601 (rewrites the four parse_error calls and the overflow checks touched here) and #5607/#5707 (touch neighboring lines in the same functions). #5711 item 6 Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Drop redundant format parameter and dummy float argument (review) binary_reader::sax_parse(format, ...) only ever had to equal the format given to the constructor, which it asserted. With every caller already on the format-less overload, remove the four-argument overload and dispatch on the stored input_format directly. binary_reader is a detail class, so this is not a public API change. get_ubjson_float_prefix() took a value only to deduce its type; make the type an explicit template argument instead. Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: Niels Lohmann <mail@nlohmann.me>

nlohmann · 2026-10-07T07:57:49Z

Thank you, @ayush-singh-0601, for this work! To get the OSS-Fuzz crash (issue 566583014) fixed soon, I've taken it over in #5781. Your commit is cherry-picked there with your authorship kept, and the follow-up commit credits you as co-author. The follow-up rebases onto develop and addresses the open review comments: shared recursion_depth_limit(), write_cbor_head/to_msgpack_length, container-only frames, is_bjdata_ndarray(), and stronger tests. Reviews on #5781 are very welcome. Closing this one in favor of it.

(Comment written by Claude Code on behalf of @nlohmann.)

@ref

… limit Address the open review on the non-recursive CBOR/MessagePack/UBJSON/BJData writers (#5518): - Delete the CBOR array/object prefix helpers; both the recursive and iterative paths call write_cbor_head(), which already existed on develop. - MessagePack: share one write_msgpack_array_prefix()/write_msgpack_object_prefix() helper per container kind between the recursive and iterative paths, both going through to_msgpack_length() so an over-long container throws out_of_range.412 identically either way. - Reuse detail::recursion_depth_limit() instead of a separate constant, the same bound serializer::dump() and write_bson_document() already use. - Redesign the frames after bson_frame/dump_frame: only a container with elements is ever pushed, its header is written at the point it is pushed, and the iterator is set in the frame's constructor instead of a default-then-assign two-step with a since-removed "started" flag. The UBJSON frame keeps only the value pointer, the per-element prefix_required flag, and the iterator; write_closer and is_object are no longer stored, since the former is always !use_count (use_count is constant for the whole document) and the latter follows from value->is_object(). - Factor the BJData ND-array shape check into is_bjdata_ndarray(), used by both the recursive object case and the iterative pushing logic. - Give the frame classes the GCC -Weffc++ treatment already used for diff_frame: a noexcept converting constructor plus the five special members defaulted with no explicit noexcept. - Fix two @ref self-references in write_cbor/write_msgpack/write_ubjson's own doc comments to point at the public to_cbor/to_msgpack/to_ubjson/to_bjdata API instead. - The iterative object-key write for CBOR/MessagePack now runs the same strict-mode check_utf8() against the parent object as diagnostics context that the recursive path already ran, so the two paths raise identical diagnostics across the switch-over. - Rewrite the tests: round trips instead of a bare size check, byte-exact comparisons against the recursive output at depths around the bound, a deep object and a BJData ND-array past the bound, a deep discarded value (type_error.321), and the OSS-Fuzz 566583014 CBOR/MessagePack regression. BSON is unaffected by this change; it already walks its documents iteratively and is covered separately by #5553. Co-authored-by: ayush-singh-0601 <179524189+ayush-singh-0601@users.noreply.github.com> Signed-off-by: Niels Lohmann <mail@nlohmann.me>

@ref

* Stop binary writers overflowing the stack on deep values to_cbor, to_msgpack, and to_ubjson recurse once per nesting level. The parser is iterative, so a value the library accepts can crash on the way back out. Keep the existing recursive path for the first 128 levels and finish anything deeper on a heap stack. Output is unchanged. BSON is left alone because its extra size walk is a separate change. Rebased onto the value-type output sink. The heap frames now initialize every member, which is what -Weffc++ was rejecting. See #5392. Signed-off-by: ayush-singh-0601 <singhayush062006@gmail.com> (cherry picked from commit cf65ac4) Signed-off-by: Niels Lohmann <mail@nlohmann.me> * Redesign the iterative binary writers around a shared recursion depth limit Address the open review on the non-recursive CBOR/MessagePack/UBJSON/BJData writers (#5518): - Delete the CBOR array/object prefix helpers; both the recursive and iterative paths call write_cbor_head(), which already existed on develop. - MessagePack: share one write_msgpack_array_prefix()/write_msgpack_object_prefix() helper per container kind between the recursive and iterative paths, both going through to_msgpack_length() so an over-long container throws out_of_range.412 identically either way. - Reuse detail::recursion_depth_limit() instead of a separate constant, the same bound serializer::dump() and write_bson_document() already use. - Redesign the frames after bson_frame/dump_frame: only a container with elements is ever pushed, its header is written at the point it is pushed, and the iterator is set in the frame's constructor instead of a default-then-assign two-step with a since-removed "started" flag. The UBJSON frame keeps only the value pointer, the per-element prefix_required flag, and the iterator; write_closer and is_object are no longer stored, since the former is always !use_count (use_count is constant for the whole document) and the latter follows from value->is_object(). - Factor the BJData ND-array shape check into is_bjdata_ndarray(), used by both the recursive object case and the iterative pushing logic. - Give the frame classes the GCC -Weffc++ treatment already used for diff_frame: a noexcept converting constructor plus the five special members defaulted with no explicit noexcept. - Fix two @ref self-references in write_cbor/write_msgpack/write_ubjson's own doc comments to point at the public to_cbor/to_msgpack/to_ubjson/to_bjdata API instead. - The iterative object-key write for CBOR/MessagePack now runs the same strict-mode check_utf8() against the parent object as diagnostics context that the recursive path already ran, so the two paths raise identical diagnostics across the switch-over. - Rewrite the tests: round trips instead of a bare size check, byte-exact comparisons against the recursive output at depths around the bound, a deep object and a BJData ND-array past the bound, a deep discarded value (type_error.321), and the OSS-Fuzz 566583014 CBOR/MessagePack regression. BSON is unaffected by this change; it already walks its documents iteratively and is covered separately by #5553. Co-authored-by: ayush-singh-0601 <179524189+ayush-singh-0601@users.noreply.github.com> Signed-off-by: Niels Lohmann <mail@nlohmann.me> --------- Signed-off-by: ayush-singh-0601 <singhayush062006@gmail.com> Signed-off-by: Niels Lohmann <mail@nlohmann.me> Co-authored-by: ayush-singh-0601 <singhayush062006@gmail.com> Co-authored-by: ayush-singh-0601 <179524189+ayush-singh-0601@users.noreply.github.com>

ayush-singh-0601 requested a review from nlohmann as a code owner September 10, 2026 16:08

ayush-singh-0601 mentioned this pull request Sep 10, 2026

Stack overflow in binary format writers (CBOR/MessagePack/UBJSON/BSON) due to unbounded recursion #5392

Closed

ayush-singh-0601 force-pushed the binary-writer-stack-overflow branch from ead14ec to 7eb828e Compare September 10, 2026 19:44

github-actions Bot added L tests labels Sep 10, 2026

nlohmann added aspect: binary formats BSON, CBOR, MessagePack, UBJSON please rebase Please rebase your branch to origin/develop labels Sep 11, 2026

nlohmann requested changes Sep 12, 2026

View reviewed changes

ayush-singh-0601 force-pushed the binary-writer-stack-overflow branch from 7eb828e to cf65ac4 Compare September 23, 2026 10:52

ayush-singh-0601 requested a review from nlohmann September 23, 2026 11:25

nlohmann removed the please rebase Please rebase your branch to origin/develop label Sep 23, 2026

nlohmann requested changes Sep 23, 2026

View reviewed changes

gregmarr reviewed Sep 24, 2026

View reviewed changes

nlohmann added the please rebase Please rebase your branch to origin/develop label Sep 24, 2026

nlohmann reviewed Sep 27, 2026

View reviewed changes

This was referenced Sep 30, 2026

binary_writer: duplicated integer/float encoding ladders, duplicated BSON and BJData traversals, stale comments #5710

Closed

Deduplicate binary reader/writer helpers and fix stale comments #5730

Merged

This was referenced Oct 4, 2026

Throw type_error.321 when serializing discarded values to binary formats #5761

Merged

Stop binary writers overflowing the stack on deep values #5781

Merged

nlohmann closed this Oct 7, 2026

	void write_cbor_array_prefix(const std::size_t N)
	void write_cbor_array_prefix(const typename BasicJsonType::array_t::size_type N)

	void write_cbor_object_prefix(const std::size_t N)
	void write_cbor_object_prefix(const typename BasicJsonType::object_t::size_type N)

Conversation

ayush-singh-0601 commented Sep 10, 2026 • edited by nlohmann Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nlohmann left a comment

Choose a reason for hiding this comment

Uh oh!

nlohmann commented Sep 22, 2026

Uh oh!

ayush-singh-0601 commented Sep 23, 2026

Uh oh!

nlohmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gregmarr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nlohmann commented Sep 27, 2026

Uh oh!

ayush-singh-0601 commented Sep 27, 2026

Uh oh!

nlohmann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nlohmann commented Sep 27, 2026

Uh oh!

nlohmann commented Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ayush-singh-0601 commented Sep 10, 2026 •

edited by nlohmann

Loading