Fix operator[](variant) ignoring NUL characters

bblanchon · Nov 14, 2024 · e007d71 · e007d71
1 parent 67a512a
commit e007d71
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 46 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,7 @@ HEAD
 
 * Forbid `deserializeJson(JsonArray|JsonObject, ...)` (issue #2135)
 * Fix VLA support in `JsonDocument::set()`
+* Fix `operator[](variant)` ignoring NUL characters
 
 v7.2.0 (2024-09-18)
 ------

diff --git a/extras/tests/JsonDocument/subscript.cpp b/extras/tests/JsonDocument/subscript.cpp
@@ -12,27 +12,40 @@ TEST_CASE("JsonDocument::operator[]") {
   const JsonDocument& cdoc = doc;
 
   SECTION("object") {
-    deserializeJson(doc, "{\"hello\":\"world\"}");
+    doc["abc"_s] = "ABC";
+    doc["abc\0d"_s] = "ABCD";
 
     SECTION("const char*") {
-      REQUIRE(doc["hello"] == "world");
-      REQUIRE(cdoc["hello"] == "world");
+      REQUIRE(doc["abc"] == "ABC");
+      REQUIRE(cdoc["abc"] == "ABC");
     }
 
     SECTION("std::string") {
-      REQUIRE(doc["hello"_s] == "world");
-      REQUIRE(cdoc["hello"_s] == "world");
+      REQUIRE(doc["abc"_s] == "ABC");
+      REQUIRE(cdoc["abc"_s] == "ABC");
+      REQUIRE(doc["abc\0d"_s] == "ABCD");
+      REQUIRE(cdoc["abc\0d"_s] == "ABCD");
     }
 
     SECTION("JsonVariant") {
-      doc["key"] = "hello";
-      REQUIRE(doc[doc["key"]] == "world");
-      REQUIRE(cdoc[cdoc["key"]] == "world");
+      doc["key1"] = "abc";
+      doc["key2"] = "abc\0d"_s;
+      doc["key3"] = "foo";
+
+      CHECK(doc[doc["key1"]] == "ABC");
+      CHECK(doc[doc["key2"]] == "ABCD");
+      CHECK(doc[doc["key3"]] == nullptr);
+      CHECK(doc[doc["key4"]] == nullptr);
+
+      CHECK(cdoc[cdoc["key1"]] == "ABC");
+      CHECK(cdoc[cdoc["key2"]] == "ABCD");
+      CHECK(cdoc[cdoc["key3"]] == nullptr);
+      CHECK(cdoc[cdoc["key4"]] == nullptr);
     }
 
     SECTION("supports operator|") {
-      REQUIRE((doc["hello"] | "nope") == "world"_s);
-      REQUIRE((doc["world"] | "nope") == "nope"_s);
+      REQUIRE((doc["abc"] | "nope") == "ABC"_s);
+      REQUIRE((doc["def"] | "nope") == "nope"_s);
     }
 
 #if defined(HAS_VARIABLE_LENGTH_ARRAY) && \
@@ -46,7 +59,6 @@ TEST_CASE("JsonDocument::operator[]") {
 
       REQUIRE(doc[vla] == "world");
       REQUIRE(cdoc[vla] == "world");
-      REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\"}");
     }
 #endif
   }

diff --git a/extras/tests/JsonObject/subscript.cpp b/extras/tests/JsonObject/subscript.cpp
@@ -253,9 +253,15 @@ TEST_CASE("JsonObject::operator[]") {
 
   SECTION("JsonVariant") {
     obj["hello"] = "world";
-    doc["key"] = "hello";
+    obj["a\0b"_s] = "ABC";
 
-    REQUIRE(obj[obj["key"]] == "world");
-    REQUIRE(obj[obj["foo"]] == nullptr);
+    doc["key1"] = "hello";
+    doc["key2"] = "a\0b"_s;
+    doc["key3"] = "foo";
+
+    REQUIRE(obj[obj["key1"]] == "world");
+    REQUIRE(obj[obj["key2"]] == "ABC");
+    REQUIRE(obj[obj["key3"]] == nullptr);
+    REQUIRE(obj[obj["key4"]] == nullptr);
   }
 }
diff --git a/extras/tests/JsonObjectConst/subscript.cpp b/extras/tests/JsonObjectConst/subscript.cpp
@@ -11,6 +11,7 @@
 TEST_CASE("JsonObjectConst::operator[]") {
   JsonDocument doc;
   doc["hello"] = "world";
+  doc["a\0b"_s] = "ABC";
   JsonObjectConst obj = doc.as<JsonObjectConst>();
 
   SECTION("supports const char*") {
@@ -19,6 +20,7 @@ TEST_CASE("JsonObjectConst::operator[]") {
 
   SECTION("supports std::string") {
     REQUIRE(obj["hello"_s] == "world");  // issue #2019
+    REQUIRE(obj["a\0b"_s] == "ABC");
   }
 
 #if defined(HAS_VARIABLE_LENGTH_ARRAY) && \
@@ -28,13 +30,17 @@ TEST_CASE("JsonObjectConst::operator[]") {
     char vla[i];
     strcpy(vla, "hello");
 
-    REQUIRE("world"_s == obj[vla]);
+    REQUIRE(obj[vla] == "world"_s);
   }
 #endif
 
   SECTION("supports JsonVariant") {
-    doc["key"] = "hello";
-    REQUIRE(obj[obj["key"]] == "world");
-    REQUIRE(obj[obj["foo"]] == nullptr);
+    doc["key1"] = "hello";
+    doc["key2"] = "a\0b"_s;
+    doc["key3"] = "foo";
+    REQUIRE(obj[obj["key1"]] == "world");
+    REQUIRE(obj[obj["key2"]] == "ABC");
+    REQUIRE(obj[obj["key3"]] == nullptr);
+    REQUIRE(obj[obj["key4"]] == nullptr);
   }
 }
diff --git a/extras/tests/JsonVariant/subscript.cpp b/extras/tests/JsonVariant/subscript.cpp
@@ -116,12 +116,19 @@ TEST_CASE("JsonVariant::operator[]") {
     }
 
     SECTION("use JsonVariant as key") {
-      object["a"] = "a";
-      object["b"] = "b";
-      object["c"] = "b";
-
-      REQUIRE(var[var["c"]] == "b");
-      REQUIRE(var[var["d"]].isNull());
+      object["a"] = "A";
+      object["ab"] = "AB";
+      object["ab\0c"_s] = "ABC";
+      object["key1"] = "a";
+      object["key2"] = "ab";
+      object["key3"] = "ab\0c"_s;
+      object["key4"] = "foo";
+
+      REQUIRE(var[var["key1"]] == "A");
+      REQUIRE(var[var["key2"]] == "AB");
+      REQUIRE(var[var["key3"]] == "ABC");
+      REQUIRE(var[var["key4"]].isNull());
+      REQUIRE(var[var["key5"]].isNull());
     }
   }
 

diff --git a/extras/tests/JsonVariantConst/subscript.cpp b/extras/tests/JsonVariantConst/subscript.cpp
@@ -50,37 +50,46 @@ TEST_CASE("JsonVariantConst::operator[]") {
 
   SECTION("object") {
     JsonObject object = doc.to<JsonObject>();
-    object["a"] = "A";
-    object["b"] = "B";
+    object["ab"_s] = "AB";
+    object["abc"_s] = "ABC";
+    object["abc\0d"_s] = "ABCD";
 
     SECTION("supports const char*") {
-      REQUIRE("A"_s == var["a"]);
-      REQUIRE("B"_s == var["b"]);
-      REQUIRE(var["c"].isNull());
+      REQUIRE(var["ab"] == "AB"_s);
+      REQUIRE(var["abc"] == "ABC"_s);
+      REQUIRE(var["def"].isNull());
       REQUIRE(var[0].isNull());
     }
 
     SECTION("supports std::string") {
-      REQUIRE("A"_s == var["a"_s]);
-      REQUIRE("B"_s == var["b"_s]);
-      REQUIRE(var["c"_s].isNull());
+      REQUIRE(var["ab"_s] == "AB"_s);
+      REQUIRE(var["abc"_s] == "ABC"_s);
+      REQUIRE(var["abc\0d"_s] == "ABCD"_s);
+      REQUIRE(var["def"_s].isNull());
     }
 
 #if defined(HAS_VARIABLE_LENGTH_ARRAY) && \
     !defined(SUBSCRIPT_CONFLICTS_WITH_BUILTIN_OPERATOR)
     SECTION("supports VLA") {
       size_t i = 16;
       char vla[i];
-      strcpy(vla, "a");
+      strcpy(vla, "abc");
 
-      REQUIRE("A"_s == var[vla]);
+      REQUIRE(var[vla] == "ABC"_s);
     }
 #endif
 
     SECTION("supports JsonVariant") {
-      object["c"] = "b";
-      REQUIRE(var[var["c"]] == "B");
-      REQUIRE(var[var["d"]].isNull());
+      object["key1"] = "ab";
+      object["key2"] = "abc";
+      object["key3"] = "abc\0d"_s;
+      object["key4"] = "foo";
+
+      REQUIRE(var[var["key1"]] == "AB"_s);
+      REQUIRE(var[var["key2"]] == "ABC"_s);
+      REQUIRE(var[var["key3"]] == "ABCD"_s);
+      REQUIRE(var[var["key4"]].isNull());
+      REQUIRE(var[var["key5"]].isNull());
     }
   }
 }
diff --git a/src/ArduinoJson/Document/JsonDocument.hpp b/src/ArduinoJson/Document/JsonDocument.hpp
@@ -239,8 +239,8 @@ class JsonDocument : public detail::VariantOperators<const JsonDocument&> {
   template <typename TVariant>
   detail::enable_if_t<detail::IsVariant<TVariant>::value, JsonVariantConst>
   operator[](const TVariant& key) const {
-    if (key.template is<const char*>())
-      return operator[](key.template as<const char*>());
+    if (key.template is<JsonString>())
+      return operator[](key.template as<JsonString>());
     if (key.template is<size_t>())
       return operator[](key.template as<size_t>());
     return {};

diff --git a/src/ArduinoJson/Object/JsonObject.hpp b/src/ArduinoJson/Object/JsonObject.hpp
@@ -121,10 +121,10 @@ class JsonObject : public detail::VariantOperators<JsonObject> {
   // https://arduinojson.org/v7/api/jsonobject/subscript/
   template <typename TVariant>
   detail::enable_if_t<detail::IsVariant<TVariant>::value,
-                      detail::MemberProxy<JsonObject, const char*>>
+                      detail::MemberProxy<JsonObject, JsonString>>
   operator[](const TVariant& key) const {
-    if (key.template is<const char*>())
-      return {*this, key.template as<const char*>()};
+    if (key.template is<JsonString>())
+      return {*this, key.template as<JsonString>()};
     else
       return {*this, nullptr};
   }

diff --git a/src/ArduinoJson/Object/JsonObjectConst.hpp b/src/ArduinoJson/Object/JsonObjectConst.hpp
@@ -121,8 +121,8 @@ class JsonObjectConst : public detail::VariantOperators<JsonObjectConst> {
   template <typename TVariant>
   detail::enable_if_t<detail::IsVariant<TVariant>::value, JsonVariantConst>
   operator[](const TVariant& key) const {
-    if (key.template is<const char*>())
-      return operator[](key.template as<const char*>());
+    if (key.template is<JsonString>())
+      return operator[](key.template as<JsonString>());
     else
       return JsonVariantConst();
   }

diff --git a/src/ArduinoJson/Variant/JsonVariantConst.hpp b/src/ArduinoJson/Variant/JsonVariantConst.hpp
@@ -136,7 +136,7 @@ class JsonVariantConst : public detail::VariantTag,
     if (key.template is<size_t>())
       return operator[](key.template as<size_t>());
     else
-      return operator[](key.template as<const char*>());
+      return operator[](key.template as<JsonString>());
   }
 
   // DEPRECATED: use obj[key].is<T>() instead

diff --git a/src/ArduinoJson/Variant/VariantRefBase.hpp b/src/ArduinoJson/Variant/VariantRefBase.hpp
@@ -207,7 +207,7 @@ class VariantRefBase : public VariantTag {
     if (key.template is<size_t>())
       return operator[](key.template as<size_t>());
     else
-      return operator[](key.template as<const char*>());
+      return operator[](key.template as<JsonString>());
   }
 
   // DEPRECATED: use add<JsonVariant>() instead