Added JsonDocument::overflowed() (closes #1358)

CHANGELOG.md

@@ -5,6 +5,7 @@ HEAD
 ----
 
 * Added a build failure when nullptr is defined as a macro (issue #1355)
+* Added `JsonDocument::overflowed()` which tells if the memory pool was too small (issue #1358)
 
 v6.16.1 (2020-08-04)
 -------

extras/tests/JsonDocument/CMakeLists.txt

@@ -11,6 +11,7 @@ add_executable(JsonDocumentTests
 	DynamicJsonDocument.cpp
 	isNull.cpp
 	nesting.cpp
+	overflowed.cpp
 	remove.cpp
 	shrinkToFit.cpp
 	size.cpp

extras/tests/JsonDocument/overflowed.cpp

@@ -0,0 +1,79 @@
+// ArduinoJson - arduinojson.org
+// Copyright Benoit Blanchon 2014-2020
+// MIT License
+
+#include <ArduinoJson.h>
+#include <catch.hpp>
+
+TEST_CASE("JsonDocument::overflowed()") {
+  SECTION("returns false on a fresh object") {
+    StaticJsonDocument<0> doc;
+    CHECK(doc.overflowed() == false);
+  }
+
+  SECTION("returns true after a failed insertion") {
+    StaticJsonDocument<0> doc;
+    doc.add(0);
+    CHECK(doc.overflowed() == true);
+  }
+
+  SECTION("returns false after successful insertion") {
+    StaticJsonDocument<JSON_ARRAY_SIZE(1)> doc;
+    doc.add(0);
+    CHECK(doc.overflowed() == false);
+  }
+
+  SECTION("returns true after a failed string copy") {
+    StaticJsonDocument<JSON_ARRAY_SIZE(1)> doc;
+    doc.add(std::string("example"));
+    CHECK(doc.overflowed() == true);
+  }
+
+  SECTION("returns false after a successful string copy") {
+    StaticJsonDocument<JSON_ARRAY_SIZE(1) + 8> doc;
+    doc.add(std::string("example"));
+    CHECK(doc.overflowed() == false);
+  }
+
+  SECTION("returns true after a failed deserialization") {
+    StaticJsonDocument<JSON_ARRAY_SIZE(1)> doc;
+    deserializeJson(doc, "[\"example\"]");
+    CHECK(doc.overflowed() == true);
+  }
+
+  SECTION("returns false after a successful deserialization") {
+    StaticJsonDocument<JSON_ARRAY_SIZE(1) + 8> doc;
+    deserializeJson(doc, "[\"example\"]");
+    CHECK(doc.overflowed() == false);
+  }
+
+  SECTION("returns false after clear()") {
+    StaticJsonDocument<0> doc;
+    doc.add(0);
+    doc.clear();
+    CHECK(doc.overflowed() == false);
+  }
+
+  SECTION("remains false after shrinkToFit()") {
+    DynamicJsonDocument doc(JSON_ARRAY_SIZE(1));
+    doc.add(0);
+    doc.shrinkToFit();
+    CHECK(doc.overflowed() == false);
+  }
+
+  SECTION("remains true after shrinkToFit()") {
+    DynamicJsonDocument doc(JSON_ARRAY_SIZE(1));
+    doc.add(0);
+    doc.add(0);
+    doc.shrinkToFit();
+    CHECK(doc.overflowed() == true);
+  }
+
+  SECTION("return false after garbageCollect()") {
+    DynamicJsonDocument doc(JSON_ARRAY_SIZE(1));
+    doc.add(0);
+    doc.add(0);
+    doc.garbageCollect();
+    CHECK(doc.overflowed() == false);
+  }
+}

extras/tests/MemoryPool/StringCopier.cpp

@@ -12,9 +12,9 @@ TEST_CASE("StringCopier") {
 
   SECTION("Works when buffer is big enough") {
     MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(6)));
-    StringCopier str;
+    StringCopier str(pool);
 
-    str.startString(&pool);
+    str.startString();
     str.append("hello");
     str.append('\0');
 
@@ -24,9 +24,9 @@ TEST_CASE("StringCopier") {
 
   SECTION("Returns null when too small") {
     MemoryPool pool(buffer, sizeof(void*));
-    StringCopier str;
+    StringCopier str(pool);
 
-    str.startString(&pool);
+    str.startString();
     str.append("hello world!");
 
     REQUIRE(str.isValid() == false);
@@ -34,22 +34,22 @@ TEST_CASE("StringCopier") {
 
   SECTION("Increases size of memory pool") {
     MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(6)));
-    StringCopier str;
+    StringCopier str(pool);
 
-    str.startString(&pool);
+    str.startString();
     str.append('h');
-    str.save(&pool);
+    str.save();
 
     REQUIRE(1 == pool.size());
   }
 }
 
-static const char* addStringToPool(MemoryPool* pool, const char* s) {
-  StringCopier str;
-  str.startString(pool);
+static const char* addStringToPool(MemoryPool& pool, const char* s) {
+  StringCopier str(pool);
+  str.startString();
   str.append(s);
   str.append('\0');
-  return str.save(pool);
+  return str.save();
 }
 
 TEST_CASE("StringCopier::save() deduplicates strings") {
@@ -57,9 +57,9 @@ TEST_CASE("StringCopier::save() deduplicates strings") {
   MemoryPool pool(buffer, 4096);
 
   SECTION("Basic") {
-    const char* s1 = addStringToPool(&pool, "hello");
-    const char* s2 = addStringToPool(&pool, "world");
-    const char* s3 = addStringToPool(&pool, "hello");
+    const char* s1 = addStringToPool(pool, "hello");
+    const char* s2 = addStringToPool(pool, "world");
+    const char* s3 = addStringToPool(pool, "hello");
 
     REQUIRE(s1 == s3);
     REQUIRE(s2 != s3);
@@ -67,16 +67,16 @@ TEST_CASE("StringCopier::save() deduplicates strings") {
   }
 
   SECTION("Requires terminator") {
-    const char* s1 = addStringToPool(&pool, "hello world");
-    const char* s2 = addStringToPool(&pool, "hello");
+    const char* s1 = addStringToPool(pool, "hello world");
+    const char* s2 = addStringToPool(pool, "hello");
 
     REQUIRE(s2 != s1);
     REQUIRE(pool.size() == 12 + 6);
   }
 
   SECTION("Don't overrun") {
-    const char* s1 = addStringToPool(&pool, "hello world");
-    const char* s2 = addStringToPool(&pool, "wor");
+    const char* s1 = addStringToPool(pool, "hello world");
+    const char* s2 = addStringToPool(pool, "wor");
 
     REQUIRE(s2 != s1);
     REQUIRE(pool.size() == 12 + 4);

extras/tests/Misc/Utf8.cpp

@@ -12,8 +12,8 @@ using namespace ARDUINOJSON_NAMESPACE;
 static void testCodepoint(uint32_t codepoint, std::string expected) {
   char buffer[4096];
   MemoryPool pool(buffer, 4096);
-  StringCopier str;
-  str.startString(&pool);
+  StringCopier str(pool);
+  str.startString();
 
   CAPTURE(codepoint);
   Utf8::encodeCodepoint(codepoint, str);

src/ArduinoJson/Deserialization/deserialize.hpp

@@ -32,8 +32,9 @@ deserialize(JsonDocument &doc, const TString &input, NestingLimit nestingLimit,
             TFilter filter) {
   Reader<TString> reader(input);
   doc.clear();
-  return makeDeserializer<TDeserializer>(doc.memoryPool(), reader,
-                                         makeStringStorage(input))
+  return makeDeserializer<TDeserializer>(
+             doc.memoryPool(), reader,
+             makeStringStorage(input, doc.memoryPool()))
       .parse(doc.data(), filter, nestingLimit);
 }
 //
@@ -47,8 +48,9 @@ DeserializationError deserialize(JsonDocument &doc, TChar *input,
                                  TFilter filter) {
   BoundedReader<TChar *> reader(input, inputSize);
   doc.clear();
-  return makeDeserializer<TDeserializer>(doc.memoryPool(), reader,
-                                         makeStringStorage(input))
+  return makeDeserializer<TDeserializer>(
+             doc.memoryPool(), reader,
+             makeStringStorage(input, doc.memoryPool()))
       .parse(doc.data(), filter, nestingLimit);
 }
 //
@@ -60,8 +62,9 @@ DeserializationError deserialize(JsonDocument &doc, TStream &input,
                                  NestingLimit nestingLimit, TFilter filter) {
   Reader<TStream> reader(input);
   doc.clear();
-  return makeDeserializer<TDeserializer>(doc.memoryPool(), reader,
-                                         makeStringStorage(input))
+  return makeDeserializer<TDeserializer>(
+             doc.memoryPool(), reader,
+             makeStringStorage(input, doc.memoryPool()))
       .parse(doc.data(), filter, nestingLimit);
 }
 

src/ArduinoJson/Document/JsonDocument.hpp

@@ -48,6 +48,10 @@ class JsonDocument : public Visitable {
     return _pool.size();
   }
 
+  bool overflowed() const {
+    return _pool.overflowed();
+  }
+
   size_t nesting() const {
     return _data.nesting();
   }
@@ -81,6 +85,7 @@ class JsonDocument : public Visitable {
     return _pool;
   }
 
+  // for internal use only
   VariantData& data() {
     return _data;
   }

src/ArduinoJson/Json/JsonDeserializer.hpp

@@ -241,7 +241,7 @@ class JsonDeserializer {
         if (!variant) {
           // Save key in memory pool.
           // This MUST be done before adding the slot.
-          key = _stringStorage.save(_pool);
+          key = _stringStorage.save();
 
           // Allocate slot in object
           VariantSlot *slot = object.addSlot(_pool);
@@ -334,7 +334,7 @@ class JsonDeserializer {
   }
 
   bool parseKey() {
-    _stringStorage.startString(_pool);
+    _stringStorage.startString();
     if (isQuote(current())) {
       return parseQuotedString();
     } else {
@@ -343,10 +343,10 @@ class JsonDeserializer {
   }
 
   bool parseStringValue(VariantData &variant) {
-    _stringStorage.startString(_pool);
+    _stringStorage.startString();
     if (!parseQuotedString())
       return false;
-    const char *value = _stringStorage.save(_pool);
+    const char *value = _stringStorage.save();
     variant.setString(make_not_null(value),
                       typename TStringStorage::storage_policy());
     return true;

src/ArduinoJson/Memory/MemoryPool.hpp

@@ -29,7 +29,8 @@ class MemoryPool {
       : _begin(buf),
         _left(buf),
         _right(buf ? buf + capa : 0),
-        _end(buf ? buf + capa : 0) {
+        _end(buf ? buf + capa : 0),
+        _overflowed(false) {
     ARDUINOJSON_ASSERT(isAligned(_begin));
     ARDUINOJSON_ASSERT(isAligned(_right));
     ARDUINOJSON_ASSERT(isAligned(_end));
@@ -48,6 +49,10 @@ class MemoryPool {
     return size_t(_left - _begin + _end - _right);
   }
 
+  bool overflowed() const {
+    return _overflowed;
+  }
+
   VariantSlot* allocVariant() {
     return allocRight<VariantSlot>();
   }
@@ -91,9 +96,14 @@ class MemoryPool {
     return str;
   }
 
+  void markAsOverflowed() {
+    _overflowed = true;
+  }
+
   void clear() {
     _left = _begin;
     _right = _end;
+    _overflowed = false;
   }
 
   bool canAlloc(size_t bytes) const {
@@ -171,8 +181,10 @@ class MemoryPool {
 #endif
 
   char* allocString(size_t n) {
-    if (!canAlloc(n))
+    if (!canAlloc(n)) {
+      _overflowed = true;
       return 0;
+    }
     char* s = _left;
     _left += n;
     checkInvariants();
@@ -185,13 +197,16 @@ class MemoryPool {
   }
 
   void* allocRight(size_t bytes) {
-    if (!canAlloc(bytes))
+    if (!canAlloc(bytes)) {
+      _overflowed = true;
       return 0;
+    }
     _right -= bytes;
     return _right;
   }
 
   char *_begin, *_left, *_right, *_end;
+  bool _overflowed;
 };
 
 }  // namespace ARDUINOJSON_NAMESPACE

src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp

@@ -239,7 +239,7 @@ class MsgPackDeserializer {
   }
 
   bool readString(const char *&result, size_t n) {
-    _stringStorage.startString(_pool);
+    _stringStorage.startString();
     for (; n; --n) {
       uint8_t c;
       if (!readBytes(c))
@@ -252,7 +252,7 @@ class MsgPackDeserializer {
       return false;
     }
 
-    result = _stringStorage.save(_pool);
+    result = _stringStorage.save();
     return true;
   }
 

src/ArduinoJson/StringStorage/StringCopier.hpp

@@ -10,14 +10,16 @@ namespace ARDUINOJSON_NAMESPACE {
 
 class StringCopier {
  public:
-  void startString(MemoryPool* pool) {
-    pool->getFreeZone(&_ptr, &_capacity);
+  StringCopier(MemoryPool& pool) : _pool(&pool) {}
+
+  void startString() {
+    _pool->getFreeZone(&_ptr, &_capacity);
     _size = 0;
   }
 
-  const char* save(MemoryPool* pool) {
+  const char* save() {
     ARDUINOJSON_ASSERT(_ptr);
-    return pool->saveStringFromFreeZone(_size);
+    return _pool->saveStringFromFreeZone(_size);
   }
 
   void append(const char* s) {
@@ -34,6 +36,7 @@ class StringCopier {
 
     if (_size >= _capacity) {
       _ptr = 0;
+      _pool->markAsOverflowed();
       return;
     }
 
@@ -51,6 +54,7 @@ class StringCopier {
   typedef storage_policies::store_by_copy storage_policy;
 
  private:
+  MemoryPool* _pool;
   char* _ptr;
   size_t _size;
   size_t _capacity;

src/ArduinoJson/StringStorage/StringMover.hpp

@@ -13,11 +13,11 @@ class StringMover {
  public:
   StringMover(char* ptr) : _writePtr(ptr) {}
 
-  void startString(MemoryPool*) {
+  void startString() {
     _startPtr = _writePtr;
   }
 
-  const char* save(MemoryPool*) const {
+  const char* save() const {
     return _startPtr;
   }
 

src/ArduinoJson/StringStorage/StringStorage.hpp

@@ -9,32 +9,15 @@
 
 namespace ARDUINOJSON_NAMESPACE {
 
-template <typename TInput, typename Enable = void>
-struct StringStorage {
-  typedef StringCopier type;
-
-  static type create(TInput&) {
-    return type();
-  }
-};
-
-template <typename TChar>
-struct StringStorage<TChar*,
-                     typename enable_if<!is_const<TChar>::value>::type> {
-  typedef StringMover type;
-
-  static type create(TChar* input) {
-    return type(reinterpret_cast<char*>(input));
-  }
-};
-
 template <typename TInput>
-typename StringStorage<TInput>::type makeStringStorage(TInput& input) {
-  return StringStorage<TInput>::create(input);
+StringCopier makeStringStorage(TInput&, MemoryPool& pool) {
+  return StringCopier(pool);
 }
 
 template <typename TChar>
-typename StringStorage<TChar*>::type makeStringStorage(TChar* input) {
-  return StringStorage<TChar*>::create(input);
+StringMover makeStringStorage(
+    TChar* input, MemoryPool&,
+    typename enable_if<!is_const<TChar>::value>::type* = 0) {
+  return StringMover(reinterpret_cast<char*>(input));
 }
 }  // namespace ARDUINOJSON_NAMESPACE