Add test coverage for documented lenient BSON input handling

Issue #5333 documented three intentionally-lenient behaviors of the BSON
reader (any non-zero byte accepted as a boolean `true`, BSON array element
keys not validated against the required decimal sequence, and the payload
of binary subtype 0x02 "old binary" returned as-is including its inner
length prefix), but none of them was pinned by a test, so a future change
could silently regress the documented behavior.

Also add coverage for the out_of_range.412 length-overflow check
(shared by binary, string, and (sub-)document BSON length fields) for
the string and document cases; only the binary case was previously
tested.

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

include/nlohmann/detail/conversions/from_json.hpp

@@ -398,34 +398,6 @@ inline void from_json(const BasicJsonType& j, CompatibleArrayType& bin)
     }
 }
 
-template<typename BasicJsonType, typename ConstructibleObjectType>
-auto from_json_object_impl(const BasicJsonType& j, ConstructibleObjectType& obj, priority_tag<1> /*unused*/)
--> decltype(
-    obj.reserve(std::declval<typename ConstructibleObjectType::size_type>()),
-    void())
-{
-    ConstructibleObjectType ret;
-    const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
-    ret.reserve(inner_object->size());
-    for (const auto& p : *inner_object)
-    {
-        ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
-    }
-    obj = std::move(ret);
-}
-
-template<typename BasicJsonType, typename ConstructibleObjectType>
-inline void from_json_object_impl(const BasicJsonType& j, ConstructibleObjectType& obj, priority_tag<0> /*unused*/)
-{
-    ConstructibleObjectType ret;
-    const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
-    for (const auto& p : *inner_object)
-    {
-        ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
-    }
-    obj = std::move(ret);
-}
-
 template<typename BasicJsonType, typename ConstructibleObjectType,
          enable_if_t<is_constructible_object_type<BasicJsonType, ConstructibleObjectType>::value, int> = 0>
 inline void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
@@ -435,7 +407,13 @@ inline void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
         JSON_THROW(type_error::create(302, concat("type must be object, but is ", j.type_name()), &j));
     }
 
-    from_json_object_impl(j, obj, priority_tag<1> {});
+    ConstructibleObjectType ret;
+    const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
+    for (const auto& p : *inner_object)
+    {
+        ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
+    }
+    obj = std::move(ret);
 }
 
 // overload for arithmetic types, not chosen for basic_json template arguments

single_include/nlohmann/json.hpp

@@ -5693,34 +5693,6 @@ inline void from_json(const BasicJsonType& j, CompatibleArrayType& bin)
     }
 }
 
-template<typename BasicJsonType, typename ConstructibleObjectType>
-auto from_json_object_impl(const BasicJsonType& j, ConstructibleObjectType& obj, priority_tag<1> /*unused*/)
--> decltype(
-    obj.reserve(std::declval<typename ConstructibleObjectType::size_type>()),
-    void())
-{
-    ConstructibleObjectType ret;
-    const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
-    ret.reserve(inner_object->size());
-    for (const auto& p : *inner_object)
-    {
-        ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
-    }
-    obj = std::move(ret);
-}
-
-template<typename BasicJsonType, typename ConstructibleObjectType>
-inline void from_json_object_impl(const BasicJsonType& j, ConstructibleObjectType& obj, priority_tag<0> /*unused*/)
-{
-    ConstructibleObjectType ret;
-    const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
-    for (const auto& p : *inner_object)
-    {
-        ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
-    }
-    obj = std::move(ret);
-}
-
 template<typename BasicJsonType, typename ConstructibleObjectType,
          enable_if_t<is_constructible_object_type<BasicJsonType, ConstructibleObjectType>::value, int> = 0>
 inline void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
@@ -5730,7 +5702,13 @@ inline void from_json(const BasicJsonType& j, ConstructibleObjectType& obj)
         JSON_THROW(type_error::create(302, concat("type must be object, but is ", j.type_name()), &j));
     }
 
