Skip to content

Write BSON in linear time, without recursing per nesting level - #5553

Merged
nlohmann merged 6 commits into
developfrom
claude/iterative-bson-writer
Sep 25, 2026
Merged

nlohmann merged 6 commits into
developfrom
claude/iterative-bson-writer

Conversation

@nlohmann

Copy link
Copy Markdown
Owner

Note

This replaces #5549, which GitHub closed as merged into claude/iterative-diff while I reordered the stack, so that it no longer waits for the diff() PR. Nothing reached develop; the commit is unchanged. It is based on #5547 (update()/merge_patch()) and sits outside the GitHub stack; GitHub retargets it to develop once #5547 is merged. #5548 (diff()) now waits for #5390.

What & why

to_bson() has two problems with nested values:

Part of the same series as #5546, see the table there.

The change

Both passes are now iterative, and each length is computed exactly once:

  • calc_bson_sizes() computes the length of every document and array in one pass, each from the lengths of its entries, into a table ordered the way they are written.
  • write_bson_document() then writes the document, taking each length from the table.

Only the enclosing objects and arrays are kept on a stack, so a flat document allocates nothing for it.

Everything observable is unchanged:

  • The same bytes are written.
  • A key containing U+0000 still throws out_of_range.409 for the same first key, with the same diagnostics path, before anything is written.
  • A document too large for BSON still throws out_of_range.412 before anything is written.
  • A binary subtype above 255 still throws out_of_range.415 after the same partial output.

Verification

  • Byte-identical: a differential against develop, with and without JSON_DIAGNOSTICS:

    • 9,967 documents give byte-identical output;
    • 1,726 with a U+0000 key give the same 409, with the same message and diagnostic path;
    • 321 with oversized subtypes give the same partial output, checked through the adapter overload, and the same 415;
    • rejecting non-objects still gives 317.

    It covers json and ordered_json, and chains up to 1,000 levels deep.

  • Deep values: 1,000,000 levels of nested objects or arrays serialize and read back fine on a 1 MiB stack; develop crashes at 100,000.

  • Speed (clang -O3, medians):

    workload develop this PR
    flat object, 20,000 members 0.48 ms 0.47 ms
    flat array, 20,000 elements (its length was computed twice) 1.27 ms 0.80 ms
    3,000 nested objects 1.78 ms 0.85 ms
    200 levels deep (×100) 30.7 ms 0.93 ms
    20,000 small objects 3.27 ms 3.39 ms
  • Tests:

    • Documents and arrays round-trip at every depth up to 300.
    • A U+0000 key 200 levels down throws before anything is written.
    • Values nested 100,000 levels deep serialize.
    • A mutant that forgets to add a nested length to its parent fails 318 assertions, and the deep test crashes on develop.
  • make amalgamate + make check-amalgamation; no new clang-tidy or -Weverything findings.

to_bson.md documented the quadratic complexity; it now says linear again, with a version-history entry.

Public API impact

No breaking changes. Same output, same exceptions at the same points; only private helpers changed.


🤖 Generated with Claude Code

@nlohmann nlohmann added aspect: binary formats BSON, CBOR, MessagePack, UBJSON review needed It would be great if someone could review the proposed changes. labels Sep 24, 2026
@nlohmann nlohmann linked an issue Sep 24, 2026 that may be closed by this pull request
@nlohmann
nlohmann force-pushed the claude/iterative-merge-patch-update branch from e5a2e8d to 3379ed5 Compare September 24, 2026 07:39
@nlohmann
nlohmann force-pushed the claude/iterative-bson-writer branch from 25affee to 66044eb Compare September 24, 2026 07:39
@nlohmann
nlohmann force-pushed the claude/iterative-merge-patch-update branch from 3379ed5 to 6f971ae Compare September 24, 2026 15:11
An error occurred while trying to automatically change base from claude/iterative-merge-patch-update to claude/iterative-hash September 24, 2026 15:12
@nlohmann
nlohmann changed the base branch from claude/iterative-merge-patch-update to develop September 24, 2026 15:20
to_bson() had two problems with nested values:

- It recursed once per nesting level, so a value nested deeply enough -
  100,000 levels on an 8 MiB stack - exhausted the call stack and
  terminated the process, although parse() accepts such values without
  complaint.
- BSON prefixes every document and array with its length. The writer
  computed that length by walking the entire value below it, again for
  every nested document it wrote, which made serializing O(size x depth).
  A 200-level document took 30 ms instead of 1.

