Make ElementProxy and MemberProxy non-copyable

This commit is contained in:
Benoit Blanchon
2024-11-25 10:46:21 +01:00
parent 5e7653b36a
commit 57a9c50b38
10 changed files with 105 additions and 63 deletions

View File

@ -5,6 +5,36 @@ HEAD
----
* Fix support for NUL characters in `deserializeJson()`
* Make `ElementProxy` and `MemberProxy` non-copyable
> ### BREAKING CHANGES
>
> In previous versions, `MemberProxy` (the class returned by `operator[]`) could lead to dangling pointers when used with a temporary string.
> To prevent this issue, `MemberProxy` and `ElementProxy` are now non-copyable.
>
> Your code is likely to be affected if you use `auto` to store the result of `operator[]`. For example, the following line won't compile anymore:
>
> ```cpp
> auto value = doc["key"];
> ```
>
> To fix the issue, you must append either `.as<T>()` or `.to<T>()`, depending on the situation.
>
> For example, if you are extracting values from a JSON document, you should update like this:
>
> ```diff
> - auto config = doc["config"];
> + auto config = doc["config"].as<JsonObject>();
> const char* name = config["name"];
> ```
>
> However, if you are building a JSON document, you should update like this:
>
> ```diff
> - auto config = doc["config"];
> + auto config = doc["config"].to<JsonObject>();
> config["name"] = "ArduinoJson";
> ```
v7.2.1 (2024-11-15)
------

View File

