Fixed segmentation fault in StaticJsonBuffer (issue #104)

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

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) {}
 

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
+  // Same with a const char*.
-  JsonObject &parseObject(const String &json, uint8_t nesting = DEFAULT_LIMIT) {
+  // 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

test/Issue104.cpp

@@ -0,0 +1,56 @@
+// Copyright Benoit Blanchon 2014-2015
+// MIT License
+//
+// Arduino JSON library
+// https://github.com/bblanchon/ArduinoJson
+
+#include <gtest/gtest.h>
+#define ARDUINOJSON_ENABLE_STD_STREAM
+#include <ArduinoJson.h>
+
+#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 <int N, typename S>
+void mustSucceedWith(S source) {
+  StaticJsonBuffer<N> jsonBuffer;
+  ASSERT_TRUE(jsonBuffer.parseObject(source).success());
+}
+
+template <int N, typename S>
+void mustFailWith(S source) {
+  StaticJsonBuffer<N> jsonBuffer;
+  ASSERT_FALSE(jsonBuffer.parseObject(source).success());
+}
+
+TEST(Issue104, CharPtrSucceeds) {
+  char source[] = SOURCE;
+  mustSucceedWith<SIZE_WITHOUT_COPY, char *>(source);
+}
+
+TEST(Issue104, CharPtrFails) {
+  char source[] = SOURCE;
+  mustFailWith<SIZE_WITHOUT_COPY - 1, char *>(source);
+}
+
+TEST(Issue104, ConstCharPtrSucceeds) {
+  mustSucceedWith<SIZE_WITH_COPY, const char *>(SOURCE);
+}
+
+TEST(Issue104, ConstCharPtrFails) {
+  mustFailWith<SIZE_WITH_COPY - 1, const char *>(SOURCE);
+}
+
+TEST(Issue104, StringSucceeds) {
+  mustSucceedWith<SIZE_WITH_COPY, const String &>(SOURCE);
+}
+
+TEST(Issue104, StringFails) {
+  mustFailWith<SIZE_WITH_COPY - 1, const String &>(SOURCE);
+}
+
+TEST(Issue104, TooSmallForStrDup) {
+  mustFailWith<sizeof(SOURCE) - 1, const char *>(SOURCE);
+}

test/StaticJsonBuffer_ParseArray_Tests.cpp

@@ -69,4 +69,12 @@ TEST_F(StaticJsonBuffer_ParseArray_Tests,
   with(bufferOfRightSize);
   whenInputIs("[{}]");
   parseMustSucceed();
-}
+}
+
+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());
+}

test/StaticJsonBuffer_ParseObject_Tests.cpp

@@ -70,4 +70,12 @@ TEST_F(StaticJsonBuffer_ParseObject_Tests,
   with(bufferOfRightSize);
   whenInputIs("{\"a\":[]}");
   parseMustSucceed();
-}
+}
+
+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());
+}