diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ad1c835..a7fbc997 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ v5.0.2 ------ * Fixed Clang warning "register specifier is deprecated" (issue #102) +* Fixed segmentation fault in `parseObject(String)` and `parseArray(String)`, when the + `StaticJsonBuffer` is too small to hold a copy of the string (issue #104) * Fixed compilation on Visual Studio 2010 and 2012 (issue #107) v5.0.1 diff --git a/include/ArduinoJson/Internals/JsonParser.hpp b/include/ArduinoJson/Internals/JsonParser.hpp index 085b75df..69ea2503 100644 --- a/include/ArduinoJson/Internals/JsonParser.hpp +++ b/include/ArduinoJson/Internals/JsonParser.hpp @@ -19,7 +19,7 @@ class JsonParser { public: JsonParser(JsonBuffer *buffer, char *json, uint8_t nestingLimit) : _buffer(buffer), - _readPtr(json), + _readPtr(json ? json : ""), _writePtr(json), _nestingLimit(nestingLimit) {} diff --git a/include/ArduinoJson/JsonBuffer.hpp b/include/ArduinoJson/JsonBuffer.hpp index 73c87374..c0614534 100644 --- a/include/ArduinoJson/JsonBuffer.hpp +++ b/include/ArduinoJson/JsonBuffer.hpp @@ -59,9 +59,15 @@ class JsonBuffer { // allocation fails. JsonArray &parseArray(char *json, uint8_t nestingLimit = DEFAULT_LIMIT); + // Same with a const char*. + // With this overload, the JsonBuffer will make a copy of the string + JsonArray &parseArray(const char *json, uint8_t nesting = DEFAULT_LIMIT) { + return parseArray(strdup(json), nesting); + } + // Same as above with a String class JsonArray &parseArray(const String &json, uint8_t nesting = DEFAULT_LIMIT) { - return parseArray(strdup(json), nesting); + return parseArray(json.c_str(), nesting); } // Allocates and populate a JsonObject from a JSON string. @@ -76,20 +82,30 @@ class JsonBuffer { // allocation fails. JsonObject &parseObject(char *json, uint8_t nestingLimit = DEFAULT_LIMIT); - // Same as above with a String class - JsonObject &parseObject(const String &json, uint8_t nesting = DEFAULT_LIMIT) { + // Same with a const char*. + // With this overload, the JsonBuffer will make a copy of the string + JsonObject &parseObject(const char *json, uint8_t nesting = DEFAULT_LIMIT) { return parseObject(strdup(json), nesting); } + // Same as above with a String class + JsonObject &parseObject(const String &json, uint8_t nesting = DEFAULT_LIMIT) { + return parseObject(json.c_str(), nesting); + } + // Duplicate a string - char *strdup(const char *src) { return strdup(src, strlen(src)); } + char *strdup(const char *src) { + return src ? strdup(src, strlen(src)) : NULL; + } char *strdup(const String &src) { return strdup(src.c_str(), src.length()); } - char *strdup(const char *, size_t); // Allocates n bytes in the JsonBuffer. // Return a pointer to the allocated memory or NULL if allocation fails. virtual void *alloc(size_t size) = 0; + private: + char *strdup(const char *, size_t); + // Default value of nesting limit of parseArray() and parseObject(). // // The nesting limit is a contain on the level of nesting allowed in the JSON diff --git a/test/Issue104.cpp b/test/Issue104.cpp new file mode 100644 index 00000000..154727f8 --- /dev/null +++ b/test/Issue104.cpp @@ -0,0 +1,56 @@ +// Copyright Benoit Blanchon 2014-2015 +// MIT License +// +// Arduino JSON library +// https://github.com/bblanchon/ArduinoJson + +#include +#define ARDUINOJSON_ENABLE_STD_STREAM +#include + +#define SOURCE "{\"mqtt.host\":\"mqtt.test.com\",\"mqtt.port\":1883}" + +#define SIZE_WITH_COPY (sizeof(SOURCE) + JSON_OBJECT_SIZE(2)) +#define SIZE_WITHOUT_COPY (JSON_OBJECT_SIZE(2)) + +template +void mustSucceedWith(S source) { + StaticJsonBuffer jsonBuffer; + ASSERT_TRUE(jsonBuffer.parseObject(source).success()); +} + +template +void mustFailWith(S source) { + StaticJsonBuffer jsonBuffer; + ASSERT_FALSE(jsonBuffer.parseObject(source).success()); +} + +TEST(Issue104, CharPtrSucceeds) { + char source[] = SOURCE; + mustSucceedWith(source); +} + +TEST(Issue104, CharPtrFails) { + char source[] = SOURCE; + mustFailWith(source); +} + +TEST(Issue104, ConstCharPtrSucceeds) { + mustSucceedWith(SOURCE); +} + +TEST(Issue104, ConstCharPtrFails) { + mustFailWith(SOURCE); +} + +TEST(Issue104, StringSucceeds) { + mustSucceedWith(SOURCE); +} + +TEST(Issue104, StringFails) { + mustFailWith(SOURCE); +} + +TEST(Issue104, TooSmallForStrDup) { + mustFailWith(SOURCE); +} diff --git a/test/StaticJsonBuffer_ParseArray_Tests.cpp b/test/StaticJsonBuffer_ParseArray_Tests.cpp index ee170471..0aa858f4 100644 --- a/test/StaticJsonBuffer_ParseArray_Tests.cpp +++ b/test/StaticJsonBuffer_ParseArray_Tests.cpp @@ -69,4 +69,12 @@ TEST_F(StaticJsonBuffer_ParseArray_Tests, with(bufferOfRightSize); whenInputIs("[{}]"); parseMustSucceed(); -} \ No newline at end of file +} + +TEST_F(StaticJsonBuffer_ParseArray_Tests, CharPtrNull) { + ASSERT_FALSE(StaticJsonBuffer<100>().parseArray((char*)0).success()); +} + +TEST_F(StaticJsonBuffer_ParseArray_Tests, ConstCharPtrNull) { + ASSERT_FALSE(StaticJsonBuffer<100>().parseArray((const char*)0).success()); +} diff --git a/test/StaticJsonBuffer_ParseObject_Tests.cpp b/test/StaticJsonBuffer_ParseObject_Tests.cpp index 342cdb2b..bae340ed 100644 --- a/test/StaticJsonBuffer_ParseObject_Tests.cpp +++ b/test/StaticJsonBuffer_ParseObject_Tests.cpp @@ -70,4 +70,12 @@ TEST_F(StaticJsonBuffer_ParseObject_Tests, with(bufferOfRightSize); whenInputIs("{\"a\":[]}"); parseMustSucceed(); -} \ No newline at end of file +} + +TEST_F(StaticJsonBuffer_ParseObject_Tests, CharPtrNull) { + ASSERT_FALSE(StaticJsonBuffer<100>().parseObject((char*)0).success()); +} + +TEST_F(StaticJsonBuffer_ParseObject_Tests, ConstCharPtrNull) { + ASSERT_FALSE(StaticJsonBuffer<100>().parseObject((const char*)0).success()); +}