8000 Fixed null values that could be pass to `strcmp()` (closes #745) · janelia-arduino/ArduinoJson@7c0af91 · GitHub
[go: up one dir, main page]
Skip to content

Commit 7c0af91

Browse files
bblanchonbblanchon
committed
Fixed null values that could be pass to strcmp() (closes bblanchon#745)
1 parent 3523296 commit 7c0af91

File tree

9 files changed

+75
-16
lines changed

9 files changed

+75
-16
lines changed

‎.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ matrix:
3737
apt:
3838
sources: ['ubuntu-toolchain-r-test']
3939
packages: ['g++-5']
40-
env: SCRIPT=cmake GCC=5 SANITIZE=undefined
40+
env: SCRIPT=cmake GCC=5
4141
- compiler: gcc
4242
addons:
4343
apt:

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ HEAD
66

77
* Fixed `JsonBuffer::parse()` not respecting nesting limit correctly (issue #693)
88
* Fixed inconsistencies in nesting level counting (PR #695 from Zhenyu Wu)
9+
* Fixed null values that could be pass to `strcmp()` (PR #745 from Mike Karlesky)
910

1011
v5.13.1
1112
-------

‎src/ArduinoJson/JsonObject.hpp

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -286,15 +286,21 @@ class JsonObject : public Internals::JsonPrintable<JsonObject>,
286286

287287
template <typename TStringRef, typename TValueRef>
288288
bool set_impl(TStringRef key, TValueRef value) {
289+
// ignore null key
290+
if (Internals::StringTraits<TStringRef>::is_null(key)) return false;
291+
292+
// search a matching key
289293
iterator it = findKey<TStringRef>(key);
290294
if (it == end()) {
295+
// add the key
291296
it = Internals::List<JsonPair>::add();
292297
if (it == end()) return false;
293-
294298
bool key_ok =
295299
Internals::ValueSaver<TStringRef>::save(_buffer, it->key, key);
296300
if (!key_ok) return false;
297301
}
302+
303+
// save the value
298304
return Internals::ValueSaver<TValueRef>::save(_buffer, it->value, value);
299305
}
300306

@@ -318,5 +324,5 @@ struct JsonVariantDefault<JsonObject> {
318324
return JsonObject::invalid();
319325
}
320326
};
321-
}
322-
}
327+
} // namespace Internals
328+
} // namespace ArduinoJson

‎src/ArduinoJson/JsonVariantComparisons.hpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,10 +129,11 @@ class JsonVariantComparisons {
129129
if (is<JsonObject>() && right.template is<JsonObject>())
130130
return as<JsonObject>() == right.template as<JsonObject>();
131131
if (is<char *>() && right.template is<char *>())
132-
return strcmp(as<char *>(), right.template as<char *>()) == 0;
132+
return StringTraits<const char *>::equals(as<char *>(),
133+
right.template as<char *>());
133134

134135
return false;
135136
}
136137
};
137-
}
138-
}
138+
} // namespace Internals
139+
} // namespace ArduinoJson

‎src/ArduinoJson/StringTraits/CharPointer.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ struct CharPointerTraits {
3030
};
3131

3232
static bool equals(const TChar* str, const char* expected) {
33-
return strcmp(reinterpret_cast<const char*>(str), expected) == 0;
33+
const char* actual = reinterpret_cast<const char*>(str);
34+
if (!actual || !expected) return actual == expected;
35+
return strcmp(actual, expected) == 0;
3436
}
3537

3638
static bool is_null(const TChar* str) {
@@ -58,5 +60,5 @@ struct CharPointerTraits {
5860
template <typename TChar>
5961
struct StringTraits<TChar*, typename EnableIf<IsChar<TChar>::value>::type>
6062
: CharPointerTraits<TChar> {};
61-
}
62-
}
63+
} // namespace Internals
64+
} // namespace ArduinoJson

‎src/ArduinoJson/StringTraits/FlashString.hpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ struct StringTraits<const __FlashStringHelper*, void> {
3131
};
3232

3333
static bool equals(const __FlashStringHelper* str, const char* expected) {
34-
return strcmp_P(expected, (const char*)str) == 0;
34+
const char* actual = reinterpret_cast<const char*>(str);
35+
if (!actual || !expected) return actual == expected;
36+
return strcmp_P(expected, actual) == 0;
3537
}
3638

3739
static bool is_null(const __FlashStringHelper* str) {
@@ -53,7 +55,7 @@ struct StringTraits<const __FlashStringHelper*, void> {
5355
static const bool has_equals = true;
5456
static const bool should_duplicate = true;
5557
};
56-
}
57-
}
58+
} // namespace Internals
59+
} // namespace ArduinoJson
5860

