Skip to content

Commit

Permalink
Fixed serializeJson(doc, String) when allocation fails (fixes #1572)
Browse files Browse the repository at this point in the history
  • Loading branch information
bblanchon committed May 30, 2021
1 parent 3b10afd commit 9bcb409
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 58 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,7 @@ HEAD

* Fixed support for `volatile float` and `volatile double` (issue #1557)
* Fixed error `[Pe070]: incomplete type is not allowed` on IAR (issue #1560)
* Fixed `serializeJson(doc, String)` when allocation fails (issue #1572)

v6.18.0 (2021-05-05)
-------
Expand Down
23 changes: 18 additions & 5 deletions extras/tests/Helpers/api/String.h
Expand Up @@ -9,12 +9,15 @@
// Reproduces Arduino's String class
class String {
public:
String() {}
explicit String(const char* s) : _str(s) {}
String() : _maxCapacity(1024) {}
explicit String(const char* s) : _str(s), _maxCapacity(1024) {}

String& operator+=(const char* rhs) {
_str += rhs;
return *this;
void limitCapacityTo(size_t maxCapacity) {
_maxCapacity = maxCapacity;
}

unsigned char concat(const char* s) {
return concat(s, strlen(s));
}

size_t length() const {
Expand All @@ -34,8 +37,18 @@ class String {
return lhs;
}

protected:
// This function is protected in most Arduino cores
unsigned char concat(const char* s, size_t n) {
if (_str.size() + n > _maxCapacity)
return 0;
_str.append(s, n);
return 1;
}

private:
std::string _str;
size_t _maxCapacity;
};

class StringSumHelper;
Expand Down
135 changes: 89 additions & 46 deletions extras/tests/Misc/StringWriter.cpp
Expand Up @@ -11,95 +11,121 @@
using namespace ARDUINOJSON_NAMESPACE;

template <typename StringWriter>
static size_t print(StringWriter& sb, const char* s) {
return sb.write(reinterpret_cast<const uint8_t*>(s), strlen(s));
static size_t print(StringWriter& writer, const char* s) {
return writer.write(reinterpret_cast<const uint8_t*>(s), strlen(s));
}

template <typename StringWriter, typename String>
void common_tests(StringWriter& sb, const String& output) {
void common_tests(StringWriter& writer, const String& output) {
SECTION("InitialState") {
REQUIRE(std::string("") == output);
}

SECTION("EmptyString") {
REQUIRE(0 == print(sb, ""));
REQUIRE(0 == print(writer, ""));
REQUIRE(std::string("") == output);
}

SECTION("OneString") {
REQUIRE(4 == print(sb, "ABCD"));
REQUIRE(4 == print(writer, "ABCD"));
REQUIRE(std::string("ABCD") == output);
}

SECTION("TwoStrings") {
REQUIRE(4 == print(sb, "ABCD"));
REQUIRE(4 == print(sb, "EFGH"));
REQUIRE(4 == print(writer, "ABCD"));
REQUIRE(4 == print(writer, "EFGH"));
REQUIRE(std::string("ABCDEFGH") == output);
}
}

TEST_CASE("StaticStringWriter") {
char output[20] = {0};
StaticStringWriter sb(output, sizeof(output));
StaticStringWriter writer(output, sizeof(output));

common_tests(sb, static_cast<const char*>(output));
common_tests(writer, static_cast<const char*>(output));

SECTION("OverCapacity") {
REQUIRE(20 == print(sb, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
REQUIRE(0 == print(sb, "ABC"));
REQUIRE(20 == print(writer, "ABCDEFGHIJKLMNOPQRSTUVWXYZ"));
REQUIRE(0 == print(writer, "ABC"));
REQUIRE("ABCDEFGHIJKLMNOPQRST" == std::string(output, 20));
}
}

TEST_CASE("Writer<std::string>") {
std::string output;
Writer<std::string> sb(output);
common_tests(sb, output);
Writer<std::string> writer(output);
common_tests(writer, output);
}

TEST_CASE("Writer<String>") {
::String output;
Writer< ::String> sb(output);

common_tests(sb, output);

SECTION("Writes characters to temporary buffer") {
// accumulate in buffer
sb.write('a');
sb.write('b');
sb.write('c');
REQUIRE(output == "");

// flush when full
sb.write('d');
REQUIRE(output == "abcd");

// flush on destruction
sb.write('e');
sb.~Writer();
REQUIRE(output == "abcde");
Writer< ::String> writer(output);

SECTION("write(char)") {
SECTION("writes to temporary buffer") {
// accumulate in buffer
writer.write('a');
writer.write('b');
writer.write('c');
writer.write('d');
REQUIRE(output == "");

// flush when full
writer.write('e');
REQUIRE(output == "abcd");

// flush on destruction
writer.write('f');
writer.~Writer();
REQUIRE(output == "abcdef");
}

SECTION("returns 1 on success") {
for (int i = 0; i < ARDUINOJSON_STRING_BUFFER_SIZE; i++) {
REQUIRE(writer.write('x') == 1);
}
}

SECTION("returns 0 on error") {
output.limitCapacityTo(1);

REQUIRE(writer.write('a') == 1);
REQUIRE(writer.write('b') == 1);
REQUIRE(writer.write('c') == 1);
REQUIRE(writer.write('d') == 1);
REQUIRE(writer.write('e') == 0);
REQUIRE(writer.write('f') == 0);
}
}

SECTION("Writes strings to temporary buffer") {
// accumulate in buffer
print(sb, "abc");
REQUIRE(output == "");

// flush when full, and continue to accumulate
print(sb, "de");
REQUIRE(output == "abcd");

// flush on destruction
sb.~Writer();
REQUIRE(output == "abcde");
SECTION("write(char*, size_t)") {
SECTION("empty string") {
REQUIRE(0 == print(writer, ""));
writer.flush();
REQUIRE(output == "");
}

SECTION("writes to temporary buffer") {
// accumulate in buffer
print(writer, "abc");
REQUIRE(output == "");

// flush when full, and continue to accumulate
print(writer, "de");
REQUIRE(output == "abcd");

// flush on destruction
writer.~Writer();
REQUIRE(output == "abcde");
}
}
}

TEST_CASE("Writer<custom_string>") {
custom_string output;
Writer<custom_string> sb(output);
Writer<custom_string> writer(output);

REQUIRE(4 == print(sb, "ABCD"));
REQUIRE(4 == print(writer, "ABCD"));
REQUIRE("ABCD" == output);
}

Expand All @@ -116,3 +142,20 @@ TEST_CASE("IsWriteableString") {
REQUIRE(IsWriteableString<std::basic_string<wchar_t> >::value == false);
}
}

TEST_CASE("serializeJson(doc, String)") {
StaticJsonDocument<1024> doc;
doc["hello"] = "world";
::String output;

SECTION("sufficient capacity") {
serializeJson(doc, output);
REQUIRE(output == "{\"hello\":\"world\"}");
}

SECTION("unsufficient capacity") { // issue #1561
output.limitCapacityTo(10);
serializeJson(doc, output);
REQUIRE(output == "{\"hello\"");
}
}
15 changes: 8 additions & 7 deletions src/ArduinoJson/Serialization/Writers/ArduinoStringWriter.hpp
Expand Up @@ -22,10 +22,10 @@ class Writer< ::String, void> {
}

size_t write(uint8_t c) {
ARDUINOJSON_ASSERT(_size < bufferCapacity);
_buffer[_size++] = static_cast<char>(c);
if (_size + 1 >= bufferCapacity)
flush();
if (flush() != 0)
return 0;
_buffer[_size++] = static_cast<char>(c);
return 1;
}

Expand All @@ -36,14 +36,15 @@ class Writer< ::String, void> {
return n;
}

private:
void flush() {
size_t flush() {
ARDUINOJSON_ASSERT(_size < bufferCapacity);
_buffer[_size] = 0;
*_destination += _buffer;
_size = 0;
if (_destination->concat(_buffer))
_size = 0;
return _size;
}

private:
::String *_destination;
char _buffer[bufferCapacity];
size_t _size;
Expand Down

0 comments on commit 9bcb409

Please sign in to comment.