From 3523296e3dfefa62c9c88dd37b06589e244a33f8 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Mon, 12 Mar 2018 18:28:37 +0100 Subject: [PATCH] Fixed `JsonBuffer::parse()` nesting limit (fixes #693) --- CHANGELOG.md | 6 ++ .../Deserialization/JsonParser.hpp | 1 - .../Deserialization/JsonParserImpl.hpp | 19 ++--- test/JsonBuffer/nestingLimit.cpp | 82 +++++++++++-------- 4 files changed, 64 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9fb7ecf9..9b25720f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ ArduinoJson: change log ======================= +HEAD +---- + +* Fixed `JsonBuffer::parse()` not respecting nesting limit correctly (issue #693) +* Fixed inconsistencies in nesting level counting (PR #695 from Zhenyu Wu) + v5.13.1 ------- diff --git a/src/ArduinoJson/Deserialization/JsonParser.hpp b/src/ArduinoJson/Deserialization/JsonParser.hpp index c348e759..4cbaf454 100644 --- a/src/ArduinoJson/Deserialization/JsonParser.hpp +++ b/src/ArduinoJson/Deserialization/JsonParser.hpp @@ -44,7 +44,6 @@ class JsonParser { const char *parseString(); bool parseAnythingTo(JsonVariant *destination); - FORCE_INLINE bool parseAnythingToUnsafe(JsonVariant *destination); inline bool parseArrayTo(JsonVariant *destination); inline bool parseObjectTo(JsonVariant *destination); diff --git a/src/ArduinoJson/Deserialization/JsonParserImpl.hpp b/src/ArduinoJson/Deserialization/JsonParserImpl.hpp index 33ad42e9..50426735 100644 --- a/src/ArduinoJson/Deserialization/JsonParserImpl.hpp +++ b/src/ArduinoJson/Deserialization/JsonParserImpl.hpp @@ -20,17 +20,6 @@ template inline bool ArduinoJson::Internals::JsonParser::parseAnythingTo( JsonVariant *destination) { - if (_nestingLimit == 0) return false; - _nestingLimit--; - bool success = parseAnythingToUnsafe(destination); - _nestingLimit++; - return success; -} - -template -inline bool -ArduinoJson::Internals::JsonParser::parseAnythingToUnsafe( - JsonVariant *destination) { skipSpacesAndComments(_reader); switch (_reader.current()) { @@ -48,6 +37,9 @@ ArduinoJson::Internals::JsonParser::parseAnythingToUnsafe( template inline ArduinoJson::JsonArray & ArduinoJson::Internals::JsonParser::parseArray() { + if (_nestingLimit == 0) return JsonArray::invalid(); + _nestingLimit--; + // Create an empty array JsonArray &array = _buffer->createArray(); @@ -69,6 +61,7 @@ ArduinoJson::Internals::JsonParser::parseArray() { SUCCESS_EMPTY_ARRAY: SUCCES_NON_EMPTY_ARRAY: + _nestingLimit++; return array; ERROR_INVALID_VALUE: @@ -91,6 +84,9 @@ inline bool ArduinoJson::Internals::JsonParser::parseArrayTo( template inline ArduinoJson::JsonObject & ArduinoJson::Internals::JsonParser::parseObject() { + if (_nestingLimit == 0) return JsonObject::invalid(); + _nestingLimit--; + // Create an empty object JsonObject &object = _buffer->createObject(); @@ -117,6 +113,7 @@ ArduinoJson::Internals::JsonParser::parseObject() { SUCCESS_EMPTY_OBJECT: SUCCESS_NON_EMPTY_OBJECT: + _nestingLimit++; return object; ERROR_INVALID_KEY: diff --git a/test/JsonBuffer/nestingLimit.cpp b/test/JsonBuffer/nestingLimit.cpp index 0d7be115..30faca90 100644 --- a/test/JsonBuffer/nestingLimit.cpp +++ b/test/JsonBuffer/nestingLimit.cpp @@ -5,44 +5,62 @@ #include #include -bool tryParseArray(const char *json, uint8_t nestingLimit) { - DynamicJsonBuffer buffer; - return buffer.parseArray(json, nestingLimit).success(); -} - -bool tryParseObject(const char *json, uint8_t nestingLimit) { - DynamicJsonBuffer buffer; - return buffer.parseObject(json, nestingLimit).success(); -} +#define SHOULD_WORK(expression) REQUIRE(true == expression.success()); +#define SHOULD_FAIL(expression) REQUIRE(false == expression.success()); TEST_CASE("JsonParser nestingLimit") { - SECTION("ParseArrayWithNestingLimit0") { - REQUIRE(true == tryParseArray("[]", 0)); - REQUIRE(false == tryParseArray("[[]]", 0)); + DynamicJsonBuffer jb; + + SECTION("parseArray()") { + SECTION("limit = 0") { + SHOULD_FAIL(jb.parseArray("[]", 0)); + } + + SECTION("limit = 1") { + SHOULD_WORK(jb.parseArray("[]", 1)); + SHOULD_FAIL(jb.parseArray("[[]]", 1)); + } + + SECTION("limit = 2") { + SHOULD_WORK(jb.parseArray("[[]]", 2)); + SHOULD_FAIL(jb.parseArray("[[[]]]", 2)); + } } - SECTION("ParseArrayWithNestingLimit1") { - REQUIRE(true == tryParseArray("[[]]", 1)); - REQUIRE(false == tryParseArray("[[[]]]", 1)); + SECTION("parseObject()") { + SECTION("limit = 0") { + SHOULD_FAIL(jb.parseObject("{}", 0)); + } + + SECTION("limit = 1") { + SHOULD_WORK(jb.parseObject("{\"key\":42}", 1)); + SHOULD_FAIL(jb.parseObject("{\"key\":{\"key\":42}}", 1)); + } + + SECTION("limit = 2") { + SHOULD_WORK(jb.parseObject("{\"key\":{\"key\":42}}", 2)); + SHOULD_FAIL(jb.parseObject("{\"key\":{\"key\":{\"key\":42}}}", 2)); + } } - SECTION("ParseArrayWithNestingLimit2") { - REQUIRE(true == tryParseArray("[[[]]]", 2)); - REQUIRE(false == tryParseArray("[[[[]]]]", 2)); - } + SECTION("parse()") { + SECTION("limit = 0") { + SHOULD_WORK(jb.parse("\"toto\"", 0)); + SHOULD_WORK(jb.parse("123", 0)); + SHOULD_WORK(jb.parse("true", 0)); + SHOULD_FAIL(jb.parse("[]", 0)); + SHOULD_FAIL(jb.parse("{}", 0)); + SHOULD_FAIL(jb.parse("[\"toto\"]", 0)); + SHOULD_FAIL(jb.parse("{\"toto\":1}", 0)); + } - SECTION("ParseObjectWithNestingLimit0") { - REQUIRE(true == tryParseObject("{}", 0)); - REQUIRE(false == tryParseObject("{\"key\":{}}", 0)); - } - - SECTION("ParseObjectWithNestingLimit1") { - REQUIRE(true == tryParseObject("{\"key\":{}}", 1)); - REQUIRE(false == tryParseObject("{\"key\":{\"key\":{}}}", 1)); - } - - SECTION("ParseObjectWithNestingLimit2") { - REQUIRE(true == tryParseObject("{\"key\":{\"key\":{}}}", 2)); - REQUIRE(false == tryParseObject("{\"key\":{\"key\":{\"key\":{}}}}", 2)); + SECTION("limit = 1") { + SHOULD_WORK(jb.parse("[\"toto\"]", 1)); + SHOULD_WORK(jb.parse("{\"toto\":1}", 1)); + SHOULD_FAIL(jb.parse("{\"toto\":{}}", 1)); + SHOULD_FAIL(jb.parse("{\"toto\":[]}", 1)); + SHOULD_FAIL(jb.parse("[[\"toto\"]]", 1)); + SHOULD_FAIL(jb.parse("[{\"toto\":1}]", 1)); + } } }