diff --git a/include/nlohmann/detail/input/binary_reader.hpp b/include/nlohmann/detail/input/binary_reader.hpp index 4c8d6ed0f..6d750bad6 100644 --- a/include/nlohmann/detail/input/binary_reader.hpp +++ b/include/nlohmann/detail/input/binary_reader.hpp @@ -584,9 +584,46 @@ class binary_reader } result.set_subtype(subtype); + // the "old binary" subtype 0x02 starts with an int32 that repeats the + // size of the payload following it; the payload does not include it + if (subtype == 0x02) + { + return get_bson_old_binary(len, result); + } + return get_binary(len, result); } + /*! + @brief Parses the payload of a BSON binary value with subtype 0x02. + @param[in] len The length of the binary value, including the inner int32. + @param[in,out] result A reference to the binary variable where the payload + (without the inner int32) is to be stored. + @pre len >= 0 + @return `true` if the inner length was present and matches @a len + */ + template + bool get_bson_old_binary(const NumberType len, binary_t& result) + { + if (JSON_HEDLEY_UNLIKELY(len < 4)) + { + return bson_length_error("length of binary subtype 0x02 must be at least 4, is ", len, "binary"); + } + + std::int32_t inner_length{}; + if (JSON_HEDLEY_UNLIKELY((!get_number(inner_length)))) + { + return false; + } + + if (JSON_HEDLEY_UNLIKELY(static_cast(inner_length) != static_cast(len) - 4)) + { + return bson_length_error("inner length of binary subtype 0x02 does not match the payload size; inner length is ", inner_length, "binary"); + } + + return get_binary(inner_length, result); + } + /*! @brief Read a BSON document element of the given @a element_type. @param[in] element_type The BSON element type, c.f. http://bsonspec.org/spec.html diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 7ff828074..d8b3e7162 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -1684,7 +1684,15 @@ class binary_writer throw_subtype_out_of_range(value.subtype(), "the BSON binary subtype", j); } - return sizeof(std::int32_t) + value.size() + 1ul; + // the "old binary" subtype 0x02 repeats the payload size in an inner + // int32 in front of the payload + return sizeof(std::int32_t) + (is_bson_old_binary(value) ? sizeof(std::int32_t) : 0ul) + value.size() + 1ul; + } + + /// @return whether @a value has the BSON subtype 0x02 ("binary (old)") + static bool is_bson_old_binary(const binary_t& value) noexcept + { + return value.has_subtype() && value.subtype() == 0x02; } /*! @@ -1697,10 +1705,18 @@ class binary_writer { write_bson_entry_header(name, 0x05); - write_number(to_bson_length(value.size()), true); + const bool old_binary = is_bson_old_binary(value); + + // the length field counts the inner int32 of the old binary subtype + write_number(to_bson_length(value.size() + (old_binary ? sizeof(std::int32_t) : 0ul)), true); write_number(value.has_subtype() ? static_cast(value.subtype()) : static_cast(0x00)); + if (old_binary) + { + write_number(to_bson_length(value.size()), true); + } + write_bytes(value.data(), value.size()); } @@ -1860,7 +1876,8 @@ class binary_writer (or the header of a string or binary value) in one reservation @return whether the entry was written; false for a discarded value, a string or binary value whose length does not fit the length - field, in which case nothing was written + field, or a binary value with subtype 0x02, in which case nothing + was written @pre @a name is at most bson_scalar_max_name_size characters long and is not changed by the error handler */ @@ -1921,7 +1938,9 @@ class binary_writer if (j.is_binary()) { const binary_t& value = *j.m_data.m_value.binary; - if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(value.size()))) + // the old binary subtype 0x02 has an inner length that the single + // reservation has no room for; write_bson_binary writes it + if (JSON_HEDLEY_UNLIKELY(!value_in_range_of(value.size()) || is_bson_old_binary(value))) { return false; } diff --git a/tests/src/unit-bson.cpp b/tests/src/unit-bson.cpp index 03c901cee..25d7b70bb 100644 --- a/tests/src/unit-bson.cpp +++ b/tests/src/unit-bson.cpp @@ -802,12 +802,11 @@ TEST_CASE("BSON") CHECK(json::from_bson(result, true, false) == j); } - SECTION("binary member with subtype 0x02 (old binary) keeps its inner length prefix (lenient parsing)") + SECTION("binary member with subtype 0x02 (old binary) carries an inner length") { - // documented lenient behavior (see gh-5333): the payload for - // binary subtype 0x02 ("old binary") is returned as-is, - // including its own inner 4-byte length prefix; it is not - // stripped or reinterpreted + // the payload of binary subtype 0x02 ("old binary") starts with an + // int32 that repeats the payload size; the bytes below are what + // pymongo and the bson-corpus (binary.json) use std::vector const input = { 0x17, 0x00, 0x00, 0x00, // size (little endian) @@ -815,16 +814,87 @@ TEST_CASE("BSON") 0x06, 0x00, 0x00, 0x00, // size of binary (little endian) 0x02, // "old binary" subtype - 0x02, 0x00, 0x00, 0x00, // inner length prefix (part of the old-binary payload) + 0x02, 0x00, 0x00, 0x00, // inner length 0x68, 0x69, // payload ('h', 'i') 0x00 // end marker }; - // the inner length prefix is part of the (unmodified) payload - const std::vector expected_payload = {0x02, 0x00, 0x00, 0x00, 0x68, 0x69}; - const json expected = { { "entry", json::binary(expected_payload, 0x02) } }; + // the inner length is not part of the payload + const json expected = { { "entry", json::binary({0x68, 0x69}, 0x02) } }; CHECK(json::from_bson(input) == expected); + CHECK(json::to_bson(expected) == input); + CHECK(json::from_bson(json::to_bson(expected)) == expected); + } + + SECTION("binary member with subtype 0x02 (old binary) and empty payload") + { + std::vector const input = + { + 0x15, 0x00, 0x00, 0x00, // size (little endian) + 0x05, 'e', 'n', 't', 'r', 'y', '\x00', // entry: binary + 0x04, 0x00, 0x00, 0x00, // size of binary (little endian) + 0x02, // "old binary" subtype + 0x00, 0x00, 0x00, 0x00, // inner length + 0x00 // end marker + }; + + const json expected = { { "entry", json::binary({}, 0x02) } }; + CHECK(json::from_bson(input) == expected); + CHECK(json::to_bson(expected) == input); + } + + SECTION("binary member with subtype 0x02 (old binary) with an invalid inner length") + { + std::vector input = + { + 0x17, 0x00, 0x00, 0x00, // size (little endian) + 0x05, 'e', 'n', 't', 'r', 'y', '\x00', // entry: binary + 0x06, 0x00, 0x00, 0x00, // size of binary (little endian) + 0x02, // "old binary" subtype + 0x02, 0x00, 0x00, 0x00, // inner length + 0x68, 0x69, // payload ('h', 'i') + 0x00 // end marker + }; + + SECTION("inner length too long") + { + input[19] = 0x03; + } + + SECTION("inner length too short") + { + input[19] = 0x01; + } + + SECTION("inner length negative") + { + input[19] = 0xFF; + input[20] = 0xFF; + input[21] = 0xFF; + input[22] = 0xFF; + } + + CAPTURE(input) + json _; + CHECK_THROWS_AS(_ = json::from_bson(input), json::parse_error&); + } + + SECTION("binary member with subtype 0x02 (old binary) without an inner length") + { + // the payload is shorter than the inner length field + std::vector const input = + { + 0x14, 0x00, 0x00, 0x00, // size (little endian) + 0x05, 'e', 'n', 't', 'r', 'y', '\x00', // entry: binary + 0x03, 0x00, 0x00, 0x00, // size of binary (little endian) + 0x02, // "old binary" subtype + 0x01, 0x02, 0x03, // payload without an inner length + 0x00 // end marker + }; + + json _; + CHECK_THROWS_AS(_ = json::from_bson(input), json::parse_error&); } SECTION("Some more complex document")