Compare dump()s instead of json values in the BJData fuzzer's round-trip check

The value-stability assertion added to fix the earlier OSS-Fuzz crash
(json::from_bjdata(to_bjdata(j2)) == j2) itself broke on a NaN payload:
IEEE 754 NaN is never equal to itself, so operator== reports two
structurally-identical trees containing a non-finite double as
different -- not a round-trip bug, just NaN's ordinary
non-reflexivity. dump() serializes any non-finite double the same
deterministic way (as JSON null, since JSON cannot represent NaN or
Infinity), so comparing dumps is stable under exactly the values that
break operator==.

Verified against both the original OSS-Fuzz crash input and the new
one (0x68 0x68 0x7c, which decodes to a NaN), plus a local 2.5M-case
random-input sweep with no failures.

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

include/nlohmann/detail/output/binary_writer.hpp

@@ -1672,6 +1672,16 @@ class binary_writer
         };
 
         string_t key = "_ArrayType_";
+        // the type name is looked up as a string below; a non-string
+        // annotation (e.g. a number, null, or an array) cannot name a known
+        // dtype, so it is treated the same as an unrecognized type name and
+        // falls back to a plain object encoding instead of throwing
+        // type_error.302 out of get<string_t>()
+        if (!value.at(key).is_string())
+        {
+            return true;
+        }
+
         // use get<string_t>() instead of static_cast<string_t> to avoid an
         // ambiguous conversion under explicit instantiation on C++17 (see #4825)
         auto it = bjdtype.find(value.at(key).template get<string_t>());
@@ -1681,6 +1691,16 @@ class binary_writer
         }
         CharType dtype = it->second;
 
+        // the 'B' (byte) marker is only defined by BJData Draft 3; emitting it
+        // under the default Draft 2 mode would produce a stream that Draft 2
+        // readers reject, so such an object falls back to a plain object
+        // encoding instead (see the "Binary values" section of the BJData
+        // documentation)
+        if (dtype == 'B' && bjdata_version != bjdata_version_t::draft3)
+        {
+            return true;
+        }
+
         key = "_ArraySize_";
         // the dimensions are written verbatim as the header length below, so a
         // value that is not an array cannot produce a valid one: null emits 'Z'

single_include/nlohmann/json.hpp

@@ -18832,6 +18832,16 @@ class binary_writer
         };
 
         string_t key = "_ArrayType_";
+        // the type name is looked up as a string below; a non-string
+        // annotation (e.g. a number, null, or an array) cannot name a known
+        // dtype, so it is treated the same as an unrecognized type name and
+        // falls back to a plain object encoding instead of throwing
+        // type_error.302 out of get<string_t>()
+        if (!value.at(key).is_string())
+        {
+            return true;
+        }
+
         // use get<string_t>() instead of static_cast<string_t> to avoid an
         // ambiguous conversion under explicit instantiation on C++17 (see #4825)
         auto it = bjdtype.find(value.at(key).template get<string_t>());
@@ -18841,6 +18851,16 @@ class binary_writer
         }
         CharType dtype = it->second;
 
+        // the 'B' (byte) marker is only defined by BJData Draft 3; emitting it
+        // under the default Draft 2 mode would produce a stream that Draft 2
+        // readers reject, so such an object falls back to a plain object
+        // encoding instead (see the "Binary values" section of the BJData
+        // documentation)
+        if (dtype == 'B' && bjdata_version != bjdata_version_t::draft3)
+        {
+            return true;
+        }
+
         key = "_ArraySize_";
         // the dimensions are written verbatim as the header length below, so a
         // value that is not an array cannot produce a valid one: null emits 'Z'

tests/src/fuzzer-parse_bjdata.cpp

@@ -21,6 +21,27 @@ array data, it performs the following steps:
 - j4 = from_bjdata(vec3)
 - assert(j1 == j4)
 
