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") {