Fix to_msgpack() reading the inactive number union member

basic_json stores number_integer and number_unsigned in a union, and
number_unsigned_t only has to be at least as wide as number_integer_t
(with the default types, both are 64-bit and have the same
representation). When number_integer_t is narrower, write_msgpack()
read the wrong union member in two places:

- The number_unsigned case wrote number_integer's bits instead of
  number_unsigned's, silently writing the wrong value whenever it
  did not fit in number_integer_t.
- The number_integer case (non-negative branch) picked the encoded
  width by comparing number_unsigned's bits, which is undefined
  behavior, though the value written was still number_integer's, so
  at worst a too-wide encoding was chosen.

Read the active member in both cases, like the other binary writers
(CBOR, UBJSON, BJData, BSON, BON8) already do.

Fixes #5644.

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

docs/mkdocs/docs/api/basic_json/to_msgpack.md

@@ -76,3 +76,6 @@ Linear in the size of the JSON value `j`.
 
 - Added in version 2.0.9.
 - Throws `out_of_range.412` and `out_of_range.415` since version 3.13.0.
+- Fixed in version 3.13.0 to serialize `number_integer_t`/`number_unsigned_t` pairs of different width correctly;
+  before, integers could be serialized with the wrong value if `number_integer_t` was narrower than
+  `number_unsigned_t`.

include/nlohmann/detail/output/binary_writer.hpp

@@ -337,24 +337,24 @@ class binary_writer
                     // MessagePack does not differentiate between positive
                     // signed integers and unsigned integers. Therefore, we used
                     // the code from the value_t::number_unsigned case here.
-                    if (j.m_data.m_value.number_unsigned < 128)
+                    if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) < 128)
                     {
                         // positive fixnum
                         write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
                     }
-                    else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint8_t>::max)())
+                    else if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) <= (std::numeric_limits<std::uint8_t>::max)())
                     {
                         // uint 8
                         oa.write_character(to_char_type(0xCC));
                         write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
                     }
-                    else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint16_t>::max)())
+                    else if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) <= (std::numeric_limits<std::uint16_t>::max)())
                     {
                         // uint 16
                         oa.write_character(to_char_type(0xCD));
                         write_number(static_cast<std::uint16_t>(j.m_data.m_value.number_integer));
                     }
-                    else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint32_t>::max)())
+                    else if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) <= (std::numeric_limits<std::uint32_t>::max)())
                     {
                         // uint 32
                         oa.write_character(to_char_type(0xCE));
@@ -410,31 +410,31 @@ class binary_writer
                 if (j.m_data.m_value.number_unsigned < 128)
                 {
                     // positive fixnum
-                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_unsigned));
                 }
                 else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint8_t>::max)())
                 {
                     // uint 8
                     oa.write_character(to_char_type(0xCC));
-                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_unsigned));
                 }
                 else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint16_t>::max)())
                 {
                     // uint 16
                     oa.write_character(to_char_type(0xCD));
-                    write_number(static_cast<std::uint16_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint16_t>(j.m_data.m_value.number_unsigned));
                 }
                 else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint32_t>::max)())
                 {
                     // uint 32
                     oa.write_character(to_char_type(0xCE));
-                    write_number(static_cast<std::uint32_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint32_t>(j.m_data.m_value.number_unsigned));
                 }
                 else
                 {
                     // uint 64
                     oa.write_character(to_char_type(0xCF));
-                    write_number(static_cast<std::uint64_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint64_t>(j.m_data.m_value.number_unsigned));
                 }
                 break;
             }

single_include/nlohmann/json.hpp

@@ -20667,24 +20667,24 @@ class binary_writer
                     // MessagePack does not differentiate between positive
                     // signed integers and unsigned integers. Therefore, we used
                     // the code from the value_t::number_unsigned case here.
-                    if (j.m_data.m_value.number_unsigned < 128)
+                    if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) < 128)
                     {
                         // positive fixnum
                         write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
                     }
-                    else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint8_t>::max)())
+                    else if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) <= (std::numeric_limits<std::uint8_t>::max)())
                     {
                         // uint 8
                         oa.write_character(to_char_type(0xCC));
                         write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
                     }
-                    else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint16_t>::max)())
+                    else if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) <= (std::numeric_limits<std::uint16_t>::max)())
                     {
                         // uint 16
                         oa.write_character(to_char_type(0xCD));
                         write_number(static_cast<std::uint16_t>(j.m_data.m_value.number_integer));
                     }