-    from_json_object_impl(j, obj, priority_tag<1> {});
+    ConstructibleObjectType ret;
+    const auto* inner_object = j.template get_ptr<const typename BasicJsonType::object_t*>();
+    for (const auto& p : *inner_object)
+    {
+        ret.emplace(p.first, p.second.template get<typename ConstructibleObjectType::mapped_type>());
+    }
+    obj = std::move(ret);
 }
 
 // overload for arithmetic types, not chosen for basic_json template arguments

tests/src/unit-bson.cpp

@@ -38,6 +38,26 @@ class huge_binary_t : public std::vector<std::uint8_t>
 using huge_binary_json = nlohmann::basic_json <
                          std::map, std::vector, std::string, bool, std::int64_t, std::uint64_t,
                          double, std::allocator, nlohmann::adl_serializer, huge_binary_t, void >;
+
+// a string type that reports a size beyond INT32_MAX without allocating that
+// much memory, so BSON length overflow can be tested for strings and
+// (embedded) documents as well, following the same idea as huge_binary_t
+class huge_string_t : public std::string
+{
+  public:
+    using std::string::string;
+    huge_string_t(const std::string& s) : std::string(s) {} // NOLINT(google-explicit-constructor,hicpp-explicit-conversions)
+
+    size_type size() const noexcept // NOLINT(readability-convert-member-functions-to-static)
+    {
+        // one byte more than the BSON length field can represent
+        return static_cast<size_type>((std::numeric_limits<std::int32_t>::max)()) + 1;
+    }
+};
+
+using huge_string_json = nlohmann::basic_json <
+                         std::map, std::vector, huge_string_t, bool, std::int64_t, std::uint64_t,
+                         double, std::allocator, nlohmann::adl_serializer, std::vector<std::uint8_t>, void >;
 } // namespace
 
 TEST_CASE("BSON")
@@ -105,10 +125,36 @@ TEST_CASE("BSON")
 
     SECTION("lengths exceeding INT32_MAX cannot be serialized to BSON")
     {
-        huge_binary_json j;
-        j["b"] = huge_binary_json::binary(huge_binary_t{});
+        // out_of_range.412 is thrown from a single shared helper
+        // (to_bson_length) that guards the BSON length fields of binary
+        // values, strings, and (embedded) documents alike
+        SECTION("binary")
+        {
+            huge_binary_json j;
+            j["b"] = huge_binary_json::binary(huge_binary_t{});
 
-        CHECK_THROWS_WITH_AS(huge_binary_json::to_bson(j), "[json.exception.out_of_range.412] BSON length 2147483661 exceeds maximum of 2147483647", huge_binary_json::out_of_range&);
+            CHECK_THROWS_WITH_AS(huge_binary_json::to_bson(j), "[json.exception.out_of_range.412] BSON length 2147483661 exceeds maximum of 2147483647", huge_binary_json::out_of_range&);
+        }
+
+        SECTION("string")
+        {
+            huge_string_json j;
+            j["s"] = huge_string_json::string_t("value");
+
+            CHECK_THROWS_WITH_AS(huge_string_json::to_bson(j), "[json.exception.out_of_range.412] BSON length 4294967308 exceeds maximum of 2147483647", huge_string_json::out_of_range&);
+        }
+
+        SECTION("document")
+        {
+            // an oversized string nested one level deep makes the
+            // *embedded* document's own length exceed INT32_MAX as well
+            huge_string_json nested;
+            nested["s"] = huge_string_json::string_t("value");
+            huge_string_json j;
+            j["nested"] = nested;
+
+            CHECK_THROWS_WITH_AS(huge_string_json::to_bson(j), "[json.exception.out_of_range.412] BSON length 6442450963 exceeds maximum of 2147483647", huge_string_json::out_of_range&);
+        }
     }
 
     SECTION("string length must be at least 1")
@@ -193,6 +239,23 @@ TEST_CASE("BSON")
             CHECK(json::from_bson(result, true, false) == j);
         }
 