@ -67,7 +67,7 @@ TEST_CASE("JsonDocument::containsKey()") {
TEST_CASE("MemberProxy::containsKey()") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("containsKey(const char*)") {
mp["key"] = "value";

View File

@ -12,7 +12,7 @@ using ElementProxy = ArduinoJson::detail::ElementProxy<JsonDocument&>;
TEST_CASE("ElementProxy::add()") {
JsonDocument doc;
doc.add<JsonVariant>();
ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
SECTION("add(int)") {
ep.add(42);
@ -50,7 +50,7 @@ TEST_CASE("ElementProxy::add()") {
TEST_CASE("ElementProxy::clear()") {
JsonDocument doc;
doc.add<JsonVariant>();
ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
SECTION("size goes back to zero") {
ep.add(42);
@ -110,7 +110,7 @@ TEST_CASE("ElementProxy::operator==()") {
TEST_CASE("ElementProxy::remove()") {
JsonDocument doc;
doc.add<JsonVariant>();
ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
SECTION("remove(int)") {
ep.add(1);
@ -157,7 +157,7 @@ TEST_CASE("ElementProxy::remove()") {
TEST_CASE("ElementProxy::set()") {
JsonDocument doc;
ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
SECTION("set(int)") {
ep.set(42);
@ -195,7 +195,7 @@ TEST_CASE("ElementProxy::set()") {
TEST_CASE("ElementProxy::size()") {
JsonDocument doc;
doc.add<JsonVariant>();
ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
SECTION("returns 0") {
REQUIRE(ep.size() == 0);
@ -216,7 +216,7 @@ TEST_CASE("ElementProxy::size()") {
TEST_CASE("ElementProxy::operator[]") {
JsonDocument doc;
ElementProxy ep = doc[1];
const ElementProxy& ep = doc[1];
SECTION("set member") {
ep["world"] = 42;
@ -247,7 +247,7 @@ TEST_CASE("ElementProxy cast to JsonVariantConst") {
JsonDocument doc;
doc[0] = "world";
const ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
JsonVariantConst var = ep;
@ -258,7 +258,7 @@ TEST_CASE("ElementProxy cast to JsonVariant") {
JsonDocument doc;
doc[0] = "world";
ElementProxy ep = doc[0];
const ElementProxy& ep = doc[0];
JsonVariant var = ep;

View File

@ -16,7 +16,7 @@ using ArduinoJson::detail::sizeofObject;
TEST_CASE("MemberProxy::add()") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("add(int)") {
mp.add(42);
@ -45,7 +45,7 @@ TEST_CASE("MemberProxy::add()") {
TEST_CASE("MemberProxy::clear()") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("size goes back to zero") {
mp.add(42);
@ -136,7 +136,7 @@ TEST_CASE("MemberProxy::operator|()") {
TEST_CASE("MemberProxy::remove()") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("remove(int)") {
mp.add(1);
@ -183,7 +183,7 @@ TEST_CASE("MemberProxy::remove()") {
TEST_CASE("MemberProxy::set()") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("set(int)") {
mp.set(42);
@ -220,7 +220,7 @@ TEST_CASE("MemberProxy::set()") {
TEST_CASE("MemberProxy::size()") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("returns 0") {
REQUIRE(mp.size() == 0);
@ -243,7 +243,7 @@ TEST_CASE("MemberProxy::size()") {
TEST_CASE("MemberProxy::operator[]") {
JsonDocument doc;
auto mp = doc["hello"];
const auto& mp = doc["hello"];
SECTION("set member") {
mp["world"] = 42;
@ -262,7 +262,7 @@ TEST_CASE("MemberProxy cast to JsonVariantConst") {
JsonDocument doc;
doc["hello"] = "world";
const auto mp = doc["hello"];
const auto& mp = doc["hello"];
JsonVariantConst var = mp;
@ -273,7 +273,7 @@ TEST_CASE("MemberProxy cast to JsonVariant") {
JsonDocument doc;
doc["hello"] = "world";
auto mp = doc["hello"];
const auto& mp = doc["hello"];
JsonVariant var = mp;

View File

@ -12,49 +12,41 @@ TEST_CASE("Issue #1120") {
SECTION("MemberProxy<std::string>::isNull()") {
SECTION("returns false") {
auto value = doc["contents"_s];
CHECK(value.isNull() == false);
CHECK(doc["contents"_s].isNull() == false);
}
SECTION("returns true") {
auto value = doc["zontents"_s];
CHECK(value.isNull() == true);
CHECK(doc["zontents"_s].isNull() == true);
}
}
SECTION("ElementProxy<MemberProxy<const char*> >::isNull()") {
SECTION("returns false") { // Issue #1120
auto value = doc["contents"][1];
CHECK(value.isNull() == false);
CHECK(doc["contents"][1].isNull() == false);
}
SECTION("returns true") {
auto value = doc["contents"][2];
CHECK(value.isNull() == true);
CHECK(doc["contents"][2].isNull() == true);
}
}
SECTION("MemberProxy<ElementProxy<MemberProxy>, const char*>::isNull()") {
SECTION("returns false") {
auto value = doc["contents"][1]["module"];
CHECK(value.isNull() == false);
CHECK(doc["contents"][1]["module"].isNull() == false);
}
SECTION("returns true") {
auto value = doc["contents"][1]["zodule"];
CHECK(value.isNull() == true);
CHECK(doc["contents"][1]["zodule"].isNull() == true);
}
}
SECTION("MemberProxy<ElementProxy<MemberProxy>, std::string>::isNull()") {
SECTION("returns false") {
auto value = doc["contents"][1]["module"_s];
CHECK(value.isNull() == false);
CHECK(doc["contents"][1]["module"_s].isNull() == false);
}
SECTION("returns true") {
auto value = doc["contents"][1]["zodule"_s];
CHECK(value.isNull() == true);
CHECK(doc["contents"][1]["zodule"_s].isNull() == true);
}
}
}

View File

@ -15,13 +15,18 @@ class ElementProxy : public VariantRefBase<ElementProxy<TUpstream>>,
public VariantOperators<ElementProxy<TUpstream>> {
friend class VariantAttorney;
friend class VariantRefBase<ElementProxy<TUpstream>>;
template <typename, typename>
friend class MemberProxy;
template <typename>
friend class ElementProxy;
public:
ElementProxy(TUpstream upstream, size_t index)
: upstream_(upstream), index_(index) {}
ElementProxy(const ElementProxy& src)
: upstream_(src.upstream_), index_(src.index_) {}
ElementProxy& operator=(const ElementProxy& src) {
this->set(src);
return *this;
@ -40,6 +45,11 @@ class ElementProxy : public VariantRefBase<ElementProxy<TUpstream>>,
}
private:
// clang-format off
ElementProxy(const ElementProxy& src) // Error here? See https://arduinojson.org/v7/proxy-non-copyable/
: upstream_(src.upstream_), index_(src.index_) {}
// clang-format on
ResourceManager* getResourceManager() const {
return VariantAttorney::getResourceManager(upstream_);
}

View File

@ -116,7 +116,7 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
// https://arduinojson.org/v7/api/jsonarray/remove/
template <typename TVariant>
detail::enable_if_t<detail::IsVariant<TVariant>::value> remove(
TVariant variant) const {
const TVariant& variant) const {
if (variant.template is<size_t>())
remove(variant.template as<size_t>());
}
@ -143,7 +143,7 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
detail::ElementProxy<JsonArray>>
operator[](const TVariant& variant) const {
if (variant.template is<size_t>())
return operator[](variant.template as<size_t>());
return {*this, variant.template as<size_t>()};
else
return {*this, size_t(-1)};
}

View File

@ -16,13 +16,18 @@ class MemberProxy
public VariantOperators<MemberProxy<TUpstream, AdaptedString>> {
friend class VariantAttorney;
friend class VariantRefBase<MemberProxy<TUpstream, AdaptedString>>;
template <typename, typename>
friend class MemberProxy;
template <typename>
friend class ElementProxy;
public:
MemberProxy(TUpstream upstream, AdaptedString key)
: upstream_(upstream), key_(key) {}
MemberProxy(const MemberProxy& src)
: upstream_(src.upstream_), key_(src.key_) {}
MemberProxy& operator=(const MemberProxy& src) {
this->set(src);
return *this;
@ -41,6 +46,11 @@ class MemberProxy
}
private:
// clang-format off
MemberProxy(const MemberProxy& src) // Error here? See https://arduinojson.org/v7/proxy-non-copyable/
: upstream_(src.upstream_), key_(src.key_) {}
// clang-format on
ResourceManager* getResourceManager() const {
return VariantAttorney::getResourceManager(upstream_);
}

View File

@ -51,7 +51,7 @@ struct VariantOperators : VariantOperatorTag {
// JsonVariant operator|(JsonVariant, JsonVariant)
template <typename T>
friend enable_if_t<IsVariant<T>::value, JsonVariantConst> operator|(
const TVariant& variant, T defaultValue) {
const TVariant& variant, const T& defaultValue) {
if (variant)
return variant;
else
@ -60,38 +60,38 @@ struct VariantOperators : VariantOperatorTag {
// value == TVariant
template <typename T>
friend bool operator==(T* lhs, TVariant rhs) {
friend bool operator==(T* lhs, const TVariant& rhs) {
return compare(rhs, lhs) == COMPARE_RESULT_EQUAL;
}
template <typename T>
friend bool operator==(const T& lhs, TVariant rhs) {
friend bool operator==(const T& lhs, const TVariant& rhs) {
return compare(rhs, lhs) == COMPARE_RESULT_EQUAL;
}
// TVariant == value
template <typename T>
friend bool operator==(TVariant lhs, T* rhs) {
friend bool operator==(const TVariant& lhs, T* rhs) {
return compare(lhs, rhs) == COMPARE_RESULT_EQUAL;
}
template <typename T>
friend enable_if_t<!is_base_of<VariantOperatorTag, T>::value, bool>
operator==(TVariant lhs, const T& rhs) {
operator==(const TVariant& lhs, const T& rhs) {
return compare(lhs, rhs) == COMPARE_RESULT_EQUAL;
}
// value != TVariant
template <typename T>
friend bool operator!=(T* lhs, TVariant rhs) {
friend bool operator!=(T* lhs, const TVariant& rhs) {
return compare(rhs, lhs) != COMPARE_RESULT_EQUAL;
}
template <typename T>
friend bool operator!=(const T& lhs, TVariant rhs) {
friend bool operator!=(const T& lhs, const TVariant& rhs) {
return compare(rhs, lhs) != COMPARE_RESULT_EQUAL;
}
// TVariant != value
template <typename T>
friend bool operator!=(TVariant lhs, T* rhs) {
friend bool operator!=(const TVariant& lhs, T* rhs) {
return compare(lhs, rhs) != COMPARE_RESULT_EQUAL;
}
template <typename T>
@ -102,17 +102,17 @@ struct VariantOperators : VariantOperatorTag {
// value < TVariant
template <typename T>
friend bool operator<(T* lhs, TVariant rhs) {
friend bool operator<(T* lhs, const TVariant& rhs) {
return compare(rhs, lhs) == COMPARE_RESULT_GREATER;
}
template <typename T>
friend bool operator<(const T& lhs, TVariant rhs) {
friend bool operator<(const T& lhs, const TVariant& rhs) {
return compare(rhs, lhs) == COMPARE_RESULT_GREATER;
}
// TVariant < value
template <typename T>
friend bool operator<(TVariant lhs, T* rhs) {
friend bool operator<(const TVariant& lhs, T* rhs) {
return compare(lhs, rhs) == COMPARE_RESULT_LESS;
}
template <typename T>
@ -123,17 +123,17 @@ struct VariantOperators : VariantOperatorTag {
// value <= TVariant
template <typename T>
friend bool operator<=(T* lhs, TVariant rhs) {
friend bool operator<=(T* lhs, const TVariant& rhs) {
return (compare(rhs, lhs) & COMPARE_RESULT_GREATER_OR_EQUAL) != 0;
}
template <typename T>
friend bool operator<=(const T& lhs, TVariant rhs) {
friend bool operator<=(const T& lhs, const TVariant& rhs) {
return (compare(rhs, lhs) & COMPARE_RESULT_GREATER_OR_EQUAL) != 0;
}
// TVariant <= value
template <typename T>
friend bool operator<=(TVariant lhs, T* rhs) {
friend bool operator<=(const TVariant& lhs, T* rhs) {
return (compare(lhs, rhs) & COMPARE_RESULT_LESS_OR_EQUAL) != 0;
}
template <typename T>
@ -144,17 +144,17 @@ struct VariantOperators : VariantOperatorTag {
// value > TVariant
template <typename T>
friend bool operator>(T* lhs, TVariant rhs) {
friend bool operator>(T* lhs, const TVariant& rhs) {
return compare(rhs, lhs) == COMPARE_RESULT_LESS;
}
template <typename T>
friend bool operator>(const T& lhs, TVariant rhs) {
friend bool operator>(const T& lhs, const TVariant& rhs) {
return compare(rhs, lhs) == COMPARE_RESULT_LESS;
}
// TVariant > value
template <typename T>
friend bool operator>(TVariant lhs, T* rhs) {
friend bool operator>(const TVariant& lhs, T* rhs) {
return compare(lhs, rhs) == COMPARE_RESULT_GREATER;
}
template <typename T>
@ -165,22 +165,22 @@ struct VariantOperators : VariantOperatorTag {
// value >= TVariant
template <typename T>
friend bool operator>=(T* lhs, TVariant rhs) {
friend bool operator>=(T* lhs, const TVariant& rhs) {
return (compare(rhs, lhs) & COMPARE_RESULT_LESS_OR_EQUAL) != 0;
}
template <typename T>
friend bool operator>=(const T& lhs, TVariant rhs) {
friend bool operator>=(const T& lhs, const TVariant& rhs) {
return (compare(rhs, lhs) & COMPARE_RESULT_LESS_OR_EQUAL) != 0;
}
// TVariant >= value
template <typename T>
friend bool operator>=(TVariant lhs, T* rhs) {
friend bool operator>=(const TVariant& lhs, T* rhs) {
return (compare(lhs, rhs) & COMPARE_RESULT_GREATER_OR_EQUAL) != 0;
}
template <typename T>
friend enable_if_t<!is_base_of<VariantOperatorTag, T>::value, bool>
operator>=(TVariant lhs, const T& rhs) {
operator>=(const TVariant& lhs, const T& rhs) {
return (compare(lhs, rhs) & COMPARE_RESULT_GREATER_OR_EQUAL) != 0;
}
};

View File

@ -119,7 +119,7 @@ inline bool VariantRefBase<TDerived>::is() const {
template <typename TDerived>
inline ElementProxy<TDerived> VariantRefBase<TDerived>::operator[](
size_t index) const {
return ElementProxy<TDerived>(derived(), index);
return {derived(), index};
}
template <typename TDerived>