Implement MsgPackBinary using raw strings and converters

extras/tests/MixedConfiguration/string_length_size_1.cpp

@@ -21,16 +21,16 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 1") {
     REQUIRE(doc.overflowed() == true);
   }
 
-  SECTION("set() returns true if binary has 255 characters") {
+  SECTION("set() returns true if binary has 253 characters") {
-    auto str = std::string(255, '?');
+    auto str = std::string(253, '?');
     auto result = doc.set(MsgPackBinary(str.data(), str.size()));
 
     REQUIRE(result == true);
     REQUIRE(doc.overflowed() == false);
   }
 
-  SECTION("set() returns false if binary has 256 characters") {
+  SECTION("set() returns false if binary has 254 characters") {
-    auto str = std::string(256, '?');
+    auto str = std::string(254, '?');
     auto result = doc.set(MsgPackBinary(str.data(), str.size()));
 
     REQUIRE(result == false);
@@ -70,8 +70,8 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 1") {
     REQUIRE(err == DeserializationError::NoMemory);
   }
 
-  SECTION("deserializeMsgPack() returns Ok if binary has 255 characters") {
+  SECTION("deserializeMsgPack() returns Ok if binary has 253 characters") {
-    auto input = "\xc4\xff" + std::string(255, '?');
+    auto input = "\xc4\xfd" + std::string(253, '?');
 
     auto err = deserializeMsgPack(doc, input);
 
@@ -79,8 +79,8 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 1") {
   }
 
   SECTION(
-      "deserializeMsgPack() returns NoMemory if binary has 256 characters") {
+      "deserializeMsgPack() returns NoMemory if binary has 254 characters") {
-    auto input = std::string("\xc5\x01\x00", 3) + std::string(256, '?');
+    auto input = "\xc4\xfe" + std::string(254, '?');
 
     auto err = deserializeMsgPack(doc, input);
 

extras/tests/MixedConfiguration/string_length_size_2.cpp

@@ -21,16 +21,16 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 2") {
     REQUIRE(doc.overflowed() == true);
   }
 
-  SECTION("set() returns true if string has 65535 characters") {
+  SECTION("set() returns true if string has 65532 characters") {
-    auto str = std::string(65535, '?');
+    auto str = std::string(65532, '?');
     auto result = doc.set(MsgPackBinary(str.data(), str.size()));
 
     REQUIRE(result == true);
     REQUIRE(doc.overflowed() == false);
   }
 
-  SECTION("set() returns false if string has 65536 characters") {
+  SECTION("set() returns false if string has 65533 characters") {
-    auto str = std::string(65536, '?');
+    auto str = std::string(65533, '?');
     auto result = doc.set(MsgPackBinary(str.data(), str.size()));
 
     REQUIRE(result == false);
@@ -71,8 +71,8 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 2") {
     REQUIRE(err == DeserializationError::NoMemory);
   }
 
-  SECTION("deserializeMsgPack() returns Ok if binary has 65535 characters") {
+  SECTION("deserializeMsgPack() returns Ok if binary has 65532 characters") {
-    auto input = "\xc5\xff\xff" + std::string(65535, '?');
+    auto input = "\xc5\xff\xfc" + std::string(65532, '?');
 
     auto err = deserializeMsgPack(doc, input);
 
@@ -80,9 +80,8 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 2") {
   }
 
   SECTION(
-      "deserializeMsgPack() returns NoMemory of binary has 65536 characters") {
+      "deserializeMsgPack() returns NoMemory of binary has 65534 characters") {
-    auto input =
+    auto input = "\xc5\xff\xfd" + std::string(65534, '?');
-        std::string("\xc6\x00\x01\x00\x00", 5) + std::string(65536, '?');
 
     auto err = deserializeMsgPack(doc, input);
 

extras/tests/MsgPackDeserializer/errors.cpp

@@ -189,19 +189,7 @@ static std::string msgPackToJson(const char* input, size_t inputSize) {
   return doc.as<std::string>();
 }
 
-TEST_CASE("deserializeMsgPack() replaces unsupported types by null") {
+TEST_CASE("deserializeMsgPack() replaces ext types by null") {
-  SECTION("bin 8") {
-    REQUIRE(msgPackToJson("\x92\xc4\x01X\x2A", 5) == "[null,42]");
-  }
-
-  SECTION("bin 16") {
-    REQUIRE(msgPackToJson("\x92\xc5\x00\x01X\x2A", 6) == "[null,42]");
-  }
-
-  SECTION("bin 32") {
-    REQUIRE(msgPackToJson("\x92\xc6\x00\x00\x00\x01X\x2A", 8) == "[null,42]");
-  }
-
   SECTION("ext 8") {
     REQUIRE(msgPackToJson("\x92\xc7\x01\x01\x01\x2A", 6) == "[null,42]");
   }

src/ArduinoJson.hpp

@@ -47,6 +47,7 @@
 #include "ArduinoJson/Json/JsonDeserializer.hpp"
 #include "ArduinoJson/Json/JsonSerializer.hpp"
 #include "ArduinoJson/Json/PrettyJsonSerializer.hpp"
+#include "ArduinoJson/MsgPack/MsgPackBinary.hpp"
 #include "ArduinoJson/MsgPack/MsgPackDeserializer.hpp"
 #include "ArduinoJson/MsgPack/MsgPackSerializer.hpp"
 

src/ArduinoJson/Json/JsonSerializer.hpp

@@ -81,10 +81,6 @@ class JsonSerializer : public VariantDataVisitor<size_t> {
     return bytesWritten();
   }
 
-  size_t visit(MsgPackBinary) {
-    return visit(nullptr);
-  }
-
   size_t visit(JsonInteger value) {
     formatter_.writeInteger(value);
     return bytesWritten();

src/ArduinoJson/MsgPack/MsgPackBinary.hpp

@@ -20,4 +20,76 @@ class MsgPackBinary {
   size_t size_;
 };
 
+template <>
+struct Converter<MsgPackBinary> : private detail::VariantAttorney {
+  static void toJson(MsgPackBinary src, JsonVariant dst) {
+    auto data = VariantAttorney::getData(dst);
+    if (!data)
+      return;
+    auto resources = getResourceManager(dst);
+    if (src.data()) {
+      size_t headerSize = src.size() >= 0x10000 ? 5
+                          : src.size() >= 0x100 ? 3
+                                                : 2;
+      auto str = resources->createString(src.size() + headerSize);
+      if (str) {
+        resources->saveString(str);
+        auto ptr = reinterpret_cast<uint8_t*>(str->data);
+        switch (headerSize) {
+          case 2:
+            ptr[0] = uint8_t(0xc4);
+            ptr[1] = uint8_t(src.size() & 0xff);
+            break;
+          case 3:
+            ptr[0] = uint8_t(0xc5);
+            ptr[1] = uint8_t(src.size() >> 8 & 0xff);
+            ptr[2] = uint8_t(src.size() & 0xff);
+            break;
+          case 5:
+            ptr[0] = uint8_t(0xc6);
+            ptr[1] = uint8_t(src.size() >> 24 & 0xff);
+            ptr[2] = uint8_t(src.size() >> 16 & 0xff);
+            ptr[3] = uint8_t(src.size() >> 8 & 0xff);
+            ptr[4] = uint8_t(src.size() & 0xff);
+            break;
+          default:
+            ARDUINOJSON_ASSERT(false);
+        }
+        memcpy(ptr + headerSize, src.data(), src.size());
+        data->setRawString(str);
+        return;
+      }
+    }
+    data->setNull();
+  }
+
+  static MsgPackBinary fromJson(JsonVariantConst src) {
+    auto data = getData(src);
+    if (!data)
+      return {};
+    auto rawstr = data->asRawString();
+    auto p = reinterpret_cast<const uint8_t*>(rawstr.c_str());
+    auto n = rawstr.size();
+    if (n >= 2 && p[0] == 0xc4) {  // bin 8
+      size_t size = p[1];
+      if (size + 2 == n)
+        return MsgPackBinary(p + 2, size);
+    } else if (n >= 3 && p[0] == 0xc5) {  // bin 16
+      size_t size = size_t(p[1] << 8) | p[2];
+      if (size + 3 == n)
+        return MsgPackBinary(p + 3, size);
+    } else if (n >= 5 && p[0] == 0xc6) {  // bin 32
+      size_t size =
+          size_t(p[1] << 24) | size_t(p[2] << 16) | size_t(p[3] << 8) | p[4];
+      if (size + 5 == n)
+        return MsgPackBinary(p + 5, size);
+    }
+    return {};
+  }
+
+  static bool checkJson(JsonVariantConst src) {
+    return fromJson(src).data() != nullptr;
+  }
+};
+
 ARDUINOJSON_END_PUBLIC_NAMESPACE

src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp

@@ -373,13 +373,50 @@ class MsgPackDeserializer {
   }
 
   DeserializationError::Code readBinary(VariantData* variant, size_t n) {
-    DeserializationError::Code err;
+    uint8_t headerSize;
 
-    err = readString(n);
+    if (n <= 0xFF) {
+      headerSize = 2;
+    }
+#if ARDUINOJSON_STRING_LENGTH_SIZE >= 2
+    else if (n <= 0xFFFF) {
+      headerSize = 3;
+    }
+#endif
+#if ARDUINOJSON_STRING_LENGTH_SIZE >= 4
+    else {
+      headerSize = 5;
+    }
+#else
+    else {
+      return DeserializationError::NoMemory;
+    }
+#endif
+
+    char* p = stringBuffer_.reserve(headerSize + n);
+    if (!p)
+      return DeserializationError::NoMemory;
+
+    if (n <= 0xFF) {
+      *p++ = '\xc4';
+      *p++ = char(n & 0xFF);
+    } else if (n <= 0xFFFF) {
+      *p++ = '\xc5';
+      *p++ = char(n >> 8 & 0xFF);
+      *p++ = char(n & 0xFF);
+    } else {
+      *p++ = '\xc6';
+      *p++ = char(n >> 24 & 0xFF);
+      *p++ = char(n >> 16 & 0xFF);
+      *p++ = char(n >> 8 & 0xFF);
+      *p++ = char(n & 0xFF);
+    }
+
+    auto err = readBytes(p, n);
     if (err)
       return err;
 
-    variant->setBinary(stringBuffer_.save());
+    variant->setRawString(stringBuffer_.save());
     return DeserializationError::Ok;
   }
 

src/ArduinoJson/MsgPack/MsgPackSerializer.hpp

@@ -123,21 +123,6 @@ class MsgPackSerializer : public VariantDataVisitor<size_t> {
     return bytesWritten();
   }
 
-  size_t visit(MsgPackBinary value) {
-    if (value.size() <= 0xFF) {
-      writeByte(0xC4);
-      writeInteger(uint8_t(value.size()));
-    } else if (value.size() <= 0xFFFF) {
-      writeByte(0xC5);
-      writeInteger(uint16_t(value.size()));
-    } else {
-      writeByte(0xC6);
-      writeInteger(uint32_t(value.size()));
-    }
-    writeBytes(reinterpret_cast<const uint8_t*>(value.data()), value.size());
-    return bytesWritten();
-  }
-
   size_t visit(JsonInteger value) {
     if (value > 0) {
       visit(static_cast<JsonUInt>(value));

src/ArduinoJson/Variant/ConverterImpl.hpp

@@ -192,21 +192,6 @@ struct Converter<SerializedValue<T>> : private detail::VariantAttorney {
   }
 };
 
-template <>
-struct Converter<MsgPackBinary> : private detail::VariantAttorney {
-  static void toJson(MsgPackBinary src, JsonVariant dst) {
-    detail::VariantData::setBinary(getData(dst), src, getResourceManager(dst));
-  }
-  static MsgPackBinary fromJson(JsonVariantConst src) {
-    auto data = getData(src);
-    return data ? data->asBinary() : MsgPackBinary();
-  }
-  static bool checkJson(JsonVariantConst src) {
-    auto data = getData(src);
-    return data && data->isBinary();
-  }
-};
-
 template <>
 struct Converter<detail::nullptr_t> : private detail::VariantAttorney {
   static void toJson(detail::nullptr_t, JsonVariant dst) {

src/ArduinoJson/Variant/VariantCompare.hpp

@@ -134,25 +134,6 @@ struct RawComparer : ComparerBase {
   using ComparerBase::visit;
 };
 
-struct MsgPackBinaryComparer : ComparerBase {
-  MsgPackBinary rhs_;
-
-  explicit MsgPackBinaryComparer(MsgPackBinary rhs) : rhs_(rhs) {}
-
-  CompareResult visit(MsgPackBinary lhs) {
-    size_t size = rhs_.size() < lhs.size() ? rhs_.size() : lhs.size();
-    int n = memcmp(lhs.data(), rhs_.data(), size);
-    if (n < 0)
-      return COMPARE_RESULT_LESS;
-    else if (n > 0)
-      return COMPARE_RESULT_GREATER;
-    else
-      return COMPARE_RESULT_EQUAL;
-  }
-
-  using ComparerBase::visit;
-};
-
 struct VariantComparer : ComparerBase {
   JsonVariantConst rhs;
 
@@ -183,11 +164,6 @@ struct VariantComparer : ComparerBase {
     return reverseResult(comparer);
   }
 
-  CompareResult visit(MsgPackBinary value) {
-    MsgPackBinaryComparer comparer(value);
-    return reverseResult(comparer);
-  }
-
   CompareResult visit(JsonInteger lhs) {
     Comparer<JsonInteger> comparer(lhs);
     return reverseResult(comparer);

src/ArduinoJson/Variant/VariantContent.hpp

@@ -21,7 +21,6 @@ enum {
   VALUE_IS_RAW_STRING = 0x03,
   VALUE_IS_LINKED_STRING = 0x04,
   VALUE_IS_OWNED_STRING = 0x05,
-  VALUE_IS_BINARY = 0x07,
 
   // CAUTION: no OWNED_VALUE_BIT below
 

src/ArduinoJson/Variant/VariantData.hpp

@@ -6,7 +6,6 @@
 
 #include <ArduinoJson/Memory/StringNode.hpp>
 #include <ArduinoJson/Misc/SerializedValue.hpp>
-#include <ArduinoJson/MsgPack/MsgPackBinary.hpp>
 #include <ArduinoJson/Numbers/convertNumber.hpp>
 #include <ArduinoJson/Strings/JsonString.hpp>
 #include <ArduinoJson/Strings/StringAdapters.hpp>
@@ -49,10 +48,6 @@ class VariantData {
         return visit.visit(RawString(content_.asOwnedString->data,
                                      content_.asOwnedString->length));
 
-      case VALUE_IS_BINARY:
-        return visit.visit(MsgPackBinary(content_.asOwnedString->data,
-                                         content_.asOwnedString->length));
-
       case VALUE_IS_SIGNED_INTEGER:
         return visit.visit(content_.asSignedInteger);
 
@@ -190,16 +185,6 @@ class VariantData {
     }
   }
 
-  MsgPackBinary asBinary() const {
-    switch (type()) {
-      case VALUE_IS_BINARY:
-        return MsgPackBinary(content_.asOwnedString->data,
-                             content_.asOwnedString->length);
-      default:
-        return MsgPackBinary(nullptr, 0);
-    }
-  }
-
   VariantData* getElement(size_t index,
                           const ResourceManager* resources) const {
     return ArrayData::getElement(asArray(), index, resources);
@@ -289,10 +274,6 @@ class VariantData {
     return type() == VALUE_IS_LINKED_STRING || type() == VALUE_IS_OWNED_STRING;
   }
 
-  bool isBinary() const {
-    return type() == VALUE_IS_BINARY;
-  }
-
   size_t nesting(const ResourceManager* resources) const {
     auto collection = asCollection();
     if (collection)
@@ -413,28 +394,6 @@ class VariantData {
     var->setRawString(value, resources);
   }
 
-  void setBinary(StringNode* s) {
-    ARDUINOJSON_ASSERT(s);
-    setType(VALUE_IS_BINARY);
-    content_.asOwnedString = s;
-  }
-
-  void setBinary(MsgPackBinary value, ResourceManager* resources) {
-    auto dup = resources->saveString(
-        adaptString(reinterpret_cast<const char*>(value.data()), value.size()));
-    if (dup)
-      setBinary(dup);
-    else
-      setNull();
-  }
-
-  static void setBinary(VariantData* var, MsgPackBinary value,
-                        ResourceManager* resources) {
-    if (!var)
-      return;
-    var->setBinary(value, resources);
-  }
-
   template <typename TAdaptedString>
   void setString(TAdaptedString value, ResourceManager* resources) {
     setNull(resources);