diff --git a/CHANGELOG.md b/CHANGELOG.md index f5aaf8dd..116e070b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ HEAD * Fixed `deserializeJson()` silently accepting a `Stream*` (issue #978) * Fixed invalid result from `operator|` (issue #981) +* Made `deserializeJson()` more picky about trailing characters (issue #980) > ### BREAKING CHANGE > diff --git a/src/ArduinoJson/Deserialization/ArduinoStreamReader.hpp b/src/ArduinoJson/Deserialization/ArduinoStreamReader.hpp index e3591c98..0551bd41 100644 --- a/src/ArduinoJson/Deserialization/ArduinoStreamReader.hpp +++ b/src/ArduinoJson/Deserialization/ArduinoStreamReader.hpp @@ -12,22 +12,14 @@ namespace ARDUINOJSON_NAMESPACE { struct ArduinoStreamReader { Stream& _stream; - char _current; - bool _ended; public: - explicit ArduinoStreamReader(Stream& stream) - : _stream(stream), _current(0), _ended(false) {} + explicit ArduinoStreamReader(Stream& stream) : _stream(stream) {} - char read() { + int read() { // don't use _stream.read() as it ignores the timeout - char c = 0; - _ended = _stream.readBytes(&c, 1) == 0; - return c; - } - - bool ended() const { - return _ended; + uint8_t c; + return _stream.readBytes(&c, 1) ? c : -1; } }; diff --git a/src/ArduinoJson/Deserialization/CharPointerReader.hpp b/src/ArduinoJson/Deserialization/CharPointerReader.hpp index b48cd921..2de68d1a 100644 --- a/src/ArduinoJson/Deserialization/CharPointerReader.hpp +++ b/src/ArduinoJson/Deserialization/CharPointerReader.hpp @@ -23,13 +23,8 @@ class UnsafeCharPointerReader { explicit UnsafeCharPointerReader(const char* ptr) : _ptr(ptr ? ptr : reinterpret_cast("")) {} - char read() { - return static_cast(*_ptr++); - } - - bool ended() const { - // we cannot know, that's why it's unsafe - return false; + int read() { + return static_cast(*_ptr++); } }; @@ -41,12 +36,11 @@ class SafeCharPointerReader { explicit SafeCharPointerReader(const char* ptr, size_t len) : _ptr(ptr ? ptr : reinterpret_cast("")), _end(_ptr + len) {} - char read() { - return static_cast(*_ptr++); - } - - bool ended() const { - return _ptr == _end; + int read() { + if (_ptr < _end) + return static_cast(*_ptr++); + else + return -1; } }; diff --git a/src/ArduinoJson/Deserialization/FlashStringReader.hpp b/src/ArduinoJson/Deserialization/FlashStringReader.hpp index 015b4806..e8066ef6 100644 --- a/src/ArduinoJson/Deserialization/FlashStringReader.hpp +++ b/src/ArduinoJson/Deserialization/FlashStringReader.hpp @@ -14,14 +14,9 @@ class UnsafeFlashStringReader { explicit UnsafeFlashStringReader(const __FlashStringHelper* ptr) : _ptr(reinterpret_cast(ptr)) {} - char read() { + int read() { return pgm_read_byte_near(_ptr++); } - - bool ended() const { - // this reader cannot detect the end - return false; - } }; class SafeFlashStringReader { @@ -32,12 +27,11 @@ class SafeFlashStringReader { explicit SafeFlashStringReader(const __FlashStringHelper* ptr, size_t size) : _ptr(reinterpret_cast(ptr)), _end(_ptr + size) {} - char read() { - return pgm_read_byte_near(_ptr++); - } - - bool ended() const { - return _ptr == _end; + int read() { + if (_ptr < _end) + return pgm_read_byte_near(_ptr++); + else + return -1; } }; diff --git a/src/ArduinoJson/Deserialization/IteratorReader.hpp b/src/ArduinoJson/Deserialization/IteratorReader.hpp index a8d425da..f2605e48 100644 --- a/src/ArduinoJson/Deserialization/IteratorReader.hpp +++ b/src/ArduinoJson/Deserialization/IteratorReader.hpp @@ -14,12 +14,11 @@ class IteratorReader { explicit IteratorReader(TIterator begin, TIterator end) : _ptr(begin), _end(end) {} - bool ended() const { - return _ptr == _end; - } - - char read() { - return char(*_ptr++); + int read() { + if (_ptr < _end) + return static_cast(*_ptr++); + else + return -1; } }; diff --git a/src/ArduinoJson/Deserialization/StdStreamReader.hpp b/src/ArduinoJson/Deserialization/StdStreamReader.hpp index b1187c11..8996a77e 100644 --- a/src/ArduinoJson/Deserialization/StdStreamReader.hpp +++ b/src/ArduinoJson/Deserialization/StdStreamReader.hpp @@ -18,12 +18,8 @@ class StdStreamReader { explicit StdStreamReader(std::istream& stream) : _stream(stream), _current(0) {} - bool ended() const { - return _stream.eof(); - } - - char read() { - return static_cast(_stream.get()); + int read() { + return _stream.get(); } private: diff --git a/src/ArduinoJson/Json/JsonDeserializer.hpp b/src/ArduinoJson/Json/JsonDeserializer.hpp index ba9ab03c..e9eccedd 100644 --- a/src/ArduinoJson/Json/JsonDeserializer.hpp +++ b/src/ArduinoJson/Json/JsonDeserializer.hpp @@ -28,19 +28,14 @@ class JsonDeserializer { _nestingLimit(nestingLimit), _loaded(false) {} DeserializationError parse(VariantData &variant) { - DeserializationError err = skipSpacesAndComments(); - if (err) return err; + DeserializationError err = parseVariant(variant); - switch (current()) { - case '[': - return parseArray(variant.toArray()); - - case '{': - return parseObject(variant.toObject()); - - default: - return parseValue(variant); + if (!err && _current != 0 && !variant.isEnclosed()) { + // We don't detect trailing characters earlier, so we need to check now + err = DeserializationError::InvalidInput; } + + return err; } private: @@ -48,10 +43,8 @@ class JsonDeserializer { char current() { if (!_loaded) { - if (_reader.ended()) - _current = 0; - else - _current = _reader.read(); + int c = _reader.read(); + _current = static_cast(c > 0 ? c : 0); _loaded = true; } return _current; @@ -67,6 +60,26 @@ class JsonDeserializer { return true; } + DeserializationError parseVariant(VariantData &variant) { + DeserializationError err = skipSpacesAndComments(); + if (err) return err; + + switch (current()) { + case '[': + return parseArray(variant.toArray()); + + case '{': + return parseObject(variant.toObject()); + + case '\"': + case '\'': + return parseStringValue(variant); + + default: + return parseNumericValue(variant); + } + } + DeserializationError parseArray(CollectionData &array) { if (_nestingLimit == 0) return DeserializationError::TooDeep; @@ -88,7 +101,7 @@ class JsonDeserializer { // 1 - Parse value _nestingLimit--; - err = parse(*value); + err = parseVariant(*value); _nestingLimit++; if (err) return err; @@ -134,7 +147,7 @@ class JsonDeserializer { // Parse value _nestingLimit--; - err = parse(*slot->data()); + err = parseVariant(*slot->data()); _nestingLimit++; if (err) return err; @@ -152,14 +165,6 @@ class JsonDeserializer { } } - DeserializationError parseValue(VariantData &variant) { - if (isQuote(current())) { - return parseStringValue(variant); - } else { - return parseNumericValue(variant); - } - } - DeserializationError parseKey(const char *&key) { if (isQuote(current())) { return parseQuotedString(key); diff --git a/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp b/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp index 70c9e94f..4c268968 100644 --- a/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp +++ b/src/ArduinoJson/MsgPack/MsgPackDeserializer.hpp @@ -134,17 +134,10 @@ class MsgPackDeserializer { // Prevent VS warning "assignment operator could not be generated" MsgPackDeserializer &operator=(const MsgPackDeserializer &); - bool skip(uint8_t n) { - while (n--) { - if (_reader.ended()) return false; - _reader.read(); - } - return true; - } - bool readByte(uint8_t &value) { - if (_reader.ended()) return false; - value = static_cast(_reader.read()); + int c = _reader.read(); + if (c < 0) return false; + value = static_cast(c); return true; } diff --git a/src/ArduinoJson/Variant/VariantData.hpp b/src/ArduinoJson/Variant/VariantData.hpp index e471c8a5..e369e6e8 100644 --- a/src/ArduinoJson/Variant/VariantData.hpp +++ b/src/ArduinoJson/Variant/VariantData.hpp @@ -177,6 +177,10 @@ class VariantData { return type() == VALUE_IS_NULL; } + bool isEnclosed() const { + return isCollection() || isString(); + } + void remove(size_t index) { if (isArray()) _content.asCollection.remove(index); } diff --git a/test/JsonDeserializer/invalid_input.cpp b/test/JsonDeserializer/invalid_input.cpp index ad1fac80..8e5dec9b 100644 --- a/test/JsonDeserializer/invalid_input.cpp +++ b/test/JsonDeserializer/invalid_input.cpp @@ -8,7 +8,8 @@ TEST_CASE("Invalid JSON input") { const char* testCases[] = {"'\\u'", "'\\u000g'", "'\\u000'", "'\\u000G'", - "'\\u000/'", "\\x1234", "6a9"}; + "'\\u000/'", "\\x1234", "6a9", "1,", + "2]", "3}"}; const size_t testCount = sizeof(testCases) / sizeof(testCases[0]); DynamicJsonDocument doc(4096); diff --git a/test/Misc/CMakeLists.txt b/test/Misc/CMakeLists.txt index 724c192d..e66246d7 100644 --- a/test/Misc/CMakeLists.txt +++ b/test/Misc/CMakeLists.txt @@ -5,6 +5,7 @@ add_executable(MiscTests conflicts.cpp FloatParts.cpp + StreamReader.cpp StringWriter.cpp TypeTraits.cpp unsigned_char.cpp diff --git a/test/Misc/StreamReader.cpp b/test/Misc/StreamReader.cpp new file mode 100644 index 00000000..ab4c244a --- /dev/null +++ b/test/Misc/StreamReader.cpp @@ -0,0 +1,33 @@ +// ArduinoJson - arduinojson.org +// Copyright Benoit Blanchon 2014-2019 +// MIT License + +#include +#include + +using namespace ARDUINOJSON_NAMESPACE; + +TEST_CASE("StdStreamReader") { + std::istringstream src("\x01\xFF"); + StdStreamReader reader(src); + + REQUIRE(reader.read() == 0x01); + REQUIRE(reader.read() == 0xFF); + REQUIRE(reader.read() == -1); +} + +TEST_CASE("SafeCharPointerReader") { + SafeCharPointerReader reader("\x01\xFF", 2); + + REQUIRE(reader.read() == 0x01); + REQUIRE(reader.read() == 0xFF); + REQUIRE(reader.read() == -1); +} + +TEST_CASE("UnsafeCharPointerReader") { + UnsafeCharPointerReader reader("\x01\xFF"); + + REQUIRE(reader.read() == 0x01); + REQUIRE(reader.read() == 0xFF); + REQUIRE(reader.read() == 0); +}