Fixed ignored Stream timeout and made sure we don't read more that …

…necessary (issue #422)
bblanchon · Jan 22, 2017 · cc8c047 · cc8c047
1 parent fb55407
commit cc8c047
Show file tree

Hide file tree

Showing 10 changed files with 89 additions and 71 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,8 @@ HEAD
 ----
 
 * Fixed parsing of comments (issue #421)
+* Fixed ignored `Stream` timeout (issue #422)
+* Made sure we don't read more that necessary (issue #422)
 
 v5.8.1
 ------

diff --git a/include/ArduinoJson/Deserialization/JsonParser.hpp b/include/ArduinoJson/Deserialization/JsonParser.hpp
@@ -9,7 +9,6 @@
 
 #include "../JsonBuffer.hpp"
 #include "../JsonVariant.hpp"
-#include "StringReader.hpp"
 #include "StringWriter.hpp"
 
 namespace ArduinoJson {
@@ -74,23 +73,23 @@ class JsonParser {
 
 template <typename TJsonBuffer, typename TString>
 struct JsonParserBuilder {
-  typedef typename Internals::StringTraits<TString>::Iterator InputIterator;
-  typedef JsonParser<StringReader<InputIterator>, TJsonBuffer &> TParser;
+  typedef typename Internals::StringTraits<TString>::Reader InputReader;
+  typedef JsonParser<InputReader, TJsonBuffer &> TParser;
 
   static TParser makeParser(TJsonBuffer *buffer, TString &json,
                             uint8_t nestingLimit) {
-    return TParser(buffer, InputIterator(json), *buffer, nestingLimit);
+    return TParser(buffer, InputReader(json), *buffer, nestingLimit);
   }
 };
 
 template <typename TJsonBuffer>
 struct JsonParserBuilder<TJsonBuffer, char *> {
-  typedef typename Internals::StringTraits<char *>::Iterator InputIterator;
-  typedef JsonParser<StringReader<InputIterator>, StringWriter> TParser;
+  typedef typename Internals::StringTraits<char *>::Reader InputReader;
+  typedef JsonParser<InputReader, StringWriter> TParser;
 
   static TParser makeParser(TJsonBuffer *buffer, char *json,
                             uint8_t nestingLimit) {
-    return TParser(buffer, InputIterator(json), json, nestingLimit);
+    return TParser(buffer, InputReader(json), json, nestingLimit);
   }
 };
 

diff --git a/include/ArduinoJson/Deserialization/JsonParserImpl.hpp b/include/ArduinoJson/Deserialization/JsonParserImpl.hpp
@@ -16,7 +16,6 @@ inline bool ArduinoJson::Internals::JsonParser<TReader, TWriter>::eat(
   skipSpacesAndComments(reader);
   if (reader.current() != charToSkip) return false;
   reader.move();
-  skipSpacesAndComments(reader);
   return true;
 }
 
@@ -148,6 +147,7 @@ ArduinoJson::Internals::JsonParser<TReader, TWriter>::parseString() {
   typename TypeTraits::RemoveReference<TWriter>::type::String str =
       _writer.startString();
 
+  skipSpacesAndComments(_reader);
   char c = _reader.current();
 
   if (isQuote(c)) {  // quotes

diff --git a/include/ArduinoJson/Deserialization/StringReader.hpp b/include/ArduinoJson/Deserialization/StringReader.hpp
diff --git a/include/ArduinoJson/StringTraits/ArduinoStream.hpp b/include/ArduinoJson/StringTraits/ArduinoStream.hpp
@@ -17,15 +17,35 @@ namespace ArduinoJson {
 namespace Internals {
 
 struct ArduinoStreamTraits {
-  class Iterator {
+  class Reader {
     Stream& _stream;
+    char _current, _next;
 
    public:
-    Iterator(Stream& stream) : _stream(stream) {}
+    Reader(Stream& stream) : _stream(stream), _current(0), _next(0) {}
+
+    void move() {
+      _current = _next;
+      _next = 0;
+    }
+
+    char current() {
+      if (!_current) _current = read();
+      return _current;
+    }
 
     char next() {
-      int n = _stream.read();
-      return n >= 0 ? static_cast<char>(n) : '\0';
+      // assumes that current() has been called
+      if (!_next) _next = read();
+      return _next;
+    }
+
+   private:
+    char read() {
+      // don't use _stream.read() as it ignores the timeout
+      char c = 0;
+      _stream.readBytes(&c, 1);
+      return c;
     }
   };
 };

diff --git a/include/ArduinoJson/StringTraits/CharPointer.hpp b/include/ArduinoJson/StringTraits/CharPointer.hpp
@@ -11,16 +11,22 @@ namespace ArduinoJson {
 namespace Internals {
 
 struct CharPointerTraits {
-  class Iterator {
+  class Reader {
     const char* _ptr;
 
    public:
-    Iterator(const char* ptr) : _ptr(ptr ? ptr : "") {}
+    Reader(const char* ptr) : _ptr(ptr ? ptr : "") {}
 
-    char next() {
-      char c = *_ptr;
-      if (c) ++_ptr;
-      return c;
+    void move() {
+      ++_ptr;
+    }
+
+    char current() const {
+      return _ptr[0];
+    }
+
+    char next() const {
+      return _ptr[1];
     }
   };
 

diff --git a/include/ArduinoJson/StringTraits/FlashString.hpp b/include/ArduinoJson/StringTraits/FlashString.hpp
@@ -11,15 +11,23 @@ namespace ArduinoJson {
 namespace Internals {
 template <>
 struct StringTraits<const __FlashStringHelper*, void> {
-  class Iterator {
+  class Reader {
     const char* _ptr;
 
    public:
-    Iterator(const __FlashStringHelper* ptr)
+    Reader(const __FlashStringHelper* ptr)
         : _ptr(reinterpret_cast<const char*>(ptr)) {}
 
-    char next() {
-      return pgm_read_byte_near(_ptr++);
+    void move() {
+      _ptr++;
+    }
+
+    char current() const {
+      return pgm_read_byte_near(_ptr);
+    }
+
+    char next() const {
+      return pgm_read_byte_near(_ptr + 1);
     }
   };
 

diff --git a/include/ArduinoJson/StringTraits/StdStream.hpp b/include/ArduinoJson/StringTraits/StdStream.hpp
@@ -16,18 +16,35 @@ namespace ArduinoJson {
 namespace Internals {
 
 struct StdStreamTraits {
-  class Iterator {
+  class Reader {
     std::istream& _stream;
+    char _current, _next;
 
    public:
-    Iterator(std::istream& stream) : _stream(stream) {}
+    Reader(std::istream& stream) : _stream(stream), _current(0), _next(0) {}
+
+    void move() {
+      _current = _next;
+      _next = 0;
+    }
+
+    char current() {
+      if (!_current) _current = read();
+      return _current;
+    }
 
     char next() {
-      return _stream.eof() ? '\0' : static_cast<char>(_stream.get());
+      // assumes that current() has been called
+      if (!_next) _next = read();
+      return _next;
     }
 
    private:
-    Iterator& operator=(const Iterator&);  // Visual Studio C4512
+    Reader& operator=(const Reader&);  // Visual Studio C4512
+
+    char read() {
+      return _stream.eof() ? '\0' : static_cast<char>(_stream.get());
+    }
   };
 };
 

diff --git a/include/ArduinoJson/StringTraits/StdString.hpp b/include/ArduinoJson/StringTraits/StdString.hpp
@@ -29,8 +29,8 @@ struct StdStringTraits {
     return static_cast<char*>(dup);
   }
 
-  struct Iterator : CharPointerTraits::Iterator {
-    Iterator(const TString& str) : CharPointerTraits::Iterator(str.c_str()) {}
+  struct Reader : CharPointerTraits::Reader {
+    Reader(const TString& str) : CharPointerTraits::Reader(str.c_str()) {}
   };
 
   static bool equals(const TString& str, const char* expected) {

diff --git a/test/StdStream_Tests.cpp b/test/StdStream_Tests.cpp
@@ -60,7 +60,7 @@ TEST(StdStream_Tests, JsonArraySubscript) {
 }
 
 TEST(StdStream_Tests, ParseArray) {
-  std::istringstream json("[42]");
+  std::istringstream json(" [ 42 /* comment */ ] ");
   DynamicJsonBuffer jsonBuffer;
   JsonArray& arr = jsonBuffer.parseArray(json);
   ASSERT_TRUE(arr.success());
@@ -69,10 +69,17 @@ TEST(StdStream_Tests, ParseArray) {
 }
 
 TEST(StdStream_Tests, ParseObject) {
-  std::istringstream json("{hello:world}");
+  std::istringstream json(" { hello : world // comment\n }");
   DynamicJsonBuffer jsonBuffer;
   JsonObject& obj = jsonBuffer.parseObject(json);
   ASSERT_TRUE(obj.success());
   ASSERT_EQ(1, obj.size());
   ASSERT_STREQ("world", obj["hello"]);
 }
+
+TEST(StdStream_Tests, ShouldNotReadPastTheEnd) {
+  std::istringstream json("{}123");
+  DynamicJsonBuffer jsonBuffer;
+  jsonBuffer.parseObject(json);
+  ASSERT_EQ('1', json.get());
+}