Both passes are now iterative, and each length is computed exactly once:

- calc_bson_sizes() computes the length of every document and array in
  one pass, each from the lengths of its entries, into a table ordered
  the way they are written.
- write_bson_document() then writes the document, taking each length from
  the table.

Everything observable is unchanged, as a differential test against
develop confirms byte for byte:

- The same bytes are written.
- A key containing U+0000 still throws out_of_range.409 for the same
  first key, with the same diagnostics path, before anything is written.
- A document too large for BSON still throws out_of_range.412 before
  anything is written.
- A binary subtype above 255 still throws out_of_range.415 after the
  same partial output.

Only the enclosing objects and arrays are kept on a stack, so a flat
document allocates nothing for it. Measured against develop (clang -O3,
median of 201 runs): flat objects unchanged, flat arrays 37% faster (the
array length was computed twice), a nested 3,000-object document 2x
faster, a 200-level document 33x faster.

to_bson.md documented the quadratic complexity since #5334; it is linear
again.

Fixes #5392 for BSON, and #5308.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
GCC 4.9 and MSVC rejected the test's huge_string_t, which has no default
constructor; develop never default-constructed string_t here either.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann force-pushed the claude/iterative-bson-writer branch from 66044eb to 448a130 Compare September 24, 2026 15:21
Comment thread include/nlohmann/detail/output/binary_writer.hpp Outdated
Comment thread include/nlohmann/detail/output/binary_writer.hpp Outdated
Comment thread include/nlohmann/detail/output/binary_writer.hpp Outdated
Comment thread include/nlohmann/detail/output/binary_writer.hpp Outdated
It returned a reference to the string it filled, so callers held a second
name for index_name. Addresses review feedback.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Signed-off-by: Niels Lohmann <mail@nlohmann.me>
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 25, 2026
Resolve the conflict with #5185 in unit-bson.cpp: both added a test case
at the end of the file; keep both.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
@nlohmann
nlohmann merged commit 95e9a59 into develop Sep 25, 2026
159 of 160 checks passed
@nlohmann
nlohmann deleted the claude/iterative-bson-writer branch September 25, 2026 19:56
nlohmann added a commit that referenced this pull request Sep 30, 2026
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>
nlohmann added a commit that referenced this pull request Sep 30, 2026
* 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 added a commit that referenced this pull request Oct 7, 2026
… 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>
nlohmann added a commit that referenced this pull request Oct 7, 2026
* 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>
nlohmann added a commit that referenced this pull request Oct 11, 2026
* Restore v3.12.0 support for custom object key types

Custom object_t types whose key_type is not string_t compiled with
v3.12.0 for several APIs that unreleased changes broke:

- to_bson failed for every custom key type (#5553 kept a const string_t*
  to the key); the nested entry's header is now written where the entry
  is found.
- Copying deep values (and parse, merge_patch, update, insert) required
  operator== on keys (#5389); keys without one are now paired via find().
- to_cbor/to_msgpack required an implicit conversion to string_t (#5746,
  #5328); keys without one go through a temporary basic_json again.
- at() required a conversion to string_t for its error message (#5727);
  other keys are passed to concat() unchanged again.

The new unit-custom-object-key-type.cpp covers five key types with
different capabilities.

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

* Use the with_object_t alias for the custom object key test types

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

* Avoid floating-point equality in custom key type test

GCC with -Werror=float-equal rejects comparing the double value with ==.

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

* Write the head of nested BSON elements in one helper

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

* Suppress bugprone-return-const-ref-from-parameter in key_for_message

The reference is only passed to concat() within the full-expression that
holds the key, like the similar helpers in binary_writer.hpp.

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

* Make the value of the custom test key types private

clang-tidy (cppcoreguidelines-non-private-member-variables-in-classes)
rejects the protected member; the derived key types use a protected
accessor instead.

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

* Pass keys with data() and size() unchanged into the at() miss message

key_for_message() converted every key that string_t can be constructed
from, so a miss on a string_t or string_view key copied it before
concat() copied it again. Keys that concat() can append through data()
and size() are now passed through; only other keys (string literals,
key types that just convert to string_t) are converted.

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

---------

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aspect: binary formats BSON, CBOR, MessagePack, UBJSON documentation L 🚀 ready to merge Ready to merge - just waiting for CI to complete. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

to_bson() is O(size × nesting depth), not linear as documented

2 participants