+Re-serializing j2/j3/j4 with the same use_size/use_type settings is checked
+for value-stability rather than byte-exact stability: from_bjdata(to_bjdata(j2))
+must equal j2 (and likewise for j3, j4). Byte-exact stability does not hold in
+general, because a BJData value can lose type fidelity across a round trip
+(e.g. a binary_t value serialized without the optimized "$U#" array header is
+parsed back as a plain array of numbers, see #5398 and the discussion on
+PR #5494) - the numeric value is preserved, but the writer's smallest-type
+selection for the now-plain numbers may legitimately pick a different, but
+equally valid, single-byte type marker than the dedicated binary-data writer
+would have. Both encodings are valid BJData and both decode to the same
+value, so this is not treated as a round-trip failure here.
+
+"Value-stable" is checked by comparing dump()s rather than with operator==
+directly: a BJData/UBJSON payload can decode to a non-finite double (NaN or
++-Infinity), and IEEE 754 NaN is never equal to itself, so operator== would
+report two structurally-identical trees as different whenever a NaN is
+involved -- not a round-trip bug, just NaN's ordinary (non-)reflexivity.
+dump() serializes any non-finite double the same deterministic way (as JSON
+`null`, since JSON itself cannot represent NaN/Infinity), so comparing
+dumps is stable under exactly the same values that break operator==.
+
 The provided function `LLVMFuzzerTestOneInput` can be used in different fuzzer
 drivers.
 */
@@ -31,6 +52,13 @@ drivers.
 
 using json = nlohmann::json;
 
+// value-stable comparison for the round-trip checks below; see the note
+// above on why this compares dump()s rather than the json values directly
+static bool is_value_stable(const json& lhs, const json& rhs)
+{
+    return lhs.dump() == rhs.dump();
+}
+
 // see http://llvm.org/docs/LibFuzzer.html
 extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
 {
@@ -56,10 +84,12 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
             json const j3 = json::from_bjdata(vec3);
             json const j4 = json::from_bjdata(vec4);
 
-            // serializations must match
-            assert(json::to_bjdata(j2, false, false) == vec2);
-            assert(json::to_bjdata(j3, true, false) == vec3);
-            assert(json::to_bjdata(j4, true, true) == vec4);
+            // re-serializing must be value-stable (see the notes above on
+            // why byte-exact stability is not guaranteed in general, and
+            // why this compares dump()s rather than the values directly)
+            assert(is_value_stable(json::from_bjdata(json::to_bjdata(j2, false, false)), j2));
+            assert(is_value_stable(json::from_bjdata(json::to_bjdata(j3, true, false)), j3));
+            assert(is_value_stable(json::from_bjdata(json::to_bjdata(j4, true, true)), j4));
         }
         catch (const json::parse_error&)
         {

tests/src/unit-bjdata.cpp

@@ -2586,7 +2586,12 @@ TEST_CASE("BJData")
                 CHECK(json::to_bjdata(json::from_bjdata(v_d), true, true) == v_d);
                 CHECK(json::to_bjdata(json::from_bjdata(v_D), true, true) == v_D);
                 CHECK(json::to_bjdata(json::from_bjdata(v_C), true, true) == v_C);
-                CHECK(json::to_bjdata(json::from_bjdata(v_B), true, true) == v_B);
+                // v_B uses the Draft-3-only 'B' marker, so it round-trips only when
+                // Draft 3 is explicitly selected (see GitHub issue #5404); the
+                // default Draft 2 falls back to a plain object instead, covered by
+                // the "ndarray with _ArrayType_ "byte" is gated by the BJData draft
+                // version" section below
+                CHECK(json::to_bjdata(json::from_bjdata(v_B), true, true, json::bjdata_version_t::draft3) == v_B);
             }
 
             SECTION("ndarray with data not matching _ArrayType_ is written as an object")
@@ -2629,8 +2634,10 @@ TEST_CASE("BJData")
                 // the C++ API stores an int literal as number_integer, so _ArrayType_
                 // names the wire type rather than the storage. Both storages have to
                 // produce the same typed array for every type.
+                // "byte" is checked separately below since it additionally requires
+                // BJData Draft 3 to be selected explicitly (see GitHub issue #5404).
                 for (const char* type :
-                        {"uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "char", "byte"
+                        {"uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "char"
                         })
                 {
                     CAPTURE(type);
@@ -2641,6 +2648,14 @@ TEST_CASE("BJData")
                     CHECK(from_text == json::to_bjdata(json({{"_ArrayType_", type}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}})));
                 }
 
+                {
+                    const std::string text = R"({"_ArrayType_":"byte","_ArraySize_":[2,3],"_ArrayData_":[1,2,3,4,5,6]})";
+                    const auto from_text = json::to_bjdata(json::parse(text), true, true, json::bjdata_version_t::draft3);
+                    CHECK(from_text.at(0) == '[');
+                    CHECK(from_text == json::to_bjdata(json({{"_ArrayType_", "byte"}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}}),
+                    true, true, json::bjdata_version_t::draft3));
+                }
+
                 // negative values under a signed type behave the same way
                 const auto from_neg = json::to_bjdata(json::parse(R"({"_ArrayType_":"int32","_ArraySize_":[2],"_ArrayData_":[-5,7]})"));
                 CHECK(from_neg.at(0) == '[');
