From cc8c0472cae5a0d40c78a6ca8775f71dd4c195a0 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Sun, 22 Jan 2017 10:31:05 +0100 Subject: [PATCH] Fixed ignored `Stream` timeout and made sure we don't read more that necessary (issue #422) --- CHANGELOG.md | 2 + .../Deserialization/JsonParser.hpp | 13 +++--- .../Deserialization/JsonParserImpl.hpp | 2 +- .../Deserialization/StringReader.hpp | 41 ------------------- .../StringTraits/ArduinoStream.hpp | 28 +++++++++++-- .../ArduinoJson/StringTraits/CharPointer.hpp | 18 +++++--- .../ArduinoJson/StringTraits/FlashString.hpp | 16 ++++++-- .../ArduinoJson/StringTraits/StdStream.hpp | 25 +++++++++-- .../ArduinoJson/StringTraits/StdString.hpp | 4 +- test/StdStream_Tests.cpp | 11 ++++- 10 files changed, 89 insertions(+), 71 deletions(-) delete mode 100644 include/ArduinoJson/Deserialization/StringReader.hpp diff --git a/CHANGELOG.md b/CHANGELOG.md index eb99e2d2..6749e265 100644 --- 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 index 676cdf8b..ef8a428e 100644 --- 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 struct JsonParserBuilder { - typedef typename Internals::StringTraits::Iterator InputIterator; - typedef JsonParser, TJsonBuffer &> TParser; + typedef typename Internals::StringTraits::Reader InputReader; + typedef JsonParser 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 struct JsonParserBuilder { - typedef typename Internals::StringTraits::Iterator InputIterator; - typedef JsonParser, StringWriter> TParser; + typedef typename Internals::StringTraits::Reader InputReader; + typedef JsonParser 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 index 260f0b84..f1c7d615 100644 --- a/include/ArduinoJson/Deserialization/JsonParserImpl.hpp +++ b/include/ArduinoJson/Deserialization/JsonParserImpl.hpp @@ -16,7 +16,6 @@ inline bool ArduinoJson::Internals::JsonParser::eat( skipSpacesAndComments(reader); if (reader.current() != charToSkip) return false; reader.move(); - skipSpacesAndComments(reader); return true; } @@ -148,6 +147,7 @@ ArduinoJson::Internals::JsonParser::parseString() { typename TypeTraits::RemoveReference::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 deleted file mode 100644 index c45eea27..00000000 --- a/include/ArduinoJson/Deserialization/StringReader.hpp +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright Benoit Blanchon 2014-2017 -// MIT License -// -// Arduino JSON library -// https://github.com/bblanchon/ArduinoJson -// If you like this project, please add a star! - -#pragma once - -namespace ArduinoJson { -namespace Internals { - -// Parse JSON string to create JsonArrays and JsonObjects -// This internal class is not indended to be used directly. -// Instead, use JsonBuffer.parseArray() or .parseObject() -template -class StringReader { - TIterator _input; - char _current, _next; - - public: - StringReader(const TIterator& input) : _input(input) { - _current = _input.next(); - _next = _input.next(); - } - - void move() { - _current = _next; - _next = _input.next(); - } - - char current() const { - return _current; - } - - char next() const { - return _next; - } -}; -} -} diff --git a/include/ArduinoJson/StringTraits/ArduinoStream.hpp b/include/ArduinoJson/StringTraits/ArduinoStream.hpp index 9b27f56d..aa59dca4 100644 --- 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(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 index e5d0efff..fde25faa 100644 --- 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 index 6c44ee97..3da0ff81 100644 --- a/include/ArduinoJson/StringTraits/FlashString.hpp +++ b/include/ArduinoJson/StringTraits/FlashString.hpp @@ -11,15 +11,23 @@ namespace ArduinoJson { namespace Internals { template <> struct StringTraits { - class Iterator { + class Reader { const char* _ptr; public: - Iterator(const __FlashStringHelper* ptr) + Reader(const __FlashStringHelper* ptr) : _ptr(reinterpret_cast(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 index d3614cd1..50fceb92 100644 --- 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(_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(_stream.get()); + } }; }; diff --git a/include/ArduinoJson/StringTraits/StdString.hpp b/include/ArduinoJson/StringTraits/StdString.hpp index 1d206c7f..613b0272 100644 --- a/include/ArduinoJson/StringTraits/StdString.hpp +++ b/include/ArduinoJson/StringTraits/StdString.hpp @@ -29,8 +29,8 @@ struct StdStringTraits { return static_cast(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 index 9c25bb05..c41a45a7 100644 --- 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()); +}