+        SECTION("non-empty object with bool from a non-0/1 byte (lenient parsing)")
+        {
+            // documented lenient behavior (see gh-5333): any non-zero byte
+            // is accepted as `true`, not just 0x01
+            std::vector<std::uint8_t> const input =
+            {
+                0x0D, 0x00, 0x00, 0x00, // size (little endian)
+                0x08,               // entry: boolean
+                'e', 'n', 't', 'r', 'y', '\x00',
+                0x02,           // value = 0x02 (neither 0x00 nor 0x01)
+                0x00                    // end marker
+            };
+
+            const json expected = { { "entry", true } };
+            CHECK(json::from_bson(input) == expected);
+        }
+
         SECTION("non-empty object with double")
         {
             json const j =
@@ -499,6 +562,29 @@ TEST_CASE("BSON")
             CHECK(json::from_bson(result, true, false) == j);
         }
 
+        SECTION("array elements with non-conforming keys (lenient parsing)")
+        {
+            // documented lenient behavior (see gh-5333): BSON array element
+            // keys are not checked against the required decimal sequence
+            // "0", "1", "2", ... - elements are taken in encoded order
+            std::vector<std::uint8_t> const input =
+            {
+                0x26, 0x00, 0x00, 0x00, // size (little endian)
+                0x04, 'e', 'n', 't', 'r', 'y', '\x00', // entry: embedded array
+
+                0x1A, 0x00, 0x00, 0x00, // size (little endian)
+                0x10, '5', 0x00, 0x0A, 0x00, 0x00, 0x00, // key "5" (bogus)      -> 10
+                0x10, 'x', 0x00, 0x14, 0x00, 0x00, 0x00, // key "x" (non-numeric) -> 20
+                0x10, '1', 0x00, 0x1E, 0x00, 0x00, 0x00, // key "1" (out of order) -> 30
+                0x00, // end marker (embedded array)
+
+                0x00 // end marker
+            };
+
+            const json expected = { { "entry", json::array({10, 20, 30}) } };
+            CHECK(json::from_bson(input) == expected);
+        }
+
         SECTION("non-empty object with binary member")
         {
             const size_t N = 10;
@@ -594,6 +680,31 @@ 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)")
+        {
+            // 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
+            std::vector<std::uint8_t> const 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 prefix (part of the old-binary payload)
+                0x68, 0x69, // payload ('h', 'i')
+
+                0x00 // end marker
+            };
+
+            // the inner length prefix is part of the (unmodified) payload
+            const std::vector<std::uint8_t> expected_payload = {0x02, 0x00, 0x00, 0x00, 0x68, 0x69};
+            const json expected = { { "entry", json::binary(expected_payload, 0x02) } };
+            CHECK(json::from_bson(input) == expected);
+        }
+
         SECTION("Some more complex document")
         {
             json const j =

tests/src/unit-conversions.cpp

@@ -1389,37 +1389,6 @@ TEST_CASE("value conversion")
                 // CHECK(m5["one"] == "eins");
             }
 
-            SECTION("reserve is called on containers that support it (#5406)")
-            {
-                // build a larger object so that a missing/incorrect reserve()
-                // call would be more likely to corrupt or drop elements
-                json j_large;
-                for (int i = 0; i < 100; ++i)
-                {
-                    j_large[std::to_string(i)] = i;
-                }
-
-                SECTION("std::unordered_map (supports reserve)")
-                {
-                    const auto m = j_large.get<std::unordered_map<std::string, int>>();
-                    CHECK(m.size() == 100);
-                    for (int i = 0; i < 100; ++i)
-                    {
-                        CHECK(m.at(std::to_string(i)) == i);
-                    }
-                }
-
-                SECTION("std::map (no reserve, fallback path)")
-                {
-                    const auto m = j_large.get<std::map<std::string, int>>();
-                    CHECK(m.size() == 100);
-                    for (int i = 0; i < 100; ++i)
-                    {
-                        CHECK(m.at(std::to_string(i)) == i);
-                    }
-                }
-            }
-
             SECTION("std::multimap")
             {
                 j1.get<std::multimap<std::string, int>>();