@@ -2731,6 +2746,83 @@ TEST_CASE("BJData")
                 CHECK(json::from_bjdata(json::to_bjdata(j_size), true, true) == j_size);
             }
 
+            SECTION("ndarray whose _ArrayType_ is not a string stays as object")
+            {
+                // the type name is looked up as a string below the annotation
+                // check; a non-string _ArrayType_ cannot name a known dtype,
+                // so calling get<string_t>() on it would throw type_error.302
+                // instead of falling back like an unrecognized type name
+                // already does (see GitHub issue #5398)
+                json const j_number = json({{"_ArrayType_", 1}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 2}}});
+                const auto out_number = json::to_bjdata(j_number);
+                CHECK(out_number.at(0) == '{');
+                CHECK(json::from_bjdata(out_number) == j_number);
+
+                json const j_null = json({{"_ArrayType_", nullptr}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 2}}});
+                const auto out_null = json::to_bjdata(j_null);
+                CHECK(out_null.at(0) == '{');
+                CHECK(json::from_bjdata(out_null) == j_null);
+
+                json const j_bool = json({{"_ArrayType_", true}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 2}}});
+                const auto out_bool = json::to_bjdata(j_bool);
+                CHECK(out_bool.at(0) == '{');
+                CHECK(json::from_bjdata(out_bool) == j_bool);
+
+                json const j_array = json({{"_ArrayType_", {"uint8"}}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 2}}});
+                const auto out_array = json::to_bjdata(j_array);
+                CHECK(out_array.at(0) == '{');
+                CHECK(json::from_bjdata(out_array) == j_array);
+
+                json const j_object = json({{"_ArrayType_", {{"a", 1}}}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 2}}});
+                const auto out_object = json::to_bjdata(j_object);
+                CHECK(out_object.at(0) == '{');
+                CHECK(json::from_bjdata(out_object) == j_object);
+            }
+
+            SECTION("re-serializing a value containing a plain-array-of-bytes is value-stable but not byte-stable")
+            {
+                // OSS-Fuzz found this input (an array whose first element is a
+                // binary_t byte, followed by an object whose _ArrayType_ is
+                // not a string) while exercising the fix for #5398 above: once
+                // the fix stops to_bjdata() from throwing type_error.302 for
+                // the third element, serialization proceeds far enough to
+                // reach a pre-existing, unrelated round-trip quirk in how a
+                // single-byte binary_t value is re-encoded.
+                std::vector<std::uint8_t> const input
+                {
+                    0x5b, 0x5b, 0x24, 0x42, 0x23, 0x5b, 0x69, 0x01, 0x5d, 0x5b, 0x5b, 0x5d, 0x7b, 0x55, 0x0b,
+                    0x5f, 0x41, 0x72, 0x72, 0x61, 0x79, 0x44, 0x61, 0x74, 0x61, 0x5f, 0x54, 0x55, 0x0b, 0x5f,
+                    0x41, 0x72, 0x72, 0x61, 0x79, 0x53, 0x69, 0x7a, 0x65, 0x5f, 0x5a, 0x55, 0x0b, 0x5f, 0x41,
+                    0x72, 0x72, 0x61, 0x79, 0x54, 0x79, 0x70, 0x65, 0x5f, 0x54, 0x7d, 0x5d
+                };
+                json const j1 = json::from_bjdata(input);
+
+                // to_bjdata() must not throw (this is what #5398 fixes)
+                std::vector<std::uint8_t> vec2;
+                CHECK_NOTHROW(vec2 = json::to_bjdata(j1, false, false));
+
+                // parsing back a plain (non-optimized) array of bytes cannot
+                // recover that it used to be a binary_t: from_bjdata() has no
+                // way to distinguish "array of uint8 numbers" from "array of
+                // bytes" unless the compact "$U#" array header is used, so
+                // the binary_t collapses into a plain JSON array
+                json const j2 = json::from_bjdata(vec2);
+                CHECK(j1 != j2);
+                CHECK(j2 == json({{91}, json::array(), {{"_ArrayData_", true}, {"_ArraySize_", nullptr}, {"_ArrayType_", true}}}));
+
+                // re-serializing j2 no longer goes through the dedicated
+                // binary_t writer (which always uses the 'U' marker for raw
+                // bytes); the now-plain number 91 goes through the generic
+                // smallest-type writer instead, which - like the rest of the
+                // UBJSON/BJData writer, and unchanged by this fix - prefers
+                // the 'i' (int8) marker over 'U' (uint8) for values that fit
+                // both. Both markers are valid BJData and both decode back to
+                // 91, so this is not byte-for-byte identical to vec2, but it
+                // is value-stable: parsing it again reproduces j2 exactly.
+                std::vector<std::uint8_t> const vec3 = json::to_bjdata(j2, false, false);
+                CHECK(json::from_bjdata(vec3) == j2);
+            }
+
             SECTION("ndarray whose dimensions overflow stays as object")
             {
                 // the product of the dimensions wraps around std::size_t to 0
@@ -2823,6 +2915,36 @@ TEST_CASE("BJData")
                 CHECK(out_single_ok.at(0) == '[');
                 CHECK(json::from_bjdata(out_single_ok) == json({1.5f}));
             }
