Fixed "linked" strings incorrectly marked as "owned" (fixes #1318)

extras/tests/JsonDeserializer/input_types.cpp

@@ -3,11 +3,27 @@
 // MIT License
 
 #include <ArduinoJson.h>
+
 #include <catch.hpp>
 #include <sstream>
 
 #include "CustomReader.hpp"
 
+TEST_CASE("deserializeJson(char*)") {
+  StaticJsonDocument<1024> doc;
+
+  SECTION("should not duplicate strings") {
+    char input[] = "{\"hello\":\"world\"}";
+
+    DeserializationError err = deserializeJson(doc, input);
+
+    REQUIRE(err == DeserializationError::Ok);
+    CHECK(doc.memoryUsage() == JSON_OBJECT_SIZE(1));
+    CHECK(doc.as<JsonVariant>().memoryUsage() ==
+          JSON_OBJECT_SIZE(1));  // issue #1318
+  }
+}
+
 TEST_CASE("deserializeJson(const std::string&)") {
   DynamicJsonDocument doc(4096);
 

src/ArduinoJson/Json/JsonDeserializer.hpp

@@ -240,7 +240,7 @@ class JsonDeserializer {
           if (!slot)
             return DeserializationError::NoMemory;
 
-          slot->setOwnedKey(make_not_null(key));
+          slot->setKey(key, typename TStringStorage::storage_policy());
 
           variant = slot->data();
         }
@@ -339,7 +339,8 @@ class JsonDeserializer {
     if (err)
       return err;
     const char *value = _stringStorage.save(_pool);
-    variant.setOwnedString(make_not_null(value));
+    variant.setString(make_not_null(value),
+                      typename TStringStorage::storage_policy());
     return DeserializationError::Ok;
   }
 

src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp

@@ -233,7 +233,8 @@ class MsgPackDeserializer {
     const char *s = 0;  // <- mute "maybe-uninitialized" (+4 bytes on AVR)
     DeserializationError err = readString(s, n);
     if (!err)
-      variant.setOwnedString(make_not_null(s));
+      variant.setString(make_not_null(s),
+                        typename TStringStorage::storage_policy());
     return err;
   }
 
@@ -303,7 +304,7 @@ class MsgPackDeserializer {
       DeserializationError err = parseKey(key);
       if (err)
         return err;
-      slot->setOwnedKey(make_not_null(key));
+      slot->setKey(key, typename TStringStorage::storage_policy());
 
       err = parse(*slot->data(), nestingLimit.decrement());
       if (err)

src/ArduinoJson/StringStorage/StringCopier.hpp

@@ -48,6 +48,8 @@ class StringCopier {
     return _ptr;
   }
 
+  typedef storage_policies::store_by_copy storage_policy;
+
  private:
   char* _ptr;
   size_t _size;

src/ArduinoJson/StringStorage/StringMover.hpp

@@ -5,6 +5,7 @@
 #pragma once
 
 #include <ArduinoJson/Namespace.hpp>
+#include <ArduinoJson/Strings/StoragePolicy.hpp>
 
 namespace ARDUINOJSON_NAMESPACE {
 
@@ -32,6 +33,8 @@ class StringMover {
     return _startPtr;
   }
 
+  typedef storage_policies::store_by_address storage_policy;
+
  private:
   char* _writePtr;
   char* _startPtr;

src/ArduinoJson/Variant/SlotFunctions.hpp

@@ -30,7 +30,7 @@ template <typename TAdaptedString>
 inline bool slotSetKey(VariantSlot* var, TAdaptedString key, MemoryPool*,
                        storage_policies::store_by_address) {
   ARDUINOJSON_ASSERT(var);
-  var->setLinkedKey(make_not_null(key.data()));
+  var->setKey(key.data(), storage_policies::store_by_address());
   return true;
 }
 
@@ -41,7 +41,7 @@ inline bool slotSetKey(VariantSlot* var, TAdaptedString key, MemoryPool* pool,
   if (!dup)
     return false;
   ARDUINOJSON_ASSERT(var);
-  var->setOwnedKey(make_not_null(dup));
+  var->setKey(dup, storage_policies::store_by_copy());
   return true;
 }
 

src/ArduinoJson/Variant/VariantData.hpp

@@ -101,7 +101,7 @@ class VariantData {
       case VALUE_IS_OBJECT:
         return toObject().copyFrom(src._content.asCollection, pool);
       case VALUE_IS_OWNED_STRING:
-        return setOwnedString(RamStringAdapter(src._content.asString), pool);
+        return setString(RamStringAdapter(src._content.asString), pool);
       case VALUE_IS_OWNED_RAW:
         return setOwnedRaw(
             serialized(src._content.asRaw.data, src._content.asRaw.size), pool);
@@ -238,27 +238,24 @@ class VariantData {
     _content.asInteger = value;
   }
 
-  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(not_null<const char *> s) {
+  void setString(not_null<const char *> s, storage_policies::store_by_copy) {
     setType(VALUE_IS_OWNED_STRING);
     _content.asString = s.get();
   }
 
-  bool setOwnedString(const char *s) {
+  void setString(not_null<const char *> s, storage_policies::store_by_address) {
+    setType(VALUE_IS_LINKED_STRING);
+    _content.asString = s.get();
+  }
+
+  template <typename TStoragePolicy>
+  bool setString(const char *s, TStoragePolicy storage_policy) {
     if (s) {
-      setOwnedString(make_not_null(s));
+      setString(make_not_null(s), storage_policy);
       return true;
     } else {
       setType(VALUE_IS_NULL);
@@ -267,8 +264,30 @@ class VariantData {
   }
 
   template <typename TAdaptedString>
-  bool setOwnedString(TAdaptedString value, MemoryPool *pool) {
+  bool setString(TAdaptedString value, MemoryPool *pool) {
-    return setOwnedString(pool->saveString(value));
+    return setString(value, pool, typename TAdaptedString::storage_policy());
+  }
+
+  template <typename TAdaptedString>
+  inline bool setString(TAdaptedString value, MemoryPool *pool,
+                        storage_policies::decide_at_runtime) {
+    if (value.isStatic())
+      return setString(value, pool, storage_policies::store_by_address());
+    else
+      return setString(value, pool, storage_policies::store_by_copy());
+  }
+
+  template <typename TAdaptedString>
+  inline bool setString(TAdaptedString value, MemoryPool *,
+                        storage_policies::store_by_address) {
+    return setString(value.data(), storage_policies::store_by_address());
+  }
+
+  template <typename TAdaptedString>
+  inline bool setString(TAdaptedString value, MemoryPool *pool,
+                        storage_policies::store_by_copy) {
+    return setString(pool->saveString(value),
+                     storage_policies::store_by_copy());
   }
 
   CollectionData &toArray() {

src/ArduinoJson/Variant/VariantFunctions.hpp

@@ -99,62 +99,18 @@ inline bool variantSetOwnedRaw(VariantData *var, SerializedValue<T> value,
   return var != 0 && var->setOwnedRaw(value, pool);
 }
 
-inline bool variantSetLinkedString(VariantData *var, const char *value) {
-  if (!var)
-    return false;
-  var->setLinkedString(value);
-  return true;
-}
-
 inline void variantSetNull(VariantData *var) {
   if (!var)
     return;
   var->setNull();
 }
 
-inline bool variantSetOwnedString(VariantData *var, char *value) {
-  if (!var)
-    return false;
-  var->setOwnedString(value);
-  return true;
-}
-
-template <typename TAdaptedString>
-inline bool variantSetOwnedString(VariantData *var, TAdaptedString value,
-                                  MemoryPool *pool) {
-  return var != 0 && var->setOwnedString(value, pool);
-}
-
-template <typename TAdaptedString>
-inline bool variantSetString(VariantData *var, TAdaptedString value,
-                             MemoryPool *pool,
-                             storage_policies::decide_at_runtime) {
-  if (value.isStatic())
-    return variantSetString(var, value, pool,
-                            storage_policies::store_by_address());
-  else
-    return variantSetString(var, value, pool,
-                            storage_policies::store_by_copy());
-}
-
 template <typename TAdaptedString>
 inline bool variantSetString(VariantData *var, TAdaptedString value,
                              MemoryPool *pool) {
-  return variantSetString(var, value, pool,
+  if (!var)
-                          typename TAdaptedString::storage_policy());
+    return false;
-}
+  return var->setString(value, pool);
-
-template <typename TAdaptedString>
-inline bool variantSetString(VariantData *var, TAdaptedString value,
-                             MemoryPool *, storage_policies::store_by_address) {
-  return variantSetLinkedString(var, value.data());
-}
-
-template <typename TAdaptedString>
-inline bool variantSetString(VariantData *var, TAdaptedString value,
-                             MemoryPool *pool,
-                             storage_policies::store_by_copy) {
-  return variantSetOwnedString(var, value, pool);
 }
 
 template <typename T>

src/ArduinoJson/Variant/VariantSlot.hpp

@@ -4,12 +4,13 @@
 
 #pragma once
 
+#include <stdint.h>  // int8_t, int16_t
+
 #include <ArduinoJson/Polyfills/gsl/not_null.hpp>
 #include <ArduinoJson/Polyfills/type_traits.hpp>
+#include <ArduinoJson/Strings/StoragePolicy.hpp>
 #include <ArduinoJson/Variant/VariantContent.hpp>
 
-#include <stdint.h>  // int8_t, int16_t
-
 namespace ARDUINOJSON_NAMESPACE {
 
 typedef conditional<sizeof(void*) <= 2, int8_t, int16_t>::type VariantSlotDiff;
@@ -69,14 +70,16 @@ class VariantSlot {
     _next = VariantSlotDiff(slot - this);
   }
 
-  void setOwnedKey(not_null<const char*> k) {
+  void setKey(const char* k, storage_policies::store_by_copy) {
+    ARDUINOJSON_ASSERT(k != NULL);
     _flags |= KEY_IS_OWNED;
-    _key = k.get();
+    _key = k;
   }
 
-  void setLinkedKey(not_null<const char*> k) {
+  void setKey(const char* k, storage_policies::store_by_address) {
+    ARDUINOJSON_ASSERT(k != NULL);
     _flags &= VALUE_MASK;
-    _key = k.get();
+    _key = k;
   }
 
   const char* key() const {