From 43b2e2e774f49d9b12ad3bc4711d16af97638933 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Sat, 20 Nov 2021 20:29:09 +0100 Subject: [PATCH] Append terminator in `saveStringFromFreeZone()` --- extras/tests/JsonDeserializer/string.cpp | 18 +++++++++++++ extras/tests/MemoryPool/StringCopier.cpp | 16 +++++++++--- extras/tests/Misc/Utf8.cpp | 1 - extras/tests/Misc/printable.cpp | 2 ++ src/ArduinoJson/Json/JsonDeserializer.hpp | 4 --- src/ArduinoJson/Memory/MemoryPool.hpp | 3 ++- .../MsgPack/MsgPackDeserializer.hpp | 1 - .../StringStorage/StringCopier.hpp | 26 +++++++++++-------- src/ArduinoJson/StringStorage/StringMover.hpp | 9 ++++++- src/ArduinoJson/Variant/ConverterImpl.hpp | 3 +-- 10 files changed, 58 insertions(+), 25 deletions(-) diff --git a/extras/tests/JsonDeserializer/string.cpp b/extras/tests/JsonDeserializer/string.cpp index 7b5b7c01..b5aaa39f 100644 --- a/extras/tests/JsonDeserializer/string.cpp +++ b/extras/tests/JsonDeserializer/string.cpp @@ -113,3 +113,21 @@ TEST_CASE("Not enough room to save the key") { DeserializationError::NoMemory); // fails in the second string } } + +TEST_CASE("Empty memory pool") { + // NOLINTNEXTLINE(clang-analyzer-optin.portability.UnixAPI) + DynamicJsonDocument doc(0); + + SECTION("Input is const char*") { + REQUIRE(deserializeJson(doc, "\"hello\"") == + DeserializationError::NoMemory); + REQUIRE(deserializeJson(doc, "\"\"") == DeserializationError::NoMemory); + } + + SECTION("Input is const char*") { + char hello[] = "\"hello\""; + REQUIRE(deserializeJson(doc, hello) == DeserializationError::Ok); + char empty[] = "\"hello\""; + REQUIRE(deserializeJson(doc, empty) == DeserializationError::Ok); + } +} diff --git a/extras/tests/MemoryPool/StringCopier.cpp b/extras/tests/MemoryPool/StringCopier.cpp index 4d3fdba7..ba1e1b84 100644 --- a/extras/tests/MemoryPool/StringCopier.cpp +++ b/extras/tests/MemoryPool/StringCopier.cpp @@ -11,12 +11,11 @@ TEST_CASE("StringCopier") { char buffer[4096]; SECTION("Works when buffer is big enough") { - MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(6))); + MemoryPool pool(buffer, addPadding(JSON_STRING_SIZE(5))); StringCopier str(pool); str.startString(); str.append("hello"); - str.append('\0'); REQUIRE(str.isValid() == true); REQUIRE(std::string(str.str()) == "hello"); @@ -31,6 +30,7 @@ TEST_CASE("StringCopier") { str.append("hello world!"); REQUIRE(str.isValid() == false); + REQUIRE(pool.overflowed() == true); } SECTION("Increases size of memory pool") { @@ -38,10 +38,19 @@ TEST_CASE("StringCopier") { StringCopier str(pool); str.startString(); - str.append('h'); str.save(); REQUIRE(1 == pool.size()); + REQUIRE(pool.overflowed() == false); + } + + SECTION("Works when memory pool is 0 bytes") { + MemoryPool pool(buffer, 0); + StringCopier str(pool); + + str.startString(); + REQUIRE(str.isValid() == false); + REQUIRE(pool.overflowed() == true); } } @@ -49,7 +58,6 @@ static const char* addStringToPool(MemoryPool& pool, const char* s) { StringCopier str(pool); str.startString(); str.append(s); - str.append('\0'); return str.save().c_str(); } diff --git a/extras/tests/Misc/Utf8.cpp b/extras/tests/Misc/Utf8.cpp index 56006c1a..ca29fe6e 100644 --- a/extras/tests/Misc/Utf8.cpp +++ b/extras/tests/Misc/Utf8.cpp @@ -18,7 +18,6 @@ static void testCodepoint(uint32_t codepoint, std::string expected) { CAPTURE(codepoint); Utf8::encodeCodepoint(codepoint, str); - str.append('\0'); REQUIRE(str.str().c_str() == expected); } diff --git a/extras/tests/Misc/printable.cpp b/extras/tests/Misc/printable.cpp index 27ac15e6..a1b6ec35 100644 --- a/extras/tests/Misc/printable.cpp +++ b/extras/tests/Misc/printable.cpp @@ -60,6 +60,7 @@ TEST_CASE("Printable") { CHECK(printable.totalBytesWritten() == 7); CHECK(doc.overflowed() == false); CHECK(doc.memoryUsage() == 8); + CHECK(doc.as().memoryUsage() == 8); } SECTION("Via Print::write(const char* size_t)") { @@ -69,6 +70,7 @@ TEST_CASE("Printable") { CHECK(printable.totalBytesWritten() == 7); CHECK(doc.overflowed() == false); CHECK(doc.memoryUsage() == 8); + CHECK(doc.as().memoryUsage() == 8); } } diff --git a/src/ArduinoJson/Json/JsonDeserializer.hpp b/src/ArduinoJson/Json/JsonDeserializer.hpp index d719e10e..99caf180 100644 --- a/src/ArduinoJson/Json/JsonDeserializer.hpp +++ b/src/ArduinoJson/Json/JsonDeserializer.hpp @@ -401,8 +401,6 @@ class JsonDeserializer { _stringStorage.append(c); } - _stringStorage.append('\0'); - if (!_stringStorage.isValid()) { _error = DeserializationError::NoMemory; return false; @@ -426,8 +424,6 @@ class JsonDeserializer { return false; } - _stringStorage.append('\0'); - if (!_stringStorage.isValid()) { _error = DeserializationError::NoMemory; return false; diff --git a/src/ArduinoJson/Memory/MemoryPool.hpp b/src/ArduinoJson/Memory/MemoryPool.hpp index 04e5b2d2..c7fa33b7 100644 --- a/src/ArduinoJson/Memory/MemoryPool.hpp +++ b/src/ArduinoJson/Memory/MemoryPool.hpp @@ -87,13 +87,14 @@ class MemoryPool { const char* saveStringFromFreeZone(size_t len) { #if ARDUINOJSON_ENABLE_STRING_DEDUPLICATION - const char* dup = findString(adaptString(_left)); + const char* dup = findString(adaptString(_left, len)); if (dup) return dup; #endif const char* str = _left; _left += len; + *_left++ = 0; checkInvariants(); return str; } diff --git a/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp b/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp index f4bfe2aa..00009e5e 100644 --- a/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp +++ b/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp @@ -343,7 +343,6 @@ class MsgPackDeserializer { return false; _stringStorage.append(static_cast(c)); } - _stringStorage.append('\0'); if (!_stringStorage.isValid()) { _error = DeserializationError::NoMemory; return false; diff --git a/src/ArduinoJson/StringStorage/StringCopier.hpp b/src/ArduinoJson/StringStorage/StringCopier.hpp index 3666a98d..8912116a 100644 --- a/src/ArduinoJson/StringStorage/StringCopier.hpp +++ b/src/ArduinoJson/StringStorage/StringCopier.hpp @@ -17,10 +17,13 @@ class StringCopier { void startString() { _pool->getFreeZone(&_ptr, &_capacity); _size = 0; + if (_capacity == 0) + _pool->markAsOverflowed(); } string_type save() { ARDUINOJSON_ASSERT(_ptr); + ARDUINOJSON_ASSERT(_size < _capacity); // needs room for the terminator return _pool->saveStringFromFreeZone(_size); } @@ -33,23 +36,24 @@ class StringCopier { } void append(char c) { - if (!_ptr) - return; - - if (_size >= _capacity) { - _ptr = 0; + if (_size + 1 < _capacity) + _ptr[_size++] = c; + else _pool->markAsOverflowed(); - return; - } - - _ptr[_size++] = c; } - bool isValid() { - return _ptr != 0; + bool isValid() const { + return !_pool->overflowed(); + } + + size_t size() const { + return _size; } string_type str() const { + ARDUINOJSON_ASSERT(_ptr); + ARDUINOJSON_ASSERT(_size < _capacity); + _ptr[_size] = 0; return _ptr; } diff --git a/src/ArduinoJson/StringStorage/StringMover.hpp b/src/ArduinoJson/StringStorage/StringMover.hpp index b2b8da8a..cea15b49 100644 --- a/src/ArduinoJson/StringStorage/StringMover.hpp +++ b/src/ArduinoJson/StringStorage/StringMover.hpp @@ -20,7 +20,10 @@ class StringMover { } FORCE_INLINE string_type save() { - return _startPtr; + _writePtr[0] = 0; // terminator + string_type s = str(); + _writePtr++; + return s; } void append(char c) { @@ -35,6 +38,10 @@ class StringMover { return string_type(_startPtr); } + size_t size() const { + return size_t(_writePtr - _startPtr); + } + private: char* _writePtr; char* _startPtr; diff --git a/src/ArduinoJson/Variant/ConverterImpl.hpp b/src/ArduinoJson/Variant/ConverterImpl.hpp index fe7e3dcf..170a8d44 100644 --- a/src/ArduinoJson/Variant/ConverterImpl.hpp +++ b/src/ArduinoJson/Variant/ConverterImpl.hpp @@ -204,8 +204,7 @@ class MemoryPoolPrint : public Print { } CopiedString str() { - _string[_size++] = 0; - ARDUINOJSON_ASSERT(_size <= _capacity); + ARDUINOJSON_ASSERT(_size < _capacity); return _pool->saveStringFromFreeZone(_size); }