-                    else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint32_t>::max)())
+                    else if (static_cast<typename BasicJsonType::number_unsigned_t>(j.m_data.m_value.number_integer) <= (std::numeric_limits<std::uint32_t>::max)())
                     {
                         // uint 32
                         oa.write_character(to_char_type(0xCE));
@@ -20740,31 +20740,31 @@ class binary_writer
                 if (j.m_data.m_value.number_unsigned < 128)
                 {
                     // positive fixnum
-                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_unsigned));
                 }
                 else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint8_t>::max)())
                 {
                     // uint 8
                     oa.write_character(to_char_type(0xCC));
-                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint8_t>(j.m_data.m_value.number_unsigned));
                 }
                 else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint16_t>::max)())
                 {
                     // uint 16
                     oa.write_character(to_char_type(0xCD));
-                    write_number(static_cast<std::uint16_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint16_t>(j.m_data.m_value.number_unsigned));
                 }
                 else if (j.m_data.m_value.number_unsigned <= (std::numeric_limits<std::uint32_t>::max)())
                 {
                     // uint 32
                     oa.write_character(to_char_type(0xCE));
-                    write_number(static_cast<std::uint32_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint32_t>(j.m_data.m_value.number_unsigned));
                 }
                 else
                 {
                     // uint 64
                     oa.write_character(to_char_type(0xCF));
-                    write_number(static_cast<std::uint64_t>(j.m_data.m_value.number_integer));
+                    write_number(static_cast<std::uint64_t>(j.m_data.m_value.number_unsigned));
                 }
                 break;
             }

tests/src/unit-msgpack.cpp

@@ -2475,3 +2475,60 @@ TEST_CASE("MessagePack lengths beyond UINT32_MAX cannot be serialized")
     }
 #endif
 }
+
+TEST_CASE("MessagePack numbers use the active union member (see #5644)")
+{
+    // when number_integer_t is narrower than number_unsigned_t, to_msgpack()
+    // used to read the union member that was not the active one, writing
+    // wrong bytes for some values; std::int64_t/std::uint64_t (the default
+    // types, where both members have the same width) were not affected
+    using int32_json = nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int32_t, std::uint64_t, double>;
+    using int16_json = nlohmann::basic_json<std::map, std::vector, std::string, bool, std::int16_t, std::uint64_t, double>;
+
+    SECTION("number_integer_t = std::int32_t")
+    {
+        SECTION("6442450944 (uint 64; the low 32 bits used to be sign-extended)")
+        {
+            const int32_json j = 6442450944ULL;
+            CHECK(j.is_number_unsigned());
+
+            std::vector<uint8_t> const expected{0xcf, 0x00, 0x00, 0x00, 0x01, 0x80, 0x00, 0x00, 0x00};
+            const auto result = int32_json::to_msgpack(j);
+            CHECK(result == expected);
+            CHECK(int32_json::from_msgpack(result) == j);
+        }
+
+        SECTION("4294967496 (uint 64; the low 32 bits used to be the whole value)")
+        {
+            const int32_json j = 4294967496ULL;
+            CHECK(j.is_number_unsigned());
+
+            std::vector<uint8_t> const expected{0xcf, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xc8};
+            const auto result = int32_json::to_msgpack(j);
+            CHECK(result == expected);
+            CHECK(int32_json::from_msgpack(result) == j);
+        }
+    }
+
+    SECTION("number_integer_t = std::int16_t, 98304 (uint 32)")
+    {
+        const int16_json j = 98304ULL;
+        CHECK(j.is_number_unsigned());
+
+        std::vector<uint8_t> const expected{0xce, 0x00, 0x01, 0x80, 0x00};
+        const auto result = int16_json::to_msgpack(j);
+        CHECK(result == expected);
+        CHECK(int16_json::from_msgpack(result) == j);
+    }
+
+    SECTION("default types (std::int64_t/std::uint64_t) are unaffected")
+    {
+        const json j = 4294967496ULL;
+        CHECK(j.is_number_unsigned());
+
+        std::vector<uint8_t> const expected{0xcf, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xc8};
+        const auto result = json::to_msgpack(j);
+        CHECK(result == expected);
+        CHECK(json::from_msgpack(result) == j);
+    }
+}