Keep BJData ndarray annotations that would not survive a round trip as objects

write_bjdata_ndarray() encoded a JData-annotated object as a BJData
ND-array whenever its dimensions' product matched _ArrayData_.size(),
which lost information in two ways:

- _ArrayData_ was never required to be an array. null has size 0, any
  other scalar has size 1, and iterating an object visits its values, so
  e.g. {"_ArraySize_":[1],"_ArrayData_":5} was written as the array [5],
  and an object _ArrayData_ came back as an array.

- The reader only restores an annotated object from an ND-array with at
  least two non-zero dimensions that is not a 1xN row vector; an empty,
  1-D, row-vector, or zero-sized shape is read back as a plain array. The
  writer nonetheless emitted ND-array headers for these shapes, so the
  annotation was silently dropped.

OSS-Fuzz issue 563659413 hit this in parse_bjdata_fuzzer: an empty binary
_ArraySize_ is written as a plain object and read back as an empty array,
after which {"_ArrayType_":"int16","_ArraySize_":[],"_ArrayData_":null}
was encoded as the ND-array header "[$I#[]" and re-read as [], failing the
harness's value-stability check.

Such objects now fall back to a plain object encoding, which round-trips.
Genuine ND-arrays (two or more positive dimensions, not a 1xN row vector)
are encoded exactly as before. Existing fallback tests that used 1-D
shapes are moved to 2-D shapes so they keep exercising the check they
were written for, and the BJData documentation is updated.

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

docs/mkdocs/docs/features/binary_formats/bjdata.md

