From 7c0af91844b0ed8c0c8a566677b832967935298a Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Fri, 1 Jun 2018 09:08:38 +0200 Subject: [PATCH 1/3] Fixed null values that could be pass to `strcmp()` (closes #745) --- .travis.yml | 2 +- CHANGELOG.md | 1 + src/ArduinoJson/JsonObject.hpp | 12 +++++-- src/ArduinoJson/JsonVariantComparisons.hpp | 7 +++-- src/ArduinoJson/StringTraits/CharPointer.hpp | 8 +++-- src/ArduinoJson/StringTraits/FlashString.hpp | 8 +++-- src/ArduinoJson/StringTraits/StdString.hpp | 9 ++++-- test/JsonObject/subscript.cpp | 11 +++++++ test/JsonVariant/compare.cpp | 33 ++++++++++++++++++++ 9 files changed, 75 insertions(+), 16 deletions(-) diff --git a/.travis.yml b/.travis.yml index 9a9dddcd..1ae95d97 100644 --- a/.travis.yml +++ b/.travis.yml @@ -37,7 +37,7 @@ matrix: apt: sources: ['ubuntu-toolchain-r-test'] packages: ['g++-5'] - env: SCRIPT=cmake GCC=5 SANITIZE=undefined + env: SCRIPT=cmake GCC=5 - compiler: gcc addons: apt: diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b25720f..4c764675 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ HEAD * Fixed `JsonBuffer::parse()` not respecting nesting limit correctly (issue #693) * Fixed inconsistencies in nesting level counting (PR #695 from Zhenyu Wu) +* Fixed null values that could be pass to `strcmp()` (PR #745 from Mike Karlesky) v5.13.1 ------- diff --git a/src/ArduinoJson/JsonObject.hpp b/src/ArduinoJson/JsonObject.hpp index c03bfa51..caf698a3 100644 --- a/src/ArduinoJson/JsonObject.hpp +++ b/src/ArduinoJson/JsonObject.hpp @@ -286,15 +286,21 @@ class JsonObject : public Internals::JsonPrintable, template bool set_impl(TStringRef key, TValueRef value) { + // ignore null key + if (Internals::StringTraits::is_null(key)) return false; + + // search a matching key iterator it = findKey(key); if (it == end()) { + // add the key it = Internals::List::add(); if (it == end()) return false; - bool key_ok = Internals::ValueSaver::save(_buffer, it->key, key); if (!key_ok) return false; } + + // save the value return Internals::ValueSaver::save(_buffer, it->value, value); } @@ -318,5 +324,5 @@ struct JsonVariantDefault { return JsonObject::invalid(); } }; -} -} +} // namespace Internals +} // namespace ArduinoJson diff --git a/src/ArduinoJson/JsonVariantComparisons.hpp b/src/ArduinoJson/JsonVariantComparisons.hpp index cae53372..47f9d632 100644 --- a/src/ArduinoJson/JsonVariantComparisons.hpp +++ b/src/ArduinoJson/JsonVariantComparisons.hpp @@ -129,10 +129,11 @@ class JsonVariantComparisons { if (is() && right.template is()) return as() == right.template as(); if (is() && right.template is()) - return strcmp(as(), right.template as()) == 0; + return StringTraits::equals(as(), + right.template as()); return false; } }; -} -} +} // namespace Internals +} // namespace ArduinoJson diff --git a/src/ArduinoJson/StringTraits/CharPointer.hpp b/src/ArduinoJson/StringTraits/CharPointer.hpp index a9f30f78..98896ccf 100644 --- a/src/ArduinoJson/StringTraits/CharPointer.hpp +++ b/src/ArduinoJson/StringTraits/CharPointer.hpp @@ -30,7 +30,9 @@ struct CharPointerTraits { }; static bool equals(const TChar* str, const char* expected) { - return strcmp(reinterpret_cast(str), expected) == 0; + const char* actual = reinterpret_cast(str); + if (!actual || !expected) return actual == expected; + return strcmp(actual, expected) == 0; } static bool is_null(const TChar* str) { @@ -58,5 +60,5 @@ struct CharPointerTraits { template struct StringTraits::value>::type> : CharPointerTraits {}; -} -} +} // namespace Internals +} // namespace ArduinoJson diff --git a/src/ArduinoJson/StringTraits/FlashString.hpp b/src/ArduinoJson/StringTraits/FlashString.hpp index 95f555d2..0701b9ba 100644 --- a/src/ArduinoJson/StringTraits/FlashString.hpp +++ b/src/ArduinoJson/StringTraits/FlashString.hpp @@ -31,7 +31,9 @@ struct StringTraits { }; static bool equals(const __FlashStringHelper* str, const char* expected) { - return strcmp_P(expected, (const char*)str) == 0; + const char* actual = reinterpret_cast(str); + if (!actual || !expected) return actual == expected; + return strcmp_P(expected, actual) == 0; } static bool is_null(const __FlashStringHelper* str) { @@ -53,7 +55,7 @@ struct StringTraits { static const bool has_equals = true; static const bool should_duplicate = true; }; -} -} +} // namespace Internals +} // namespace ArduinoJson #endif diff --git a/src/ArduinoJson/StringTraits/StdString.hpp b/src/ArduinoJson/StringTraits/StdString.hpp index 39124dac..35f4461d 100644 --- a/src/ArduinoJson/StringTraits/StdString.hpp +++ b/src/ArduinoJson/StringTraits/StdString.hpp @@ -40,7 +40,10 @@ struct StdStringTraits { }; static bool equals(const TString& str, const char* expected) { - return 0 == strcmp(str.c_str(), expected); + // Arduino's String::c_str() can return NULL + const char* actual = str.c_str(); + if (!actual || !expected) return actual == expected; + return 0 == strcmp(actual, expected); } static void append(TString& str, char c) { @@ -68,7 +71,7 @@ struct StringTraits : StdStringTraits { template <> struct StringTraits : StdStringTraits {}; #endif -} -} +} // namespace Internals +} // namespace ArduinoJson #endif diff --git a/test/JsonObject/subscript.cpp b/test/JsonObject/subscript.cpp index d08e8c77..365353ed 100644 --- a/test/JsonObject/subscript.cpp +++ b/test/JsonObject/subscript.cpp @@ -149,4 +149,15 @@ TEST_CASE("JsonObject::operator[]") { const size_t expectedSize = JSON_OBJECT_SIZE(1) + 12; REQUIRE(expectedSize <= _jsonBuffer.size()); } + + SECTION("should ignore null key") { + // object must have a value to make a call to strcmp() + _object["dummy"] = 42; + + const char* null = 0; + _object[null] = 666; + + REQUIRE(_object.size() == 1); + REQUIRE(_object[null] == 0); + } } diff --git a/test/JsonVariant/compare.cpp b/test/JsonVariant/compare.cpp index 965ec8ef..496a6a9a 100644 --- a/test/JsonVariant/compare.cpp +++ b/test/JsonVariant/compare.cpp @@ -5,6 +5,8 @@ #include #include +static const char* null = 0; + template void checkEquals(JsonVariant a, T b) { REQUIRE(b == a); @@ -96,38 +98,69 @@ TEST_CASE("JsonVariant comparisons") { checkComparisons(122, 123, 124); } + SECTION("null") { + JsonVariant variant = null; + + REQUIRE(variant == variant); + REQUIRE_FALSE(variant != variant); + + REQUIRE(variant == null); + REQUIRE_FALSE(variant != null); + + REQUIRE(variant != "null"); + REQUIRE_FALSE(variant == "null"); + } + SECTION("StringLiteral") { DynamicJsonBuffer jsonBuffer; JsonVariant variant = jsonBuffer.parse("\"hello\""); + REQUIRE(variant == variant); + REQUIRE_FALSE(variant != variant); + REQUIRE(variant == "hello"); REQUIRE_FALSE(variant != "hello"); REQUIRE(variant != "world"); REQUIRE_FALSE(variant == "world"); + REQUIRE(variant != null); + REQUIRE_FALSE(variant == null); + REQUIRE("hello" == variant); REQUIRE_FALSE("hello" != variant); REQUIRE("world" != variant); REQUIRE_FALSE("world" == variant); + + REQUIRE(null != variant); + REQUIRE_FALSE(null == variant); } SECTION("String") { DynamicJsonBuffer jsonBuffer; JsonVariant variant = jsonBuffer.parse("\"hello\""); + REQUIRE(variant == variant); + REQUIRE_FALSE(variant != variant); + REQUIRE(variant == std::string("hello")); REQUIRE_FALSE(variant != std::string("hello")); REQUIRE(variant != std::string("world")); REQUIRE_FALSE(variant == std::string("world")); + REQUIRE(variant != null); + REQUIRE_FALSE(variant == null); + REQUIRE(std::string("hello") == variant); REQUIRE_FALSE(std::string("hello") != variant); REQUIRE(std::string("world") != variant); REQUIRE_FALSE(std::string("world") == variant); + + REQUIRE(null != variant); + REQUIRE_FALSE(null == variant); } SECTION("IntegerInVariant") { From eb20ae6a3f39f620627725da5d4213345faef993 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Fri, 1 Jun 2018 09:16:45 +0200 Subject: [PATCH 2/3] Added macros `ARDUINOJSON_VERSION`, `ARDUINOJSON_VERSION_MAJOR`... --- CHANGELOG.md | 1 + src/ArduinoJson.hpp | 2 ++ src/ArduinoJson/version.hpp | 10 ++++++++++ test/Misc/CMakeLists.txt | 1 + test/Misc/version.cpp | 16 ++++++++++++++++ 5 files changed, 30 insertions(+) create mode 100644 src/ArduinoJson/version.hpp create mode 100644 test/Misc/version.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c764675..70dc7040 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ HEAD * Fixed `JsonBuffer::parse()` not respecting nesting limit correctly (issue #693) * Fixed inconsistencies in nesting level counting (PR #695 from Zhenyu Wu) * Fixed null values that could be pass to `strcmp()` (PR #745 from Mike Karlesky) +* Added macros `ARDUINOJSON_VERSION`, `ARDUINOJSON_VERSION_MAJOR`... v5.13.1 ------- diff --git a/src/ArduinoJson.hpp b/src/ArduinoJson.hpp index 445a2c8a..c493c06a 100644 --- a/src/ArduinoJson.hpp +++ b/src/ArduinoJson.hpp @@ -4,6 +4,8 @@ #pragma once +#include "ArduinoJson/version.hpp" + #include "ArduinoJson/DynamicJsonBuffer.hpp" #include "ArduinoJson/JsonArray.hpp" #include "ArduinoJson/JsonObject.hpp" diff --git a/src/ArduinoJson/version.hpp b/src/ArduinoJson/version.hpp new file mode 100644 index 00000000..6d2870fb --- /dev/null +++ b/src/ArduinoJson/version.hpp @@ -0,0 +1,10 @@ +// ArduinoJson - arduinojson.org +// Copyright Benoit Blanchon 2014-2018 +// MIT License + +#pragma once + +#define ARDUINOJSON_VERSION "5.13.1" +#define ARDUINOJSON_VERSION_MAJOR 5 +#define ARDUINOJSON_VERSION_MINOR 13 +#define ARDUINOJSON_VERSION_REVISION 1 diff --git a/test/Misc/CMakeLists.txt b/test/Misc/CMakeLists.txt index 0b54bd7b..56bbd5c3 100644 --- a/test/Misc/CMakeLists.txt +++ b/test/Misc/CMakeLists.txt @@ -11,6 +11,7 @@ add_executable(MiscTests StringTraits.cpp TypeTraits.cpp unsigned_char.cpp + version.cpp vla.cpp ) diff --git a/test/Misc/version.cpp b/test/Misc/version.cpp new file mode 100644 index 00000000..a9bbbe72 --- /dev/null +++ b/test/Misc/version.cpp @@ -0,0 +1,16 @@ +// ArduinoJson - arduinojson.org +// Copyright Benoit Blanchon 2014-2018 +// MIT License + +#include +#include +#include + +TEST_CASE("ARDUINOJSON_VERSION") { + std::stringstream version; + + version << ARDUINOJSON_VERSION_MAJOR << "." << ARDUINOJSON_VERSION_MINOR + << "." << ARDUINOJSON_VERSION_REVISION; + + REQUIRE(version.str() == ARDUINOJSON_VERSION); +} From 011aac43d20630452f6ae619597ce18003477fa4 Mon Sep 17 00:00:00 2001 From: Benoit Blanchon Date: Fri, 1 Jun 2018 09:21:42 +0200 Subject: [PATCH 3/3] Set version to 5.13.2 --- CHANGELOG.md | 4 ++-- appveyor.yml | 2 +- library.json | 2 +- library.properties | 2 +- src/ArduinoJson/version.hpp | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70dc7040..1e512038 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,8 +1,8 @@ ArduinoJson: change log ======================= -HEAD ----- +v5.13.2 +------- * Fixed `JsonBuffer::parse()` not respecting nesting limit correctly (issue #693) * Fixed inconsistencies in nesting level counting (PR #695 from Zhenyu Wu) diff --git a/appveyor.yml b/appveyor.yml index f5d79b18..c864cfda 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,4 +1,4 @@ -version: 5.13.1.{build} +version: 5.13.2.{build} environment: matrix: - APPVEYOR_BUILD_WORKER_IMAGE: Visual Studio 2017 diff --git a/library.json b/library.json index eb790b92..6b87f062 100644 --- a/library.json +++ b/library.json @@ -7,7 +7,7 @@ "type": "git", "url": "https://github.com/bblanchon/ArduinoJson.git" }, - "version": "5.13.1", + "version": "5.13.2", "authors": { "name": "Benoit Blanchon", "url": "https://blog.benoitblanchon.fr" diff --git a/library.properties b/library.properties index 97e2f03a..abf6cfc6 100644 --- a/library.properties +++ b/library.properties @@ -1,5 +1,5 @@ name=ArduinoJson -version=5.13.1 +version=5.13.2 author=Benoit Blanchon maintainer=Benoit Blanchon sentence=An efficient and elegant JSON library for Arduino. diff --git a/src/ArduinoJson/version.hpp b/src/ArduinoJson/version.hpp index 6d2870fb..a71c3ab4 100644 --- a/src/ArduinoJson/version.hpp +++ b/src/ArduinoJson/version.hpp @@ -4,7 +4,7 @@ #pragma once -#define ARDUINOJSON_VERSION "5.13.1" +#define ARDUINOJSON_VERSION "5.13.2" #define ARDUINOJSON_VERSION_MAJOR 5 #define ARDUINOJSON_VERSION_MINOR 13 -#define ARDUINOJSON_VERSION_REVISION 1 +#define ARDUINOJSON_VERSION_REVISION 2