+
+            SECTION("ndarray with _ArrayType_ \"byte\" is gated by the BJData draft version")
+            {
+                // the 'B' (byte) marker used by _ArrayType_ "byte" is only defined
+                // by BJData Draft 3; Draft 2 (the default) has no such marker, so
+                // emitting it unconditionally produced a stream that a Draft 2
+                // reader could not parse as intended (see GitHub issue #5404).
+                // Two dimensions are used so that a successfully written ndarray
+                // round-trips back into the annotated object (a single dimension
+                // is, by the BJData ndarray convention, read back as a plain
+                // binary value rather than the annotated object, same as every
+                // other single-dimension ndarray of a non-"byte" type is read
+                // back as a plain array instead of the annotated object).
+                json const j_byte = json({{"_ArrayType_", "byte"}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}});
+
+                // default (Draft 2): falls back to a plain object and round-trips
+                const auto out_draft2 = json::to_bjdata(j_byte);
+                CHECK(out_draft2.at(0) == '{');
+                CHECK(json::from_bjdata(out_draft2) == j_byte);
+
+                // explicit Draft 2: same as the default
+                const auto out_draft2_explicit = json::to_bjdata(j_byte, true, true, json::bjdata_version_t::draft2);
+                CHECK(out_draft2_explicit.at(0) == '{');
+                CHECK(json::from_bjdata(out_draft2_explicit) == j_byte);
+
+                // Draft 3 explicitly selected: still uses the compact 'B' ndarray encoding
+                const auto out_draft3 = json::to_bjdata(j_byte, true, true, json::bjdata_version_t::draft3);
+                CHECK(out_draft3 == std::vector<uint8_t>({'[', '$', 'B', '#', '[', '$', 'i', '#', 'i', 2, 2, 3, 1, 2, 3, 4, 5, 6}));
+                CHECK(json::from_bjdata(out_draft3) == j_byte);
+            }
         }
     }