@@ -116,18 +116,22 @@ The library uses the following mapping from JSON values types to BJData types ac
     ```
 
     Likewise, when a JSON object in the above form is serialized using
-    [`to_bjdata`](../../api/basic_json/to_bjdata.md), it is automatically converted into a compact BJData ND-array. When
-    the 1-dimensional vector stored in `"_ArraySize_"` contains a single integer or two integers with one being 1, a
-    regular 1-D optimized array is generated instead.
+    [`to_bjdata`](../../api/basic_json/to_bjdata.md), it is automatically converted into a compact BJData ND-array.
 
-    An object is only converted if the annotation actually describes a packed array; otherwise it is serialized as a
-    regular JSON object. This requires all of the following:
+    When parsing, an ND-array whose dimension vector is empty, contains a single integer, contains two integers with the
+    first being 1, or contains a 0 is returned as a regular (possibly empty) array rather than an annotated object.
+
+    An object is only converted if the annotation describes a packed array that is parsed back into the same annotated
+    object; otherwise it is serialized as a regular JSON object, so the annotation is never lost in a round trip. This requires
+    all of the following:
 
     - `"_ArrayType_"` is one of `uint8`, `int8`, `uint16`, `int16`, `uint32`, `int32`, `uint64`, `int64`, `single`,
       `double`, `char`, or `byte`,
     - `"_ArraySize_"` is an array, since the dimensions are written as the ND-array header's length,
-    - every entry of `"_ArraySize_"` is a non-negative integer, and their product is representable as a `std::size_t`,
-    - `"_ArrayData_"` holds exactly that many elements, and
+    - `"_ArraySize_"` has at least two entries and is not a 1×N row vector (first entry 1), since other shapes are
+      parsed back as a regular array,
+    - every entry of `"_ArraySize_"` is a positive integer, and their product is representable as a `std::size_t`,
+    - `"_ArrayData_"` is an array holding exactly that many elements, and
     - every element of `"_ArrayData_"` is a number of the kind named by `"_ArrayType_"` (a floating-point number for
       `single` and `double`, an integer otherwise).
 

include/nlohmann/detail/output/binary_writer.hpp

@@ -1800,8 +1800,19 @@ class binary_writer
             return true;
         }
 
-        std::size_t len = (value.at(key).empty() ? 0 : 1);
-        for (const auto& el : value.at(key))
+        // the reader only restores an annotated object from an ND-array header
+        // with at least two dimensions: an empty dimension vector, a single
+        // dimension, or a 1xN row vector is read back as a plain array, which
+        // would silently drop the annotation, so such an object falls back to
+        // a plain object encoding instead
+        const auto& dims = value.at(key);
+        if (dims.size() < 2 || (dims.size() == 2 && dims.at(0).is_number_integer() && dims.at(0).template get<std::int64_t>() == 1))
+        {
+            return true;
+        }
+
+        std::size_t len = 1;
+        for (const auto& el : dims)
         {
             // a dimension is read as an unsigned value below, so anything that
             // is not a non-negative integer is rejected: a non-integer entry
@@ -1823,15 +1834,26 @@ class binary_writer
                 return true;
             }
             const auto dim_size = static_cast<std::size_t>(dim);
-            if (dim_size != 0 && len > (std::numeric_limits<std::size_t>::max)() / dim_size)
+
+            // the reader turns an ND-array with any zero dimension into an
+            // empty plain array, dropping the annotation, so keep the object
+            if (dim_size == 0)
+            {
+                return true;
+            }
+            if (len > (std::numeric_limits<std::size_t>::max)() / dim_size)
             {
                 return true;
             }
             len *= dim_size;
         }
 
+        // the elements are written from _ArrayData_ as a flat list, so it has
+        // to be an array: size() is 0 for null and 1 for any other scalar, and
+        // iterating an object visits its values, so any of these could match
+        // the dimensions by accident and be encoded as an unrelated ND-array
         key = "_ArrayData_";
-        if (value.at(key).size() != len)
+        if (!value.at(key).is_array() || value.at(key).size() != len)
         {
             return true;
         }

single_include/nlohmann/json.hpp

@@ -20629,8 +20629,19 @@ class binary_writer
             return true;
         }
 
-        std::size_t len = (value.at(key).empty() ? 0 : 1);
-        for (const auto& el : value.at(key))
+        // the reader only restores an annotated object from an ND-array header
+        // with at least two dimensions: an empty dimension vector, a single
+        // dimension, or a 1xN row vector is read back as a plain array, which
+        // would silently drop the annotation, so such an object falls back to
+        // a plain object encoding instead
+        const auto& dims = value.at(key);
+        if (dims.size() < 2 || (dims.size() == 2 && dims.at(0).is_number_integer() && dims.at(0).template get<std::int64_t>() == 1))
+        {
+            return true;
+        }
+
+        std::size_t len = 1;
+        for (const auto& el : dims)
         {
             // a dimension is read as an unsigned value below, so anything that
             // is not a non-negative integer is rejected: a non-integer entry
@@ -20652,15 +20663,26 @@ class binary_writer
                 return true;
             }
             const auto dim_size = static_cast<std::size_t>(dim);
-            if (dim_size != 0 && len > (std::numeric_limits<std::size_t>::max)() / dim_size)
+
+            // the reader turns an ND-array with any zero dimension into an
+            // empty plain array, dropping the annotation, so keep the object
+            if (dim_size == 0)
+            {
+                return true;
+            }
+            if (len > (std::numeric_limits<std::size_t>::max)() / dim_size)
             {
                 return true;
             }
             len *= dim_size;
         }
 
+        // the elements are written from _ArrayData_ as a flat list, so it has
+        // to be an array: size() is 0 for null and 1 for any other scalar, and
+        // iterating an object visits its values, so any of these could match
+        // the dimensions by accident and be encoded as an unrelated ND-array
         key = "_ArrayData_";
-        if (value.at(key).size() != len)
+        if (!value.at(key).is_array() || value.at(key).size() != len)
         {
             return true;
         }

tests/src/unit-bjdata.cpp

@@ -2604,25 +2604,25 @@ TEST_CASE("BJData")
                 // that still round-trips.
 
                 // string data declared as a uint64 array
-                json const j_str = json({{"_ArrayType_", "uint64"}, {"_ArraySize_", {1}}, {"_ArrayData_", {"pointer"}}});
+                json const j_str = json({{"_ArrayType_", "uint64"}, {"_ArraySize_", {2, 1}}, {"_ArrayData_", {"pointer", "value"}}});
                 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}}});
+                json const j_float = json({{"_ArrayType_", "double"}, {"_ArraySize_", {2, 1}}, {"_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}}});
+                json const j_size = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {"x", 1}}, {"_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]})");
+                json const j_neg = json::parse(R"({"_ArrayType_":"uint8","_ArraySize_":[-1,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);
@@ -2657,14 +2657,14 @@ TEST_CASE("BJData")
                 }
 
                 // 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]})"));
+                const auto from_neg = json::to_bjdata(json::parse(R"({"_ArrayType_":"int32","_ArraySize_":[2,1],"_ArrayData_":[-5,7]})"));
                 CHECK(from_neg.at(0) == '[');
-                CHECK(from_neg == json::to_bjdata(json({{"_ArrayType_", "int32"}, {"_ArraySize_", {2}}, {"_ArrayData_", {-5, 7}}})));
+                CHECK(from_neg == json::to_bjdata(json({{"_ArrayType_", "int32"}, {"_ArraySize_", {2, 1}}, {"_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]})"));
+                const auto from_float = json::to_bjdata(json::parse(R"({"_ArrayType_":"double","_ArraySize_":[2,1],"_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}}})));
+                CHECK(from_float == json::to_bjdata(json({{"_ArrayType_", "double"}, {"_ArraySize_", {2, 1}}, {"_ArrayData_", {1.5, 2.5}}})));
             }
 
             SECTION("optimized ndarray (type and vector-size as 1D array)")
@@ -2835,7 +2835,7 @@ TEST_CASE("BJData")
                 // a single dimension that does not fit into std::size_t is
                 // rejected for the same reason (only observable where
                 // std::size_t is narrower than 64 bit)
-                json j_huge = json({{"_ArrayData_", json::array()}, {"_ArraySize_", {18446744073709551615ull}}, {"_ArrayType_", "uint8"}});
+                json j_huge = json({{"_ArrayData_", json::array()}, {"_ArraySize_", {18446744073709551615ull, 2}}, {"_ArrayType_", "uint8"}});
                 CHECK(json::from_bjdata(json::to_bjdata(j_huge), true, true) == j_huge);
 
                 // a well-formed ndarray is still encoded as one
@@ -2878,42 +2878,116 @@ TEST_CASE("BJData")
                 // 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}}});
+                json const j_uint8 = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2, 1}}, {"_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}}});
+                json const j_int8 = json({{"_ArrayType_", "int8"}, {"_ArraySize_", {2, 1}}, {"_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}}});
+                json const j_uint16_neg = json({{"_ArrayType_", "uint16"}, {"_ArraySize_", {2, 1}}, {"_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}}});
+                json const j_single = json({{"_ArrayType_", "single"}, {"_ArraySize_", {2, 1}}, {"_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_uint8_ok = json({{"_ArrayType_", "uint8"}, {"_ArraySize_", {2, 1}}, {"_ArrayData_", {0, 255}}});
+                CHECK(json::to_bjdata(j_uint8_ok) == std::vector<uint8_t>({'[', '$', 'U', '#', '[', 'i', 2, 'i', 1, ']', 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_int8_ok = json({{"_ArrayType_", "int8"}, {"_ArraySize_", {2, 1}}, {"_ArrayData_", {-128, 127}}});
+                CHECK(json::to_bjdata(j_int8_ok) == std::vector<uint8_t>({'[', '$', 'i', '#', '[', 'i', 2, 'i', 1, ']', 0x80, 0x7F}));
 
-                json const j_single_ok = json({{"_ArrayType_", "single"}, {"_ArraySize_", {1}}, {"_ArrayData_", {1.5}}});
+                json const j_single_ok = json({{"_ArrayType_", "single"}, {"_ArraySize_", {2, 1}}, {"_ArrayData_", {1.5, -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}));
+                CHECK(json::from_bjdata(out_single_ok) == json({{"_ArrayType_", "single"}, {"_ArraySize_", {2, 1}}, {"_ArrayData_", {1.5f, -1.5f}}}));
+            }
+
+            SECTION("ndarray that would not be read back as an annotated object stays as object")
+            {
+                // the reader only restores an annotated object from an ND-array
+                // with at least two non-zero dimensions that is not a 1xN row
+                // vector; any other shape is read back as a plain array. Writing
+                // such an object as an ND-array would drop its annotation, so it
+                // falls back to a plain object encoding that round-trips.
+                for (const char* text :
+                        {
+                            R"({"_ArrayType_":"int16","_ArraySize_":[],"_ArrayData_":[]})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[2],"_ArrayData_":[1,2]})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[1,2],"_ArrayData_":[1,2]})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[0],"_ArrayData_":[]})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[2,0],"_ArrayData_":[]})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[0,2],"_ArrayData_":[]})"
+                        })
+                {
+                    CAPTURE(text);
+                    const json j = json::parse(text);
+                    for (const bool use_size :
+                            {
+                                false, true
+                            })
+                    {
+                        const auto out = json::to_bjdata(j, use_size, use_size);
+                        CHECK(out.at(0) == '{');
+                        CHECK(json::from_bjdata(out) == j);
+                    }
+                }
+
+                // a genuine ND-array still uses the compact encoding and round-trips
+                const json j_2d = json::parse(R"({"_ArrayType_":"int16","_ArraySize_":[2,1],"_ArrayData_":[1,2]})");
+                const auto out_2d = json::to_bjdata(j_2d);
+                CHECK(out_2d.at(0) == '[');
+                CHECK(json::from_bjdata(out_2d) == j_2d);
+            }
+
+            SECTION("ndarray with non-array _ArrayData_ stays as object")
+            {
+                // the elements are written from _ArrayData_ as a flat list, so it
+                // has to be an array: null has size 0, any other scalar has size 1,
+                // and iterating an object visits its values, so each of these could
+                // match the dimensions and be encoded as an unrelated ND-array
+                for (const char* text :
+                        {
+                            R"({"_ArrayType_":"int16","_ArraySize_":[2,1],"_ArrayData_":null})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[2,1],"_ArrayData_":{"a":1,"b":2}})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[1],"_ArrayData_":5})",
+                            R"({"_ArrayType_":"int16","_ArraySize_":[],"_ArrayData_":null})"
+                        })
+                {
+                    CAPTURE(text);
+                    const json j = json::parse(text);
+                    const auto out = json::to_bjdata(j);
+                    CHECK(out.at(0) == '{');
+                    CHECK(json::from_bjdata(out) == j);
+                }
+
+                // OSS-Fuzz issue 563659413: an empty binary _ArraySize_ is written
+                // as a plain object and read back as an empty array, after which
+                // the object with a null _ArrayData_ was encoded as an empty
+                // ND-array and re-read as [], so a second round trip lost the value
+                const std::vector<uint8_t> input =
+                {
+                    '{', 'U', 11, '_', 'A', 'r', 'r', 'a', 'y', 'D', 'a', 't', 'a', '_', 'Z',
+                    'U', 11, '_', 'A', 'r', 'r', 'a', 'y', 'T', 'y', 'p', 'e', '_', 'S', 'i', 5, 'i', 'n', 't', '1', '6',
+                    'U', 11, '_', 'A', 'r', 'r', 'a', 'y', 'S', 'i', 'z', 'e', '_', '[', '$', 'B', '#', '[', ']', '}'
+                };
+                const json j1 = json::from_bjdata(input);
+                const json j2 = json::from_bjdata(json::to_bjdata(j1, false, false));
+                CHECK(j2 == json::parse(R"({"_ArrayType_":"int16","_ArraySize_":[],"_ArrayData_":null})"));
+                CHECK(json::from_bjdata(json::to_bjdata(j2, false, false)) == j2);
             }
 
             SECTION("ndarray with _ArrayType_ \"byte\" is gated by the BJData draft version")