Fixed JsonVariant::isNull() not returning true after set((char*)0)

CHANGELOG.md

@@ -13,6 +13,8 @@ HEAD
 * Renamed `JsonArray::add()` (without arg) to `addElement()`
 * Renamed `JsonObject::get()` to `getMember()`
 * Renamed `JsonObject::getOrCreate()` to `getOrAddMember()`
+* Fixed `JsonVariant::isNull()` not returning `true` after `set((char*)0)`
+* Fixed segfault after `variant.set(serialized((char*)0))`
 
 v6.8.0-beta (2019-01-30)
 -----------

src/ArduinoJson/Json/JsonDeserializer.hpp

@@ -19,7 +19,6 @@ template <typename TReader, typename TStringStorage>
 class JsonDeserializer {
   typedef typename remove_reference<TStringStorage>::type::StringBuilder
       StringBuilder;
-  typedef const char *StringType;
 
  public:
   JsonDeserializer(MemoryPool &pool, TReader reader,
@@ -124,10 +123,10 @@ class JsonDeserializer {
       if (!slot) return DeserializationError::NoMemory;
 
       // Parse key
-      StringType key;
+      const char *key;
       err = parseKey(key);
       if (err) return err;
-      slot->setOwnedKey(key);
+      slot->setOwnedKey(make_not_null(key));
 
       // Skip spaces
       err = skipSpacesAndComments();
@@ -162,7 +161,7 @@ class JsonDeserializer {
     }
   }
 
-  DeserializationError parseKey(StringType &key) {
+  DeserializationError parseKey(const char *&key) {
     if (isQuote(current())) {
       return parseQuotedString(key);
     } else {
@@ -171,14 +170,14 @@ class JsonDeserializer {
   }
 
   DeserializationError parseStringValue(VariantData &variant) {
-    StringType value;
+    const char *value;
     DeserializationError err = parseQuotedString(value);
     if (err) return err;
-    variant.setOwnedString(value);
+    variant.setOwnedString(make_not_null(value));
     return DeserializationError::Ok;
   }
 
-  DeserializationError parseQuotedString(StringType &result) {
+  DeserializationError parseQuotedString(const char *&result) {
     StringBuilder builder = _stringStorage.startString();
     const char stopChar = current();
 
@@ -219,7 +218,7 @@ class JsonDeserializer {
     return DeserializationError::Ok;
   }
 
-  DeserializationError parseNonQuotedString(StringType &result) {
+  DeserializationError parseNonQuotedString(const char *&result) {
     StringBuilder builder = _stringStorage.startString();
 
     char c = current();

src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp

@@ -17,7 +17,6 @@ template <typename TReader, typename TStringStorage>
 class MsgPackDeserializer {
   typedef typename remove_reference<TStringStorage>::type::StringBuilder
       StringBuilder;
-  typedef const char *StringType;
 
  public:
   MsgPackDeserializer(MemoryPool &pool, TReader reader,
@@ -227,20 +226,20 @@ class MsgPackDeserializer {
   }
 
   template <typename T>
-  DeserializationError readString(StringType &str) {
+  DeserializationError readString(const char *&str) {
     T size;
     if (!readInteger(size)) return DeserializationError::IncompleteInput;
     return readString(str, size);
   }
 
   DeserializationError readString(VariantData &variant, size_t n) {
-    StringType s;
+    const char *s;
     DeserializationError err = readString(s, n);
-    if (!err) variant.setOwnedString(s);
+    if (!err) variant.setOwnedString(make_not_null(s));
     return err;
   }
 
-  DeserializationError readString(StringType &result, size_t n) {
+  DeserializationError readString(const char *&result, size_t n) {
     StringBuilder builder = _stringStorage.startString();
     for (; n; --n) {
       uint8_t c;
@@ -287,10 +286,10 @@ class MsgPackDeserializer {
       VariantSlot *slot = object.addSlot(_pool);
       if (!slot) return DeserializationError::NoMemory;
 
-      StringType key;
+      const char *key;
       DeserializationError err = parseKey(key);
       if (err) return err;
-      slot->setOwnedKey(key);
+      slot->setOwnedKey(make_not_null(key));
 
       err = parse(*slot->data());
       if (err) return err;
@@ -299,7 +298,7 @@ class MsgPackDeserializer {
     return DeserializationError::Ok;
   }
 
-  DeserializationError parseKey(StringType &key) {
+  DeserializationError parseKey(const char *&key) {
     uint8_t code;
     if (!readByte(code)) return DeserializationError::IncompleteInput;
 

src/ArduinoJson/Polyfills/gsl/not_null.hpp

@@ -0,0 +1,33 @@
+// ArduinoJson - arduinojson.org
+// Copyright Benoit Blanchon 2014-2019
+// MIT License
+
+#pragma once
+
+#include "../assert.hpp"
+
+namespace ARDUINOJSON_NAMESPACE {
+
+template <typename T>
+class not_null {
+ public:
+  explicit not_null(T ptr) : _ptr(ptr) {
+    ARDUINOJSON_ASSERT(ptr != NULL);
+  }
+
+  T get() const {
+    ARDUINOJSON_ASSERT(_ptr != NULL);
+    return _ptr;
+  }
+
+ private:
+  T _ptr;
+};
+
+template <typename T>
+not_null<T> make_not_null(T ptr) {
+  ARDUINOJSON_ASSERT(ptr != NULL);
+  return not_null<T>(ptr);
+}
+
+}  // namespace ARDUINOJSON_NAMESPACE

src/ArduinoJson/Strings/ConstRamStringAdapter.hpp

@@ -29,6 +29,7 @@ class ConstRamStringAdapter {
   }
 
   size_t size() const {
+    if (!_str) return 0;
     return strlen(_str);
   }
 

src/ArduinoJson/Strings/FlashStringAdapter.hpp

@@ -33,6 +33,7 @@ class FlashStringAdapter {
   }
 
   size_t size() const {
+    if (!_str) return 0;
     return strlen_P(reinterpret_cast<const char*>(_str));
   }
 

src/ArduinoJson/Strings/SizedFlashStringAdapter.hpp

@@ -29,7 +29,7 @@ class SizedFlashStringAdapter {
   }
 
   size_t size() const {
-    return strlen_P(reinterpret_cast<const char*>(_str));
+    return _size;
   }
 
   bool isStatic() const {

src/ArduinoJson/Strings/SizedRamStringAdapter.hpp

@@ -30,7 +30,7 @@ class SizedRamStringAdapter {
   }
 
   size_t size() const {
-    return strlen(reinterpret_cast<const char*>(_str));
+    return _size;
   }
 
   bool isStatic() const {

src/ArduinoJson/Variant/SlotFunctions.hpp

@@ -15,11 +15,11 @@ template <typename TAdaptedString>
 inline bool slotSetKey(VariantSlot* var, TAdaptedString key, MemoryPool* pool) {
   if (!var) return false;
   if (key.isStatic()) {
-    var->setLinkedKey(key.data());
+    var->setLinkedKey(make_not_null(key.data()));
   } else {
-    char* dup = key.save(pool);
+    const char* dup = key.save(pool);
     if (!dup) return false;
-    var->setOwnedKey(dup);
+    var->setOwnedKey(make_not_null(dup));
   }
   return true;
 }

src/ArduinoJson/Variant/VariantData.hpp

@@ -5,6 +5,7 @@
 #pragma once
 
 #include "../Misc/SerializedValue.hpp"
+#include "../Polyfills/gsl/not_null.hpp"
 #include "VariantContent.hpp"
 
 namespace ARDUINOJSON_NAMESPACE {
@@ -179,9 +180,13 @@ class VariantData {
   }
 
   void setLinkedRaw(SerializedValue<const char *> value) {
+    if (value.data()) {
       setType(VALUE_IS_LINKED_RAW);
       _content.asRaw.data = value.data();
       _content.asRaw.size = value.size();
+    } else {
+      setType(VALUE_IS_NULL);
+    }
   }
 
   template <typename T>
@@ -220,25 +225,26 @@ class VariantData {
   }
 
   void setLinkedString(const char *value) {
+    if (value) {
       setType(VALUE_IS_LINKED_STRING);
       _content.asString = value;
+    } else {
+      setType(VALUE_IS_NULL);
+    }
   }
 
   void setNull() {
     setType(VALUE_IS_NULL);
   }
 
-  void setOwnedString(const char *s) {
+  void setOwnedString(not_null<const char *> s) {
     setType(VALUE_IS_OWNED_STRING);
-    _content.asString = s;
+    _content.asString = s.get();
   }
 
-  template <typename T>
-  bool setOwnedString(T value, MemoryPool *pool) {
-    char *dup = value.save(pool);
-    if (dup) {
-      setType(VALUE_IS_OWNED_STRING);
-      _content.asString = dup;
+  bool setOwnedString(const char *s) {
+    if (s) {
+      setOwnedString(make_not_null(s));
       return true;
     } else {
       setType(VALUE_IS_NULL);
@@ -246,6 +252,11 @@ class VariantData {
     }
   }
 
+  template <typename T>
+  bool setOwnedString(T value, MemoryPool *pool) {
+    return setOwnedString(value.save(pool));
+  }
+
   void setUnsignedInteger(UInt value) {
     setType(VALUE_IS_POSITIVE_INTEGER);
     _content.asInteger = static_cast<UInt>(value);

src/ArduinoJson/Variant/VariantSlot.hpp

@@ -4,6 +4,7 @@
 
 #pragma once
 
+#include "../Polyfills/gsl/not_null.hpp"
 #include "../Polyfills/type_traits.hpp"
 #include "../Variant/VariantContent.hpp"
 
@@ -67,14 +68,14 @@ class VariantSlot {
     _next = VariantSlotDiff(slot - this);
   }
 
-  void setOwnedKey(const char* k) {
+  void setOwnedKey(not_null<const char*> k) {
     _flags |= KEY_IS_OWNED;
-    _key = k;
+    _key = k.get();
   }
 
-  void setLinkedKey(const char* k) {
+  void setLinkedKey(not_null<const char*> k) {
     _flags &= VALUE_MASK;
-    _key = k;
+    _key = k.get();
   }
 
   const char* key() const {

test/JsonDeserializer/deserializeJsonString.cpp

@@ -64,3 +64,11 @@ TEST_CASE("Invalid JSON string") {
     REQUIRE(deserializeJson(doc, input) == DeserializationError::InvalidInput);
   }
 }
+
+TEST_CASE("Not enough room to duplicate the string") {
+  DynamicJsonDocument doc(4);
+
+  REQUIRE(deserializeJson(doc, "\"hello world!\"") ==
+          DeserializationError::NoMemory);
+  REQUIRE(doc.isNull() == true);
+}

test/JsonVariant/isnull.cpp

@@ -35,15 +35,40 @@ TEST_CASE("JsonVariant::isNull()") {
     REQUIRE(variant.isNull() == false);
   }
 
-  /*  SECTION("return true when InvalidArray") {
+  SECTION("return true after set(JsonArray())") {
     variant.set(JsonArray());
     REQUIRE(variant.isNull() == true);
   }
-  */
-  /*  SECTION("return true when InvalidObject") {
+
+  SECTION("return true after set(JsonObject())") {
     variant.set(JsonObject());
     REQUIRE(variant.isNull() == true);
-    }*/
+  }
+
+  SECTION("return false after set('hello')") {
+    variant.set("hello");
+    REQUIRE(variant.isNull() == false);
+  }
+
+  SECTION("return true after set((char*)0)") {
+    variant.set(static_cast<char*>(0));
+    REQUIRE(variant.isNull() == true);
+  }
+
+  SECTION("return true after set((const char*)0)") {
+    variant.set(static_cast<const char*>(0));
+    REQUIRE(variant.isNull() == true);
+  }
+
+  SECTION("return true after set(serialized((char*)0))") {
+    variant.set(serialized(static_cast<char*>(0)));
+    REQUIRE(variant.isNull() == true);
+  }
+
+  SECTION("return true after set(serialized((const char*)0))") {
+    variant.set(serialized(static_cast<const char*>(0)));
+    REQUIRE(variant.isNull() == true);
+  }
 
   SECTION("works with JsonVariantConst") {
     variant.set(42);