From fd72ecfc8cd83b891ef084a9e99aa89bca57fac9 Mon Sep 17 00:00:00 2001 From: Angadi56 Date: Fri, 31 Jul 2026 12:22:27 +0530 Subject: [PATCH] validate ndarray element types in write_bjdata_ndarray (#5301) * validate ndarray element types in write_bjdata_ndarray Signed-off-by: Angadi Yashaswini * read ndarray elements through get<> instead of a fixed union member _ArrayType_ names the wire type, not how the value is stored: parsing keeps a non-negative integer as number_unsigned while the C++ API keeps an int literal as number_integer. Selecting the union member from the type marker therefore reads the inactive alternative for one of the two, so read through get<> instead, which dispatches on the active member. Also reject a negative _ArraySize_ entry, which is not a usable dimension, and cover the parse-built path in the tests. Signed-off-by: Angadi Yashaswini --------- Signed-off-by: Angadi Yashaswini --- .../nlohmann/detail/output/binary_writer.hpp | 48 ++++++++++---- single_include/nlohmann/json.hpp | 48 ++++++++++---- tests/src/unit-bjdata.cpp | 63 +++++++++++++++++++ 3 files changed, 137 insertions(+), 22 deletions(-) diff --git a/include/nlohmann/detail/output/binary_writer.hpp b/include/nlohmann/detail/output/binary_writer.hpp index 2d79c193f..9a8b53df7 100644 --- a/include/nlohmann/detail/output/binary_writer.hpp +++ b/include/nlohmann/detail/output/binary_writer.hpp @@ -1662,7 +1662,15 @@ class binary_writer std::size_t len = (value.at(key).empty() ? 0 : 1); for (const auto& el : value.at(key)) { - len *= static_cast(el.m_data.m_value.number_unsigned); + // a dimension is read as an unsigned value below, so anything that + // is not a non-negative integer is rejected: a non-integer entry + // would pun unrelated bytes as the dimension, and a negative one + // would wrap into a nonsensical length + if (!el.is_number_integer() || (!el.is_number_unsigned() && el.template get() < 0)) + { + return true; + } + len *= static_cast(el.template get()); } key = "_ArrayData_"; @@ -1671,6 +1679,24 @@ class binary_writer return true; } + // every element is written below as the number kind dtype names, so it + // has to actually be a number of that category: an element of any other + // type would reinterpret unrelated bytes, e.g. a string's heap pointer, + // as that number. Such an object falls back to a plain object encoding. + // dtype names the wire type, not the storage type: whether an integer + // is held as number_integer or number_unsigned depends on how the value + // was built (parsing stores non-negative integers as unsigned, the C++ + // API stores int literals as signed), so both are accepted here and the + // writes below go through get<>, which reads the member that is active. + const bool ndarray_is_float = (dtype == 'd' || dtype == 'D'); + for (const auto& el : value.at(key)) + { + if (ndarray_is_float ? !el.is_number_float() : !el.is_number_integer()) + { + return true; + } + } + oa->write_character('['); oa->write_character('$'); oa->write_character(dtype); @@ -1684,70 +1710,70 @@ class binary_writer { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'i') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'u') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'I') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'm') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'l') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'M') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(el.template get(), true); } } else if (dtype == 'L') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(el.template get(), true); } } else if (dtype == 'd') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_float), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'D') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_float), true); + write_number(el.template get(), true); } } return false; diff --git a/single_include/nlohmann/json.hpp b/single_include/nlohmann/json.hpp index b580130c3..ec2651390 100644 --- a/single_include/nlohmann/json.hpp +++ b/single_include/nlohmann/json.hpp @@ -18457,7 +18457,15 @@ class binary_writer std::size_t len = (value.at(key).empty() ? 0 : 1); for (const auto& el : value.at(key)) { - len *= static_cast(el.m_data.m_value.number_unsigned); + // a dimension is read as an unsigned value below, so anything that + // is not a non-negative integer is rejected: a non-integer entry + // would pun unrelated bytes as the dimension, and a negative one + // would wrap into a nonsensical length + if (!el.is_number_integer() || (!el.is_number_unsigned() && el.template get() < 0)) + { + return true; + } + len *= static_cast(el.template get()); } key = "_ArrayData_"; @@ -18466,6 +18474,24 @@ class binary_writer return true; } + // every element is written below as the number kind dtype names, so it + // has to actually be a number of that category: an element of any other + // type would reinterpret unrelated bytes, e.g. a string's heap pointer, + // as that number. Such an object falls back to a plain object encoding. + // dtype names the wire type, not the storage type: whether an integer + // is held as number_integer or number_unsigned depends on how the value + // was built (parsing stores non-negative integers as unsigned, the C++ + // API stores int literals as signed), so both are accepted here and the + // writes below go through get<>, which reads the member that is active. + const bool ndarray_is_float = (dtype == 'd' || dtype == 'D'); + for (const auto& el : value.at(key)) + { + if (ndarray_is_float ? !el.is_number_float() : !el.is_number_integer()) + { + return true; + } + } + oa->write_character('['); oa->write_character('$'); oa->write_character(dtype); @@ -18479,70 +18505,70 @@ class binary_writer { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'i') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'u') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'I') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'm') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'l') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'M') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_unsigned), true); + write_number(el.template get(), true); } } else if (dtype == 'L') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_integer), true); + write_number(el.template get(), true); } } else if (dtype == 'd') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_float), true); + write_number(static_cast(el.template get()), true); } } else if (dtype == 'D') { for (const auto& el : value.at(key)) { - write_number(static_cast(el.m_data.m_value.number_float), true); + write_number(el.template get(), true); } } return false; diff --git a/tests/src/unit-bjdata.cpp b/tests/src/unit-bjdata.cpp index a60d0a164..4334f32d5 100644 --- a/tests/src/unit-bjdata.cpp +++ b/tests/src/unit-bjdata.cpp @@ -2587,6 +2587,69 @@ TEST_CASE("BJData") CHECK(json::to_bjdata(json::from_bjdata(v_B), true, true) == v_B); } + SECTION("ndarray with data not matching _ArrayType_ is written as an object") + { + // A JData-annotated object is only serialized as an ndarray when + // its _ArrayData_ elements are actually stored as the number kind + // named by _ArrayType_. Otherwise the writer would read the wrong + // union member (e.g. a std::string's heap pointer as a uint64) and + // emit it, so such an object falls back to a plain object encoding + // that still round-trips. + + // string data declared as a uint64 array + json const j_str = json({{"_ArrayType_", "uint64"}, {"_ArraySize_", {1}}, {"_ArrayData_", {"pointer"}}}); + const auto out_str = json::to_bjdata(j_str); + CHECK(out_str.at(0) == '{'); + CHECK(json::from_bjdata(out_str) == j_str); + + // integer data declared as a double array + json const j_float = json({{"_ArrayType_", "double"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 2}}}); + const auto out_float = json::to_bjdata(j_float); + CHECK(out_float.at(0) == '{'); + CHECK(json::from_bjdata(out_float) == j_float); + + // a non-integer shape entry is likewise not treated as an ndarray + json const j_size = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {"x"}}, {"_ArrayData_", {1}}}); + const auto out_size = json::to_bjdata(j_size); + CHECK(out_size.at(0) == '{'); + CHECK(json::from_bjdata(out_size) == j_size); + + // a negative shape entry is not a usable dimension either + json const j_neg = json::parse(R"({"_ArrayType_":"uint8","_ArraySize_":[-1],"_ArrayData_":[1]})"); + const auto out_neg = json::to_bjdata(j_neg); + CHECK(out_neg.at(0) == '{'); + CHECK(json::from_bjdata(out_neg) == j_neg); + } + + SECTION("ndarray parsed from text is written as a typed array") + { + // json::parse stores a non-negative integer as number_unsigned while + // 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. + for (const char* type : + {"uint8", "int8", "uint16", "int16", "uint32", "int32", "uint64", "int64", "char", "byte" + }) + { + CAPTURE(type); + const std::string text = std::string(R"({"_ArrayType_":")") + type + + R"(","_ArraySize_":[2,3],"_ArrayData_":[1,2,3,4,5,6]})"; + const auto from_text = json::to_bjdata(json::parse(text)); + CHECK(from_text.at(0) == '['); + CHECK(from_text == json::to_bjdata(json({{"_ArrayType_", type}, {"_ArraySize_", {2, 3}}, {"_ArrayData_", {1, 2, 3, 4, 5, 6}}}))); + } + + // 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) == '['); + CHECK(from_neg == json::to_bjdata(json({{"_ArrayType_", "int32"}, {"_ArraySize_", {2}}, {"_ArrayData_", {-5, 7}}}))); + + // and so do the floating point types + const auto from_float = json::to_bjdata(json::parse(R"({"_ArrayType_":"double","_ArraySize_":[2],"_ArrayData_":[1.5,2.5]})")); + CHECK(from_float.at(0) == '['); + CHECK(from_float == json::to_bjdata(json({{"_ArrayType_", "double"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1.5, 2.5}}}))); + } + SECTION("optimized ndarray (type and vector-size as 1D array)") { // create vector with two elements of the same type