diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index aaa638801..4bd173257 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -1698,6 +1698,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() + if (!value.at(key).is_string()) + { + return true; + } + // use get() instead of static_cast to avoid an // ambiguous conversion under explicit instantiation on C++17 (see #4825) auto it = bjdtype.find(value.at(key).template get()); diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index 4253fcdcf..213236b51 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -20375,6 +20375,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() + if (!value.at(key).is_string()) + { + return true; + } + // use get() instead of static_cast to avoid an // ambiguous conversion under explicit instantiation on C++17 (see #4825) auto it = bjdtype.find(value.at(key).template get()); diff --git a/tests/src/fuzzer-parse_bjdata.cpp b/tests/src/fuzzer-parse_bjdata.cpp index 1d1d56a5c..a88479933 100644 --- a/tests/src/fuzzer-parse_bjdata.cpp +++ b/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&) { diff --git a/tests/src/unit-bjdata.cpp b/tests/src/unit-bjdata.cpp index a53bd17ce..334259fb7 100644 --- a/tests/src/unit-bjdata.cpp +++ b/tests/src/unit-bjdata.cpp @@ -2746,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() 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 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 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 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