From e48800f09d6f5e31320371da7ebdda7f7c6e911e Mon Sep 17 00:00:00 2001 From: Niels Lohmann Date: Wed, 4 Nov 2026 23:42:17 +0100 Subject: [PATCH] Binary reader and writer: remove checks that invariants make dead (#6001) * Binary reader and writer: remove checks that invariants make dead Branch coverage of the test suite and of 24k mutated inputs showed these conditions as never taken; each is implied by a check made earlier on the same path: - check_bson_document_size: `document_size < 0` (the size was rejected below 5 before it was stored) - get_ubjson_ndarray_size: `second != 'N'` ('N' is rejected as a type marker before `second` can be set) - bjd_type_name() == nullptr for an ndarray dtype: every marker is_valid_optimized_type() accepts for BJData has a name (now an assert) - the early is_bjd_excluded_optimized_type() check in get_ubjson_size_type(): is_valid_optimized_type() rejects the same eight markers with the same error - `input_format == bjdata` next to `is_ndarray`, bit 8 of the type, or a 'B'/'u'/'m'/'M' marker: these only arise for BJData - `first != npos` next to bit 8: the bit is only set after a count - MessagePack writer: the `<= max` halves of the negative int8/16/32 ladder (the ladder is in the `else` of `n >= 0`) No change in behaviour: a build that aborts on each removed side passes the test suite and the fuzz corpora, and a 1.6M-input differential run of the reader against the original headers found no difference. Signed-off-by: Niels Lohmann * Remove is_bjd_excluded_optimized_type(), which has no caller left; fix two comments is_valid_optimized_type() rejects the markers BJData excludes, so the reader no longer needs its own list; the test now checks is_valid_optimized_type() of a BJData reader against the writer's list. The BSON size comment now says that open_bson_document() rejects a size below 5. Signed-off-by: Niels Lohmann * Make the BJData reader in the lookup test const (misc-const-correctness) Signed-off-by: Niels Lohmann --------- Signed-off-by: Niels Lohmann --- .../nlohmann/detail/input/binary_reader.hpp | 65 +++++----------- .../nlohmann/detail/output/binary_writer.hpp | 11 +-- single_include/nlohmann/json.hpp | 76 ++++++------------- tests/src/unit-bjdata.cpp | 16 ++-- 4 files changed, 57 insertions(+), 111 deletions(-) diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index b793ae097..93f707097 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -344,8 +344,8 @@ class binary_reader nested document could declare a length that disagrees with where its terminator actually falls and quietly hand the bytes in between to the enclosing document. A well-formed document is at least 5 bytes (the prefix - plus the terminator); the equality also rejects those impossible sizes, - since at least 5 bytes are always consumed. + plus the terminator); open_bson_document() rejects a smaller size before + it is stored, so @a document_size is never negative here. @param[in] document_start value of chars_read before the size prefix @param[in] document_size the declared document size @@ -353,7 +353,7 @@ class binary_reader */ bool check_bson_document_size(const std::size_t document_start, const std::int32_t document_size) { - if (JSON_HEDLEY_UNLIKELY(document_size < 0 || static_cast(document_size) != chars_read - document_start)) + if (JSON_HEDLEY_UNLIKELY(static_cast(document_size) != chars_read - document_start)) { return bson_document_size_error(document_size, chars_read - document_start); } @@ -3094,16 +3094,13 @@ class binary_reader { if (size_and_type.second != 0) { - if (size_and_type.second != 'N') + for (std::size_t i = 0; i < size_and_type.first; ++i) { - for (std::size_t i = 0; i < size_and_type.first; ++i) + if (JSON_HEDLEY_UNLIKELY(!get_ubjson_size_value(dimlen, no_ndarray, size_and_type.second))) { - if (JSON_HEDLEY_UNLIKELY(!get_ubjson_size_value(dimlen, no_ndarray, size_and_type.second))) - { - return false; - } - dim.push_back(dimlen); + return false; } + dim.push_back(dimlen); } } else @@ -3323,11 +3320,9 @@ class binary_reader // follow the documented _ArrayType_, _ArraySize_, _ArrayData_ order if (ndarray_dtype != 0) { + // every optimized type marker is_valid_optimized_type() accepts for BJData has a name const char* type_name = bjd_type_name(ndarray_dtype); - if (JSON_HEDLEY_UNLIKELY(type_name == nullptr)) - { - return unexpected_byte("invalid byte", "type"); - } + JSON_ASSERT(type_name != nullptr); string_t type_key = "_ArrayType_"; string_t type = type_name; // sax->string() takes a reference @@ -3410,12 +3405,6 @@ class binary_reader if (current == '$') { result.second = get(); // must not ignore 'N', because 'N' maybe the type - if (input_format == input_format_t::bjdata - && JSON_HEDLEY_UNLIKELY(is_bjd_excluded_optimized_type(result.second))) - { - return optimized_type_error(); - } - if (JSON_HEDLEY_UNLIKELY(!unexpect_eof("type"))) { return false; @@ -3443,7 +3432,7 @@ class binary_reader // an ndarray was read here only if the flag flipped; when it was // seeded true, get_ubjson_size_value() already rejected the nested // dimension vector - if (input_format == input_format_t::bjdata && is_ndarray && !inside_ndarray) + if (is_ndarray && !inside_ndarray) { result.second |= (1 << 8); // use bit 8 to indicate ndarray, all UBJSON and BJData markers should be ASCII letters } @@ -3453,7 +3442,7 @@ class binary_reader if (current == '#') { const bool is_error = get_ubjson_size_value(result.first, is_ndarray); - if (input_format == input_format_t::bjdata && is_ndarray && !inside_ndarray) + if (is_ndarray && !inside_ndarray) { return last_byte_error(exception_id::unexpected_byte, "ndarray requires both type and size", "size"); } @@ -3627,7 +3616,6 @@ class binary_reader JSON_HEDLEY_NEVER_INLINE bool parse_ubjson_typed_run(std::size_t& remaining, const char_int_type type, std::true_type /*bulk*/) { - const bool bjdata = input_format == input_format_t::bjdata; switch (type) { case 'U': @@ -3641,11 +3629,11 @@ class binary_reader case 'L': return parse_ubjson_typed_run(remaining); case 'u': - return !bjdata || parse_ubjson_typed_run(remaining); + return parse_ubjson_typed_run(remaining); case 'm': - return !bjdata || parse_ubjson_typed_run(remaining); + return parse_ubjson_typed_run(remaining); case 'M': - return !bjdata || parse_ubjson_typed_run(remaining); + return parse_ubjson_typed_run(remaining); case 'd': return parse_ubjson_typed_run(remaining); case 'D': @@ -3726,7 +3714,7 @@ class binary_reader // if bit-8 of size_and_type.second is set to 1, encode bjdata ndarray as an object in JData annotated array format (https://github.com/NeuroJSON/jdata): // {"_ArrayType_" : "typeid", "_ArraySize_" : [n1, n2, ...], "_ArrayData_" : [v1, v2, ...]} - if (input_format == input_format_t::bjdata && size_and_type.first != npos && (size_and_type.second & (1 << 8)) != 0) + if ((size_and_type.second & (1 << 8)) != 0) { size_and_type.second &= ~(static_cast(1) << 8); // use bit 8 to indicate ndarray, here we remove the bit to restore the type marker @@ -3785,7 +3773,7 @@ class binary_reader } // If BJData type marker is 'B' decode as binary - if (input_format == input_format_t::bjdata && size_and_type.first != npos && size_and_type.second == 'B') + if (size_and_type.first != npos && size_and_type.second == 'B') { binary_t result; return get_binary(size_and_type.first, result) && sax->binary(result); @@ -3834,7 +3822,7 @@ class binary_reader } // do not accept ND-array size in objects in BJData - if (input_format == input_format_t::bjdata && size_and_type.first != npos && (size_and_type.second & (1 << 8)) != 0) + if ((size_and_type.second & (1 << 8)) != 0) { return last_byte_error(exception_id::unexpected_byte, "BJData object does not support ND-array size in optimized format", "object"); } @@ -5115,8 +5103,9 @@ class binary_reader JSON_PRIVATE_UNLESS_TESTED: /*! @brief check whether @a marker is a type marker that may follow '$' - UBJSON: Z N T F i U I l L d D H C S [ {; BJData: the numeric markers and C - (BJData's excluded markers are rejected separately). + UBJSON: Z N T F i U I l L d D H C S [ {; BJData: the numeric markers and C. + The markers BJData excludes (F H N S T Z [ {) are the ones + binary_writer::is_bjdata_excluded_type_marker() lists; keep the two in sync. */ bool is_valid_optimized_type(const char_int_type marker) const noexcept { @@ -5151,20 +5140,6 @@ class binary_reader } } - /*! - @brief whether @a marker is excluded from BJData's optimized ND-array types - @return whether @a marker is one of 'F', 'H', 'N', 'S', 'T', 'Z', '[', '{' - - Mirrors binary_writer's @ref binary_writer::is_bjdata_excluded_type_marker - "is_bjdata_excluded_type_marker()`, which encodes the same list the other - way; keep the two in sync. - */ - static constexpr bool is_bjd_excluded_optimized_type(const char_int_type marker) noexcept - { - return marker == '[' || marker == '{' || marker == 'S' || marker == 'H' - || marker == 'T' || marker == 'F' || marker == 'N' || marker == 'Z'; - } - /*! @brief look up the ND-array element type name for a BJData dtype marker @return the type name ("uint8", "int8", ...), or nullptr if @a marker does diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 5bbaa42d2..dda9c3758 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -3126,22 +3126,19 @@ class binary_writer // negative fixnum write_number(static_cast(j.m_data.m_value.number_integer)); } - else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)() && - j.m_data.m_value.number_integer <= (std::numeric_limits::max)()) + else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)()) { // int 8 oa.write_character(to_char_type(0xD0)); write_number(static_cast(j.m_data.m_value.number_integer)); } - else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)() && - j.m_data.m_value.number_integer <= (std::numeric_limits::max)()) + else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)()) { // int 16 oa.write_character(to_char_type(0xD1)); write_number(static_cast(j.m_data.m_value.number_integer)); } - else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)() && - j.m_data.m_value.number_integer <= (std::numeric_limits::max)()) + else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)()) { // int 32 oa.write_character(to_char_type(0xD2)); @@ -4057,7 +4054,7 @@ class binary_writer Containers, strings, high-precision numbers, booleans and null cannot be declared as the single type of an optimized container in BJData; such a container is written unoptimized. The reader rejects them with the same - list (binary_reader::is_bjd_excluded_optimized_type()). + list (binary_reader::is_valid_optimized_type()). */ static constexpr bool is_bjdata_excluded_type_marker(const CharType marker) noexcept { diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 50089514f..eedb9c561 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -18360,8 +18360,8 @@ class binary_reader nested document could declare a length that disagrees with where its terminator actually falls and quietly hand the bytes in between to the enclosing document. A well-formed document is at least 5 bytes (the prefix - plus the terminator); the equality also rejects those impossible sizes, - since at least 5 bytes are always consumed. + plus the terminator); open_bson_document() rejects a smaller size before + it is stored, so @a document_size is never negative here. @param[in] document_start value of chars_read before the size prefix @param[in] document_size the declared document size @@ -18369,7 +18369,7 @@ class binary_reader */ bool check_bson_document_size(const std::size_t document_start, const std::int32_t document_size) { - if (JSON_HEDLEY_UNLIKELY(document_size < 0 || static_cast(document_size) != chars_read - document_start)) + if (JSON_HEDLEY_UNLIKELY(static_cast(document_size) != chars_read - document_start)) { return bson_document_size_error(document_size, chars_read - document_start); } @@ -21110,16 +21110,13 @@ class binary_reader { if (size_and_type.second != 0) { - if (size_and_type.second != 'N') + for (std::size_t i = 0; i < size_and_type.first; ++i) { - for (std::size_t i = 0; i < size_and_type.first; ++i) + if (JSON_HEDLEY_UNLIKELY(!get_ubjson_size_value(dimlen, no_ndarray, size_and_type.second))) { - if (JSON_HEDLEY_UNLIKELY(!get_ubjson_size_value(dimlen, no_ndarray, size_and_type.second))) - { - return false; - } - dim.push_back(dimlen); + return false; } + dim.push_back(dimlen); } } else @@ -21339,11 +21336,9 @@ class binary_reader // follow the documented _ArrayType_, _ArraySize_, _ArrayData_ order if (ndarray_dtype != 0) { + // every optimized type marker is_valid_optimized_type() accepts for BJData has a name const char* type_name = bjd_type_name(ndarray_dtype); - if (JSON_HEDLEY_UNLIKELY(type_name == nullptr)) - { - return unexpected_byte("invalid byte", "type"); - } + JSON_ASSERT(type_name != nullptr); string_t type_key = "_ArrayType_"; string_t type = type_name; // sax->string() takes a reference @@ -21426,12 +21421,6 @@ class binary_reader if (current == '$') { result.second = get(); // must not ignore 'N', because 'N' maybe the type - if (input_format == input_format_t::bjdata - && JSON_HEDLEY_UNLIKELY(is_bjd_excluded_optimized_type(result.second))) - { - return optimized_type_error(); - } - if (JSON_HEDLEY_UNLIKELY(!unexpect_eof("type"))) { return false; @@ -21459,7 +21448,7 @@ class binary_reader // an ndarray was read here only if the flag flipped; when it was // seeded true, get_ubjson_size_value() already rejected the nested // dimension vector - if (input_format == input_format_t::bjdata && is_ndarray && !inside_ndarray) + if (is_ndarray && !inside_ndarray) { result.second |= (1 << 8); // use bit 8 to indicate ndarray, all UBJSON and BJData markers should be ASCII letters } @@ -21469,7 +21458,7 @@ class binary_reader if (current == '#') { const bool is_error = get_ubjson_size_value(result.first, is_ndarray); - if (input_format == input_format_t::bjdata && is_ndarray && !inside_ndarray) + if (is_ndarray && !inside_ndarray) { return last_byte_error(exception_id::unexpected_byte, "ndarray requires both type and size", "size"); } @@ -21643,7 +21632,6 @@ class binary_reader JSON_HEDLEY_NEVER_INLINE bool parse_ubjson_typed_run(std::size_t& remaining, const char_int_type type, std::true_type /*bulk*/) { - const bool bjdata = input_format == input_format_t::bjdata; switch (type) { case 'U': @@ -21657,11 +21645,11 @@ class binary_reader case 'L': return parse_ubjson_typed_run(remaining); case 'u': - return !bjdata || parse_ubjson_typed_run(remaining); + return parse_ubjson_typed_run(remaining); case 'm': - return !bjdata || parse_ubjson_typed_run(remaining); + return parse_ubjson_typed_run(remaining); case 'M': - return !bjdata || parse_ubjson_typed_run(remaining); + return parse_ubjson_typed_run(remaining); case 'd': return parse_ubjson_typed_run(remaining); case 'D': @@ -21742,7 +21730,7 @@ class binary_reader // if bit-8 of size_and_type.second is set to 1, encode bjdata ndarray as an object in JData annotated array format (https://github.com/NeuroJSON/jdata): // {"_ArrayType_" : "typeid", "_ArraySize_" : [n1, n2, ...], "_ArrayData_" : [v1, v2, ...]} - if (input_format == input_format_t::bjdata && size_and_type.first != npos && (size_and_type.second & (1 << 8)) != 0) + if ((size_and_type.second & (1 << 8)) != 0) { size_and_type.second &= ~(static_cast(1) << 8); // use bit 8 to indicate ndarray, here we remove the bit to restore the type marker @@ -21801,7 +21789,7 @@ class binary_reader } // If BJData type marker is 'B' decode as binary - if (input_format == input_format_t::bjdata && size_and_type.first != npos && size_and_type.second == 'B') + if (size_and_type.first != npos && size_and_type.second == 'B') { binary_t result; return get_binary(size_and_type.first, result) && sax->binary(result); @@ -21850,7 +21838,7 @@ class binary_reader } // do not accept ND-array size in objects in BJData - if (input_format == input_format_t::bjdata && size_and_type.first != npos && (size_and_type.second & (1 << 8)) != 0) + if ((size_and_type.second & (1 << 8)) != 0) { return last_byte_error(exception_id::unexpected_byte, "BJData object does not support ND-array size in optimized format", "object"); } @@ -23131,8 +23119,9 @@ class binary_reader JSON_PRIVATE_UNLESS_TESTED: /*! @brief check whether @a marker is a type marker that may follow '$' - UBJSON: Z N T F i U I l L d D H C S [ {; BJData: the numeric markers and C - (BJData's excluded markers are rejected separately). + UBJSON: Z N T F i U I l L d D H C S [ {; BJData: the numeric markers and C. + The markers BJData excludes (F H N S T Z [ {) are the ones + binary_writer::is_bjdata_excluded_type_marker() lists; keep the two in sync. */ bool is_valid_optimized_type(const char_int_type marker) const noexcept { @@ -23167,20 +23156,6 @@ class binary_reader } } - /*! - @brief whether @a marker is excluded from BJData's optimized ND-array types - @return whether @a marker is one of 'F', 'H', 'N', 'S', 'T', 'Z', '[', '{' - - Mirrors binary_writer's @ref binary_writer::is_bjdata_excluded_type_marker - "is_bjdata_excluded_type_marker()`, which encodes the same list the other - way; keep the two in sync. - */ - static constexpr bool is_bjd_excluded_optimized_type(const char_int_type marker) noexcept - { - return marker == '[' || marker == '{' || marker == 'S' || marker == 'H' - || marker == 'T' || marker == 'F' || marker == 'N' || marker == 'Z'; - } - /*! @brief look up the ND-array element type name for a BJData dtype marker @return the type name ("uint8", "int8", ...), or nullptr if @a marker does @@ -31144,22 +31119,19 @@ class binary_writer // negative fixnum write_number(static_cast(j.m_data.m_value.number_integer)); } - else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)() && - j.m_data.m_value.number_integer <= (std::numeric_limits::max)()) + else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)()) { // int 8 oa.write_character(to_char_type(0xD0)); write_number(static_cast(j.m_data.m_value.number_integer)); } - else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)() && - j.m_data.m_value.number_integer <= (std::numeric_limits::max)()) + else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)()) { // int 16 oa.write_character(to_char_type(0xD1)); write_number(static_cast(j.m_data.m_value.number_integer)); } - else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)() && - j.m_data.m_value.number_integer <= (std::numeric_limits::max)()) + else if (j.m_data.m_value.number_integer >= (std::numeric_limits::min)()) { // int 32 oa.write_character(to_char_type(0xD2)); @@ -32075,7 +32047,7 @@ class binary_writer Containers, strings, high-precision numbers, booleans and null cannot be declared as the single type of an optimized container in BJData; such a container is written unoptimized. The reader rejects them with the same - list (binary_reader::is_bjd_excluded_optimized_type()). + list (binary_reader::is_valid_optimized_type()). */ static constexpr bool is_bjdata_excluded_type_marker(const CharType marker) noexcept { diff --git a/tests/src/unit-bjdata.cpp b/tests/src/unit-bjdata.cpp index ecc79959d..dd5831dbd 100644 --- a/tests/src/unit-bjdata.cpp +++ b/tests/src/unit-bjdata.cpp @@ -78,23 +78,25 @@ TEST_CASE("BJData") { SECTION("binary_reader BJData lookup tables") { - // both lookups are static member functions + // bjd_type_name() is static; is_valid_optimized_type() needs a BJData reader std::vector const data; using reader_t = nlohmann::detail::binary_reader; - // the excluded optimized-type markers must match binary_writer's - // is_bjdata_excluded_type_marker(), which encodes the same 8 markers + // the markers BJData excludes as optimized types must match + // binary_writer's is_bjdata_excluded_type_marker(), which encodes the + // same 8 markers + const reader_t reader(nlohmann::detail::input_adapter(data), json::input_format_t::bjdata); for (const char marker : - {'[', '{', 'S', 'H', 'T', 'F', 'N', 'Z' + {'[', '{', 'S', 'H', 'T', 'F', 'N', 'Z', 'x' }) { - CHECK(reader_t::is_bjd_excluded_optimized_type(static_cast(marker))); + CHECK(!reader.is_valid_optimized_type(static_cast(marker))); } for (const char marker : - {'U', 'i', 'u', 'I', 'm', 'l', 'M', 'L', 'd', 'D', 'C', 'B', 'x' + {'U', 'i', 'u', 'I', 'm', 'l', 'M', 'L', 'd', 'D', 'C', 'B', 'h' }) { - CHECK(!reader_t::is_bjd_excluded_optimized_type(static_cast(marker))); + CHECK(reader.is_valid_optimized_type(static_cast(marker))); } // every dtype marker must round-trip to its ND-array type name