Compare commits

..
Author SHA1 Message Date
Niels Lohmann d2c2db92a9 Fix to_bjdata() silently truncating out-of-range _ArrayData_ elements
write_bjdata_ndarray() validated that each _ArrayData_ element matched
the number kind (integer vs. float) named by _ArrayType_, but not its
range. An element that did not fit the target C++ type (e.g. 256 for
"uint8") was silently wrapped by the static_cast used to write it, or,
for "single", silently overflowed to infinity.

Range-check each element against the type named by _ArrayType_ before
writing it, reusing the existing fallback path that already encodes
the annotated object as a plain object for other invalid-annotation
cases in this function.

Fixes #5403.

Signed-off-by: Niels Lohmann <mail@nlohmann.me>
2026-09-05 20:43:55 +02:00
6 changed files with 185 additions and 93 deletions
+2 -6
View File
@@ -34,14 +34,10 @@ void swap(typename binary_t::container_type& other);
```
1. Exchanges the contents of the JSON value with those of `other`. Does not invoke any move, copy, or swap operations on
individual elements. All iterators and references remain valid. The past-the-end iterator is invalidated. If macro
[`JSON_DIAGNOSTIC_POSITIONS`](../macros/json_diagnostic_positions.md) is defined to `#!cpp 1`, the
[`start_pos()`](start_pos.md)/[`end_pos()`](end_pos.md) diagnostic positions are exchanged along with the value.
individual elements. All iterators and references remain valid. The past-the-end iterator is invalidated.
2. Exchanges the contents of the JSON value from `left` with those of `right`. Does not invoke any move, copy, or swap
operations on individual elements. All iterators and references remain valid. The past-the-end iterator is
invalidated. Implemented as a friend function callable via ADL. If macro
[`JSON_DIAGNOSTIC_POSITIONS`](../macros/json_diagnostic_positions.md) is defined to `#!cpp 1`, the
[`start_pos()`](start_pos.md)/[`end_pos()`](end_pos.md) diagnostic positions are exchanged along with the value.
invalidated. Implemented as a friend function callable via ADL.
3. Exchanges the contents of a JSON array with those of `other`. Does not invoke any move, copy, or swap operations on
individual elements. All iterators and references remain valid. The past-the-end iterator is invalidated.
4. Exchanges the contents of a JSON object with those of `other`. Does not invoke any move, copy, or swap operations on
@@ -1647,6 +1647,20 @@ class binary_writer
return 'D'; // float 64
}
/*!
@brief checks whether a JSON number fits into @a TargetType
@param[in] el a JSON number of either the signed or unsigned integer kind
@return whether @a el's value can be represented by @a TargetType without
wrapping, regardless of which of the two kinds it is stored as
*/
template<typename TargetType>
static bool bjdata_ndarray_value_in_range(const BasicJsonType& el)
{
return el.is_number_unsigned()
? value_in_range_of<TargetType>(el.template get<std::uint64_t>())
: value_in_range_of<TargetType>(el.template get<std::int64_t>());
}
/*!
@return false if the object is successfully converted to a bjdata ndarray, true if the type or size is invalid
*/
@@ -1731,6 +1745,60 @@ class binary_writer
}
}
// every element is cast to the (possibly narrower) C++ type matching
// dtype below; a value that does not fit that type would silently
// wrap (integers) or overflow to infinity (the "single" precision
// float) instead of being reported, so such an object falls back to
// a plain object encoding as well
for (const auto& el : value.at(key))
{
bool in_range = true;
switch (dtype)
{
case 'U':
case 'C':
case 'B':
in_range = bjdata_ndarray_value_in_range<std::uint8_t>(el);
break;
case 'i':
in_range = bjdata_ndarray_value_in_range<std::int8_t>(el);
break;
case 'u':
in_range = bjdata_ndarray_value_in_range<std::uint16_t>(el);
break;
case 'I':
in_range = bjdata_ndarray_value_in_range<std::int16_t>(el);
break;
case 'm':
in_range = bjdata_ndarray_value_in_range<std::uint32_t>(el);
break;
case 'l':
in_range = bjdata_ndarray_value_in_range<std::int32_t>(el);
break;
case 'M':
in_range = bjdata_ndarray_value_in_range<std::uint64_t>(el);
break;
case 'L':
in_range = bjdata_ndarray_value_in_range<std::int64_t>(el);
break;
case 'd':
{
const auto dval = el.template get<double>();
in_range = !std::isfinite(dval) ||
(dval >= static_cast<double>(std::numeric_limits<float>::lowest()) &&
dval <= static_cast<double>((std::numeric_limits<float>::max)()));
break;
}
default:
// 'D' (double) already spans the full range of number_float_t
break;
}
if (!in_range)
{
return true;
}
}
oa->write_character('[');
oa->write_character('$');
oa->write_character(dtype);
-5
View File
@@ -3547,11 +3547,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
std::swap(m_data.m_type, other.m_data.m_type);
std::swap(m_data.m_value, other.m_data.m_value);
#if JSON_DIAGNOSTIC_POSITIONS
std::swap(start_position, other.start_position);
std::swap(end_position, other.end_position);
#endif
set_parents();
other.set_parents();
assert_invariant();
+68 -5
View File
@@ -18655,6 +18655,20 @@ class binary_writer
return 'D'; // float 64
}
/*!
@brief checks whether a JSON number fits into @a TargetType
@param[in] el a JSON number of either the signed or unsigned integer kind
@return whether @a el's value can be represented by @a TargetType without
wrapping, regardless of which of the two kinds it is stored as
*/
template<typename TargetType>
static bool bjdata_ndarray_value_in_range(const BasicJsonType& el)
{
return el.is_number_unsigned()
? value_in_range_of<TargetType>(el.template get<std::uint64_t>())
: value_in_range_of<TargetType>(el.template get<std::int64_t>());
}
/*!
@return false if the object is successfully converted to a bjdata ndarray, true if the type or size is invalid
*/
@@ -18739,6 +18753,60 @@ class binary_writer
}
}
// every element is cast to the (possibly narrower) C++ type matching
// dtype below; a value that does not fit that type would silently
// wrap (integers) or overflow to infinity (the "single" precision
// float) instead of being reported, so such an object falls back to
// a plain object encoding as well
for (const auto& el : value.at(key))
{
bool in_range = true;
switch (dtype)
{
case 'U':
case 'C':
case 'B':
in_range = bjdata_ndarray_value_in_range<std::uint8_t>(el);
break;
case 'i':
in_range = bjdata_ndarray_value_in_range<std::int8_t>(el);
break;
case 'u':
in_range = bjdata_ndarray_value_in_range<std::uint16_t>(el);
break;
case 'I':
in_range = bjdata_ndarray_value_in_range<std::int16_t>(el);
break;
case 'm':
in_range = bjdata_ndarray_value_in_range<std::uint32_t>(el);
break;
case 'l':
in_range = bjdata_ndarray_value_in_range<std::int32_t>(el);
break;
case 'M':
in_range = bjdata_ndarray_value_in_range<std::uint64_t>(el);
break;
case 'L':
in_range = bjdata_ndarray_value_in_range<std::int64_t>(el);
break;
case 'd':
{
const auto dval = el.template get<double>();
in_range = !std::isfinite(dval) ||
(dval >= static_cast<double>(std::numeric_limits<float>::lowest()) &&
dval <= static_cast<double>((std::numeric_limits<float>::max)()));
break;
}
default:
// 'D' (double) already spans the full range of number_float_t
break;
}
if (!in_range)
{
return true;
}
}
oa->write_character('[');
oa->write_character('$');
oa->write_character(dtype);
@@ -24975,11 +25043,6 @@ class basic_json // NOLINT(cppcoreguidelines-special-member-functions,hicpp-spec
std::swap(m_data.m_type, other.m_data.m_type);
std::swap(m_data.m_value, other.m_data.m_value);
#if JSON_DIAGNOSTIC_POSITIONS
std::swap(start_position, other.start_position);
std::swap(end_position, other.end_position);
#endif
set_parents();
other.set_parents();
assert_invariant();
+47
View File
@@ -2776,6 +2776,53 @@ TEST_CASE("BJData")
CHECK(out_num.at(0) == '{');
CHECK(json::from_bjdata(out_num) == j_num);
}
SECTION("ndarray with out-of-range _ArrayData_ elements stays as object")
{
// each element is cast to the (possibly narrower) C++ type
// named by _ArrayType_ before being written; a value that
// does not fit that type would silently wrap instead of
// being reported, so such an object falls back to a plain
// object encoding that still round-trips (see GitHub issue #5403)
// an unsigned element that does not fit uint8
json const j_uint8 = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 256}}});
const auto out_uint8 = json::to_bjdata(j_uint8);
CHECK(out_uint8.at(0) == '{');
CHECK(json::from_bjdata(out_uint8) == j_uint8);
// a signed element that does not fit int8
json const j_int8 = json({{"_ArrayType_", "int8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, 200}}});
const auto out_int8 = json::to_bjdata(j_int8);
CHECK(out_int8.at(0) == '{');
CHECK(json::from_bjdata(out_int8) == j_int8);
// a negative element is likewise out of range for an
// unsigned _ArrayType_
json const j_uint16_neg = json({{"_ArrayType_", "uint16"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1, -1}}});
const auto out_uint16_neg = json::to_bjdata(j_uint16_neg);
CHECK(out_uint16_neg.at(0) == '{');
CHECK(json::from_bjdata(out_uint16_neg) == j_uint16_neg);
// a double element that overflows to infinity when narrowed
// to the "single" (float) precision named by _ArrayType_
json const j_single = json({{"_ArrayType_", "single"}, {"_ArraySize_", {2}}, {"_ArrayData_", {1.5, 1e40}}});
const auto out_single = json::to_bjdata(j_single);
CHECK(out_single.at(0) == '{');
CHECK(json::from_bjdata(out_single) == j_single);
// in-range boundary values still use the compact ndarray encoding
json const j_uint8_ok = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {0, 255}}});
CHECK(json::to_bjdata(j_uint8_ok) == std::vector<uint8_t>({'[', '$', 'U', '#', '[', 'i', 2, ']', 0, 255}));
json const j_int8_ok = json({{"_ArrayType_", "int8"}, {"_ArraySize_", {2}}, {"_ArrayData_", {-128, 127}}});
CHECK(json::to_bjdata(j_int8_ok) == std::vector<uint8_t>({'[', '$', 'i', '#', '[', 'i', 2, ']', 0x80, 0x7F}));
json const j_single_ok = json({{"_ArrayType_", "single"}, {"_ArraySize_", {1}}, {"_ArrayData_", {1.5}}});
const auto out_single_ok = json::to_bjdata(j_single_ok);
CHECK(out_single_ok.at(0) == '[');
CHECK(json::from_bjdata(out_single_ok) == json({1.5f}));
}
}
}
@@ -1955,80 +1955,3 @@ TEST_CASE("parser class")
}
}
}
TEST_CASE("diagnostic positions: value lifetime")
{
SECTION("copy constructor copies positions, recursively")
{
const std::string s = R"({"a":1,"b":[1,2,3]})";
const json a = json::parse(s);
const json b = a; // NOLINT(performance-unnecessary-copy-initialization)
CHECK(b.start_pos() == a.start_pos());
CHECK(b.end_pos() == a.end_pos());
CHECK(b["b"].start_pos() == a["b"].start_pos());
CHECK(b["b"].end_pos() == a["b"].end_pos());
}
SECTION("move constructor resets the moved-from value to npos")
{
const std::string s = R"({"a":1,"b":[1,2,3]})";
json a = json::parse(s);
const auto a_start = a.start_pos();
const auto a_end = a.end_pos();
const json b(std::move(a));
CHECK(b.start_pos() == a_start);
CHECK(b.end_pos() == a_end);
CHECK(a.start_pos() == std::string::npos); // NOLINT(bugprone-use-after-move,clang-analyzer-cplusplus.Move)
CHECK(a.end_pos() == std::string::npos); // NOLINT(bugprone-use-after-move,clang-analyzer-cplusplus.Move)
}
SECTION("swap() exchanges positions along with the values")
{
// basic_json::swap() (and the friend swap() that forwards to it) used
// to swap only m_data.m_type/m_data.m_value, leaving
// start_position/end_position untouched -- unlike copy-assignment's
// operator=(basic_json), which swaps positions as part of its
// copy-and-swap implementation. After swap(a, b), each value ended up
// with the *other* value's content but its *own* original position.
// This is now fixed so that swap() is consistent with copy-assignment.
json a = json::parse(R"({"a":1})");
json b = json::parse(R"([1,2,3,4,5])");
const auto a_start = a.start_pos();
const auto a_end = a.end_pos();
const auto b_start = b.start_pos();
const auto b_end = b.end_pos();
// lengths (and thus end positions) differ, which is enough to tell
// after the swap whether positions actually moved with the values
CHECK(a_end != b_end);
using std::swap;
swap(a, b);
CHECK(a == json::parse(R"([1,2,3,4,5])"));
CHECK(b == json::parse(R"({"a":1})"));
CHECK(a.start_pos() == b_start);
CHECK(a.end_pos() == b_end);
CHECK(b.start_pos() == a_start);
CHECK(b.end_pos() == a_end);
// member swap() behaves the same as the free function
json c = json::parse(R"({"a":1})");
json d = json::parse(R"([1,2,3,4,5])");
const auto c_start = c.start_pos();
const auto c_end = c.end_pos();
const auto d_start = d.start_pos();
const auto d_end = d.end_pos();
c.swap(d);
CHECK(c.start_pos() == d_start);
CHECK(c.end_pos() == d_end);
CHECK(d.start_pos() == c_start);
CHECK(d.end_pos() == c_end);
}
}