diff --git a/CHANGELOG.md b/CHANGELOG.md index 9434822d..f316b7fa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ 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) * Added `DynamicJsonArray` and `StaticJsonArray` * Added `DynamicJsonObject` and `StaticJsonObject` * Added `DynamicJsonVariant` and `StaticJsonVariant` diff --git a/src/ArduinoJson/Deserialization/JsonParser.hpp b/src/ArduinoJson/Deserialization/JsonParser.hpp index 3c132448..cbc7df6f 100644 --- a/src/ArduinoJson/Deserialization/JsonParser.hpp +++ b/src/ArduinoJson/Deserialization/JsonParser.hpp @@ -38,12 +38,9 @@ class JsonParser { } const char *parseString(); - JsonError parseAnythingTo(JsonVariant *destination); - FORCE_INLINE JsonError parseAnythingToUnsafe(JsonVariant *destination); - - inline JsonError parseArrayTo(JsonVariant *destination); - inline JsonError parseObjectTo(JsonVariant *destination); - inline JsonError parseStringTo(JsonVariant *destination); + JsonError parseArray(JsonVariant &variant); + JsonError parseObject(JsonVariant &variant); + JsonError parseValue(JsonVariant &variant); static inline bool isBetween(char c, char min, char max) { return min <= c && c <= max; diff --git a/src/ArduinoJson/Deserialization/JsonParserImpl.hpp b/src/ArduinoJson/Deserialization/JsonParserImpl.hpp index 3e767a2a..e9cbd2cf 100644 --- a/src/ArduinoJson/Deserialization/JsonParserImpl.hpp +++ b/src/ArduinoJson/Deserialization/JsonParserImpl.hpp @@ -17,38 +17,11 @@ inline bool ArduinoJson::Internals::JsonParser::eat( return true; } -template -inline ArduinoJson::JsonError -ArduinoJson::Internals::JsonParser::parseAnythingTo( - JsonVariant *destination) { - if (_nestingLimit == 0) return JsonError::TooDeep; - _nestingLimit--; - JsonError error = parseAnythingToUnsafe(destination); - _nestingLimit++; - return error; -} - -template -inline ArduinoJson::JsonError -ArduinoJson::Internals::JsonParser::parseAnythingToUnsafe( - JsonVariant *destination) { - skipSpacesAndComments(_reader); - - switch (_reader.current()) { - case '[': - return parseArrayTo(destination); - - case '{': - return parseObjectTo(destination); - - default: - return parseStringTo(destination); - } -} - template inline ArduinoJson::JsonError ArduinoJson::Internals::JsonParser::parse(JsonArray &array) { + if (_nestingLimit == 0) return JsonError::TooDeep; + // Check opening braket if (!eat('[')) return JsonError::OpeningBracketExpected; if (eat(']')) return JsonError::Ok; @@ -57,7 +30,9 @@ ArduinoJson::Internals::JsonParser::parse(JsonArray &array) { for (;;) { // 1 - Parse value JsonVariant value; - JsonError error = parseAnythingTo(&value); + _nestingLimit--; + JsonError error = parse(value); + _nestingLimit++; if (error != JsonError::Ok) return error; if (!array.add(value)) return JsonError::NoMemory; @@ -71,6 +46,8 @@ template inline ArduinoJson::JsonError ArduinoJson::Internals::JsonParser::parse( JsonObject &object) { + if (_nestingLimit == 0) return JsonError::TooDeep; + // Check opening brace if (!eat('{')) return JsonError::OpeningBraceExpected; if (eat('}')) return JsonError::Ok; @@ -84,7 +61,9 @@ ArduinoJson::Internals::JsonParser::parse( // 2 - Parse value JsonVariant value; - JsonError error = parseAnythingTo(&value); + _nestingLimit--; + JsonError error = parse(value); + _nestingLimit++; if (error != JsonError::Ok) return error; if (!object.set(key, value)) return JsonError::NoMemory; @@ -98,29 +77,55 @@ template inline ArduinoJson::JsonError ArduinoJson::Internals::JsonParser::parse( JsonVariant &variant) { - return parseAnythingTo(&variant); + skipSpacesAndComments(_reader); + + switch (_reader.current()) { + case '[': + return parseArray(variant); + + case '{': + return parseObject(variant); + + default: + return parseValue(variant); + } } template inline ArduinoJson::JsonError -ArduinoJson::Internals::JsonParser::parseArrayTo( - JsonVariant *destination) { +ArduinoJson::Internals::JsonParser::parseArray( + JsonVariant &variant) { JsonArray *array = new (_buffer) JsonArray(_buffer); if (!array) return JsonError::NoMemory; - *destination = array; + variant = array; return parse(*array); } template inline ArduinoJson::JsonError -ArduinoJson::Internals::JsonParser::parseObjectTo( - JsonVariant *destination) { +ArduinoJson::Internals::JsonParser::parseObject( + JsonVariant &variant) { JsonObject *object = new (_buffer) JsonObject(_buffer); if (!object) return JsonError::NoMemory; - *destination = object; + variant = object; return parse(*object); } +template +inline ArduinoJson::JsonError +ArduinoJson::Internals::JsonParser::parseValue( + JsonVariant &variant) { + bool hasQuotes = isQuote(_reader.current()); + const char *value = parseString(); + if (value == NULL) return JsonError::NoMemory; + if (hasQuotes) { + variant = value; + } else { + variant = RawJson(value); + } + return JsonError::Ok; +} + template inline const char * ArduinoJson::Internals::JsonParser::parseString() { @@ -159,18 +164,3 @@ ArduinoJson::Internals::JsonParser::parseString() { return str.c_str(); } - -template -inline ArduinoJson::JsonError -ArduinoJson::Internals::JsonParser::parseStringTo( - JsonVariant *destination) { - bool hasQuotes = isQuote(_reader.current()); - const char *value = parseString(); - if (value == NULL) return JsonError::NoMemory; - if (hasQuotes) { - *destination = value; - } else { - *destination = RawJson(value); - } - return JsonError::Ok; -} diff --git a/src/ArduinoJson/JsonError.hpp b/src/ArduinoJson/JsonError.hpp index e62dae6f..6349a98b 100644 --- a/src/ArduinoJson/JsonError.hpp +++ b/src/ArduinoJson/JsonError.hpp @@ -21,12 +21,20 @@ class JsonError { JsonError(Code code) : _code(code) {} - bool operator==(Code code) const { - return _code == code; + friend bool operator==(const JsonError& err, Code code) { + return err._code == code; } - bool operator!=(Code code) const { - return _code != code; + friend bool operator==(Code code, const JsonError& err) { + return err._code == code; + } + + friend bool operator!=(const JsonError& err, Code code) { + return err._code != code; + } + + friend bool operator!=(Code code, const JsonError& err) { + return err._code != code; } operator bool() const { diff --git a/test/JsonParser/nestingLimit.cpp b/test/JsonParser/nestingLimit.cpp index 710a9fb5..74148956 100644 --- a/test/JsonParser/nestingLimit.cpp +++ b/test/JsonParser/nestingLimit.cpp @@ -5,45 +5,66 @@ #include #include -JsonError tryParseArray(const char *json, uint8_t nestingLimit) { - DynamicJsonArray array; - return deserializeJson(array, json, nestingLimit); -} - -JsonError tryParseObject(const char *json, uint8_t nestingLimit) { - DynamicJsonObject obj; - return deserializeJson(obj, json, nestingLimit); -} +#define SHOULD_WORK(expression) REQUIRE(JsonError::Ok == expression); +#define SHOULD_FAIL(expression) REQUIRE(JsonError::TooDeep == expression); TEST_CASE("JsonParser nestingLimit") { - SECTION("ParseArrayWithNestingLimit0") { - REQUIRE(tryParseArray("[]", 0) == JsonError::Ok); - REQUIRE(tryParseArray("[[]]", 0) == JsonError::TooDeep); + SECTION("deserializeJson(JsonArray&)") { + DynamicJsonArray arr; + + SECTION("limit = 0") { + SHOULD_FAIL(deserializeJson(arr, "[]", 0)); + } + + SECTION("limit = 1") { + SHOULD_WORK(deserializeJson(arr, "[]", 1)); + SHOULD_FAIL(deserializeJson(arr, "[[]]", 1)); + } + + SECTION("limit = 2") { + SHOULD_WORK(deserializeJson(arr, "[[]]", 2)); + SHOULD_FAIL(deserializeJson(arr, "[[[]]]", 2)); + } } - SECTION("ParseArrayWithNestingLimit1") { - REQUIRE(tryParseArray("[[]]", 1) == JsonError::Ok); - REQUIRE(tryParseArray("[[[]]]", 1) == JsonError::TooDeep); + SECTION("deserializeJson(JsonObject&)") { + DynamicJsonObject obj; + + SECTION("limit = 0") { + SHOULD_FAIL(deserializeJson(obj, "{}", 0)); + } + + SECTION("limit = 1") { + SHOULD_WORK(deserializeJson(obj, "{\"key\":42}", 1)); + SHOULD_FAIL(deserializeJson(obj, "{\"key\":{\"key\":42}}", 1)); + } + + SECTION("limit = 2") { + SHOULD_WORK(deserializeJson(obj, "{\"key\":{\"key\":42}}", 2)); + SHOULD_FAIL(deserializeJson(obj, "{\"key\":{\"key\":{\"key\":42}}}", 2)); + } } - SECTION("ParseArrayWithNestingLimit2") { - REQUIRE(tryParseArray("[[[]]]", 2) == JsonError::Ok); - REQUIRE(tryParseArray("[[[[]]]]", 2) == JsonError::TooDeep); - } + SECTION("deserializeJson(JsonVariant&)") { + DynamicJsonVariant var; - SECTION("ParseObjectWithNestingLimit0") { - REQUIRE(tryParseObject("{}", 0) == JsonError::Ok); - REQUIRE(tryParseObject("{\"key\":{}}", 0) == JsonError::TooDeep); - } + SECTION("limit = 0") { + SHOULD_WORK(deserializeJson(var, "\"toto\"", 0)); + SHOULD_WORK(deserializeJson(var, "123", 0)); + SHOULD_WORK(deserializeJson(var, "true", 0)); + SHOULD_FAIL(deserializeJson(var, "[]", 0)); + SHOULD_FAIL(deserializeJson(var, "{}", 0)); + SHOULD_FAIL(deserializeJson(var, "[\"toto\"]", 0)); + SHOULD_FAIL(deserializeJson(var, "{\"toto\":1}", 0)); + } - SECTION("ParseObjectWithNestingLimit1") { - REQUIRE(tryParseObject("{\"key\":{}}", 1) == JsonError::Ok); - REQUIRE(tryParseObject("{\"key\":{\"key\":{}}}", 1) == JsonError::TooDeep); - } - - SECTION("ParseObjectWithNestingLimit2") { - REQUIRE(tryParseObject("{\"key\":{\"key\":{}}}", 2) == JsonError::Ok); - REQUIRE(tryParseObject("{\"key\":{\"key\":{\"key\":{}}}}", 2) == - JsonError::TooDeep); + SECTION("limit = 1") { + SHOULD_WORK(deserializeJson(var, "[\"toto\"]", 1)); + SHOULD_WORK(deserializeJson(var, "{\"toto\":1}", 1)); + SHOULD_FAIL(deserializeJson(var, "{\"toto\":{}}", 1)); + SHOULD_FAIL(deserializeJson(var, "{\"toto\":[]}", 1)); + SHOULD_FAIL(deserializeJson(var, "[[\"toto\"]]", 1)); + SHOULD_FAIL(deserializeJson(var, "[{\"toto\":1}]", 1)); + } } }