5961
#endif

‎src/ArduinoJson/StringTraits/StdString.hpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,10 @@ struct StdStringTraits {
4040
};
4141

4242
static bool equals(const TString& str, const char* expected) {
43-
return 0 == strcmp(str.c_str(), expected);
43+
// Arduino's String::c_str() can return NULL
44+
const char* actual = str.c_str();
45+
if (!actual || !expected) return actual == expected;
46+
return 0 == strcmp(actual, expected);
4447
}
4548

4649
static void append(TString& str, char c) {
@@ -68,7 +71,7 @@ struct StringTraits<StringSumHelper, void> : StdStringTraits<StringSumHelper> {
6871
template <>
6972
struct StringTraits<std::string, void> : StdStringTraits<std::string> {};
7073
#endif
71-
}
72-
}
74+
} // namespace Internals
75+
} // namespace ArduinoJson
7376

7477
#endif

‎test/JsonObject/subscript.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,4 +149,15 @@ TEST_CASE("JsonObject::operator[]") {
149149
const size_t expectedSize = JSON_OBJECT_SIZE(1) + 12;
150150
REQUIRE(expectedSize <= _jsonBuffer.size());
151151
}
152+
153+
SECTION("should ignore null key") {
154+
// object must have a value to make a call to strcmp()
155+
_object["dummy"] = 42;
156+
157+
const char* null = 0;
158+
_object[null] = 666;
159+
160+
REQUIRE(_object.size() == 1);
161+
REQUIRE(_object[null] == 0);
162+
}
152163
}

‎test/JsonVariant/compare.cpp

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
#include <ArduinoJson.h>
66
#include <catch.hpp>
77

8+
static const char* null = 0;
9+
810
template <typename T>
911
void checkEquals(JsonVariant a, T b) {
1012
REQUIRE(b == a);
@@ -96,38 +98,69 @@ TEST_CASE("JsonVariant comparisons") {
9698
checkComparisons<unsigned short>(122, 123, 124);
9799
}
98100

101+
SECTION("null") {
102+
JsonVariant variant = null;
103+
104+
REQUIRE(variant == variant);
105+
REQUIRE_FALSE(variant != variant);
106+
107+
REQUIRE(variant == null);
108+
REQUIRE_FALSE(variant != null);
109+
110+
REQUIRE(variant != "null");
111+
REQUIRE_FALSE(variant == "null");
112+
}
113+
99114
SECTION("StringLiteral") {
100115
DynamicJsonBuffer jsonBuffer;
101116
JsonVariant variant = jsonBuffer.parse("\"hello\"");
102117

118+
REQUIRE(variant == variant);
119+
REQUIRE_FALSE(variant != variant);
120+
103121
REQUIRE(variant == "hello");
104122
REQUIRE_FALSE(variant != "hello");
105123

106124
REQUIRE(variant != "world");
107125
REQUIRE_FALSE(variant == "world");
108126

127+
REQUIRE(variant != null);
128+
REQUIRE_FALSE(variant == null);
129+
109130
REQUIRE("hello" == variant);
110131
REQUIRE_FALSE("hello" != variant);
111132

112133
REQUIRE("world" != variant);
113134
REQUIRE_FALSE("world" == variant);
135+
136+
REQUIRE(null != variant);
137+
REQUIRE_FALSE(null == variant);
114138
}
115139

116140
SECTION("String") {
117141
DynamicJsonBuffer jsonBuffer;
118142
JsonVariant variant = jsonBuffer.parse("\"hello\"");
119143

144+
REQUIRE(variant == variant);
145+
REQUIRE_FALSE(variant != variant);
146+
120147
REQUIRE(variant == std::string("hello"));
121148
REQUIRE_FALSE(variant != std::string("hello"));
122149

123150
REQUIRE(variant != std::string("world"));
124151
REQUIRE_FALSE(variant == std::string("world"));
125152

153+
REQUIRE(variant != null);
154+
REQUIRE_FALSE(variant == null);
155+
126156
REQUIRE(std::string("hello") == variant);
127157
REQUIRE_FALSE(std::string("hello") != variant);
128158

129159
REQUIRE(std::string("world") != variant);
130160
REQUIRE_FALSE(std::string("world") == variant);
161+
162+
REQUIRE(null != variant);
163+
REQUIRE_FALSE(null == variant);
131164
}
132165

133166
SECTION("IntegerInVariant") {

0 commit comments

Comments
 (0)
0