Skip to content

Commit 6e0a82a

Browse files
bblanchonbblanchon
committed
Make ElementProxy and MemberProxy non-copyable
1 parent 5e7653b commit 6e0a82a

File tree

10 files changed

+101
-63
lines changed

10 files changed

+101
-63
lines changed

‎CHANGELOG.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,36 @@ HEAD
55
----
66

77
* Fix support for NUL characters in `deserializeJson()`
8+
* Make `ElementProxy` and `MemberProxy` non-copyable
9+
10+
> ### BREAKING CHANGES
11+
>
12+
> In previous versions, `MemberProxy` (the class returned by `operator[]`) could lead to dangling pointers when used with a temporary string.
13+
> To prevent this issue, `MemberProxy` and `ElementProxy` are now non-copyable.
14+
>
15+
> 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:
16+
>
17+
> ```cpp
18+
> auto value = doc["key"];
19+
> ```
20+
>
21+
> To fix the issue, you must append either `.as<T>()` or `.to<T>()`, depending on the situation.
22+
>
23+
> For example, if you are extracting values from a JSON document, you should update like this:
24+
>
25+
> ```diff
26+
> - auto config = doc["config"];
27+
> + auto config = doc["config"].as<JsonObject>();
28+
> const char* name = config["name"];
29+
> ```
30+
>
31+
> However, if you are building a JSON document, you should update like this:
32+
>
33+
> ```diff
34+
> - auto config = doc["config"];
35+
> + auto config = doc["config"].to<JsonObject>();
36+
> config["name"] = "ArduinoJson";
37+
> ```
838
939
v7.2.1 (2024-11-15)
1040
------

‎extras/tests/Deprecated/containsKey.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ TEST_CASE("JsonDocument::containsKey()") {
6767

6868
TEST_CASE("MemberProxy::containsKey()") {
6969
JsonDocument doc;
70-
auto mp = doc["hello"];
70+
const auto& mp = doc["hello"];
7171

7272
SECTION("containsKey(const char*)") {
7373
mp["key"] = "value";

‎extras/tests/JsonDocument/ElementProxy.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ using ElementProxy = ArduinoJson::detail::ElementProxy<JsonDocument&>;
1212
TEST_CASE("ElementProxy::add()") {
1313
JsonDocument doc;
1414
doc.add<JsonVariant>();
15-
ElementProxy ep = doc[0];
15+
const ElementProxy& ep = doc[0];
1616

1717
SECTION("add(int)") {
1818
ep.add(42);
@@ -50,7 +50,7 @@ TEST_CASE("ElementProxy::add()") {
5050
TEST_CASE("ElementProxy::clear()") {
5151
JsonDocument doc;
5252
doc.add<JsonVariant>();
53-
ElementProxy ep = doc[0];
53+
const ElementProxy& ep = doc[0];
5454

5555
SECTION("size goes back to zero") {
5656
ep.add(42);
@@ -110,7 +110,7 @@ TEST_CASE("ElementProxy::operator==()") {
110110
TEST_CASE("ElementProxy::remove()") {
111111
JsonDocument doc;
112112
doc.add<JsonVariant>();
113-
ElementProxy ep = doc[0];
113+
const ElementProxy& ep = doc[0];
114114

115115
SECTION("remove(int)") {
116116
ep.add(1);
@@ -157,7 +157,7 @@ TEST_CASE("ElementProxy::remove()") {
157157

158158
TEST_CASE("ElementProxy::set()") {
159159
JsonDocument doc;
160-
ElementProxy ep = doc[0];
160+
const ElementProxy& ep = doc[0];
161161

162162
SECTION("set(int)") {
163163
ep.set(42);
@@ -195,7 +195,7 @@ TEST_CASE("ElementProxy::set()") {
195195
TEST_CASE("ElementProxy::size()") {
196196
JsonDocument doc;
197197
doc.add<JsonVariant>();
198-
ElementProxy ep = doc[0];
198+
const ElementProxy& ep = doc[0];
199199

200200
SECTION("returns 0") {
201201
REQUIRE(ep.size() == 0);
@@ -216,7 +216,7 @@ TEST_CASE("ElementProxy::size()") {
216216

217217
TEST_CASE("ElementProxy::operator[]") {
218218
JsonDocument doc;
219-
ElementProxy ep = doc[1];
219+
const ElementProxy& ep = doc[1];
220220

221221
SECTION("set member") {
222222
ep["world"] = 42;
@@ -247,7 +247,7 @@ TEST_CASE("ElementProxy cast to JsonVariantConst") {
247247
JsonDocument doc;
248248
doc[0] = "world";
249249

250-
const ElementProxy ep = doc[0];
250+
const ElementProxy& ep = doc[0];
251251

252252
JsonVariantConst var = ep;
253253

@@ -258,7 +258,7 @@ TEST_CASE("ElementProxy cast to JsonVariant") {
258258
JsonDocument doc;
259259
doc[0] = "world";
260260

261-
ElementProxy ep = doc[0];
261+
const ElementProxy& ep = doc[0];
262262

263263
JsonVariant var = ep;
264264

‎extras/tests/JsonDocument/MemberProxy.cpp‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ using ArduinoJson::detail::sizeofObject;
1616

1717
TEST_CASE("MemberProxy::add()") {
1818
JsonDocument doc;
19-
auto mp = doc["hello"];
19+
const auto& mp = doc["hello"];
2020

2121
SECTION("add(int)") {
2222
mp.add(42);
@@ -45,7 +45,7 @@ TEST_CASE("MemberProxy::add()") {
4545

4646
TEST_CASE("MemberProxy::clear()") {
4747
JsonDocument doc;
48-
auto mp = doc["hello"];
48+
const auto& mp = doc["hello"];
4949

5050
SECTION("size goes back to zero") {
5151
mp.add(42);
@@ -136,7 +136,7 @@ TEST_CASE("MemberProxy::operator|()") {
136136

137137
TEST_CASE("MemberProxy::remove()") {
138138
JsonDocument doc;
139-
auto mp = doc["hello"];
139+
const auto& mp = doc["hello"];
140140

141141
SECTION("remove(int)") {
142142
mp.add(1);
@@ -183,7 +183,7 @@ TEST_CASE("MemberProxy::remove()") {
183183

184184
TEST_CASE("MemberProxy::set()") {
185185
JsonDocument doc;
186-
auto mp = doc["hello"];
186+
const auto& mp = doc["hello"];
187187

188188
SECTION("set(int)") {
189189
mp.set(42);
@@ -220,7 +220,7 @@ TEST_CASE("MemberProxy::set()") {
220220

221221
TEST_CASE("MemberProxy::size()") {
222222
JsonDocument doc;
223-
auto mp = doc["hello"];
223+
const auto& mp = doc["hello"];
224224

225225
SECTION("returns 0") {
226226
REQUIRE(mp.size() == 0);
@@ -243,7 +243,7 @@ TEST_CASE("MemberProxy::size()") {
243243

244244
TEST_CASE("MemberProxy::operator[]") {
245245
JsonDocument doc;
246-
auto mp = doc["hello"];
246+
const auto& mp = doc["hello"];
247247

248248
SECTION("set member") {
249249
mp["world"] = 42;
@@ -262,7 +262,7 @@ TEST_CASE("MemberProxy cast to JsonVariantConst") {
262262
JsonDocument doc;
263263
doc["hello"] = "world";
264264

265-
const auto mp = doc["hello"];
265+
const auto& mp = doc["hello"];
266266

267267
JsonVariantConst var = mp;
268268

@@ -273,7 +273,7 @@ TEST_CASE("MemberProxy cast to JsonVariant") {
273273
JsonDocument doc;
274274
doc["hello"] = "world";
275275

276-
auto mp = doc["hello"];
276+
const auto& mp = doc["hello"];
277277

278278
JsonVariant var = mp;
279279

‎extras/tests/JsonDocument/issue1120.cpp‎

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,49 +12,41 @@ TEST_CASE("Issue #1120") {
1212

1313
SECTION("MemberProxy<std::string>::isNull()") {
1414
SECTION("returns false") {
15-
auto value = doc["contents"_s];
16-
CHECK(value.isNull() == false);
15+
CHECK(doc["contents"_s].isNull() == false);
1716
}
1817

1918
SECTION("returns true") {
20-
auto value = doc["zontents"_s];
21-
CHECK(value.isNull() == true);
19+
CHECK(doc["zontents"_s].isNull() == true);
2220
}
2321
}
2422

2523
SECTION("ElementProxy<MemberProxy<const char*> >::isNull()") {
2624
SECTION("returns false") { // Issue #1120
27-
auto value = doc["contents"][1];
28-
CHECK(value.isNull() == false);
25+
CHECK(doc["contents"][1].isNull() == false);
2926
}
3027

3128
SECTION("returns true") {
32-
auto value = doc["contents"][2];
33-
CHECK(value.isNull() == true);
29+
CHECK(doc["contents"][2].isNull() == true);
3430
}
3531
}
3632

3733
SECTION("MemberProxy<ElementProxy<MemberProxy>, const char*>::isNull()") {
3834
SECTION("returns false") {
39-
auto value = doc["contents"][1]["module"];
40-
CHECK(value.isNull() == false);
35+
CHECK(doc["contents"][1]["module"].isNull() == false);
4136
}
4237

4338
SECTION("returns true") {
44-
auto value = doc["contents"][1]["zodule"];
45-
CHECK(value.isNull() == true);
39+
CHECK(doc["contents"][1]["zodule"].isNull() == true);
4640
}
4741
}
4842

4943
SECTION("MemberProxy<ElementProxy<MemberProxy>, std::string>::isNull()") {
5044
SECTION("returns false") {
51-
auto value = doc["contents"][1]["module"_s];
52-
CHECK(value.isNull() == false);
45+
CHECK(doc["contents"][1]["module"_s].isNull() == false);
5346
}
5447

5548
SECTION("returns true") {
56-
auto value = doc["contents"][1]["zodule"_s];
57-
CHECK(value.isNull() == true);
49+
CHECK(doc["contents"][1]["zodule"_s].isNull() == true);
5850
}
5951
}
6052
}

‎src/ArduinoJson/Array/ElementProxy.hpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,18 @@ class ElementProxy : public VariantRefBase<ElementProxy<TUpstream>>,
1515
public VariantOperators<ElementProxy<TUpstream>> {
1616
friend class VariantAttorney;
1717

18+
friend class VariantRefBase<ElementProxy<TUpstream>>;
19+
20+
template <typename, typename>
21+
friend class MemberProxy;
22+
23+
template <typename>
24+
friend class ElementProxy;
25+
1826
public:
1927
ElementProxy(TUpstream upstream, size_t index)
2028
: upstream_(upstream), index_(index) {}
2129

22-
ElementProxy(const ElementProxy& src)
23-
: upstream_(src.upstream_), index_(src.index_) {}
24-
2530
ElementProxy& operator=(const ElementProxy& src) {
2631
this->set(src);
2732
return *this;
@@ -40,6 +45,9 @@ class ElementProxy : public VariantRefBase<ElementProxy<TUpstream>>,
4045
}
4146

4247
private:
48+
ElementProxy(const ElementProxy& src)
49+
: upstream_(src.upstream_), index_(src.index_) {}
50+
4351
ResourceManager* getResourceManager() const {
4452
return VariantAttorney::getResourceManager(upstream_);
4553
}

‎src/ArduinoJson/Array/JsonArray.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
116116
// https://arduinojson.org/v7/api/jsonarray/remove/
117117
template <typename TVariant>
118118
detail::enable_if_t<detail::IsVariant<TVariant>::value> remove(
119-
TVariant variant) const {
119+
const TVariant& variant) const {
120120
if (variant.template is<size_t>())
121121
remove(variant.template as<size_t>());
122122
}
@@ -143,7 +143,7 @@ class JsonArray : public detail::VariantOperators<JsonArray> {
143143
detail::ElementProxy<JsonArray>>
144144
operator[](const TVariant& variant) const {
145145
if (variant.template is<size_t>())
146-
return operator[](variant.template as<size_t>());
146+
return {*this, variant.template as<size_t>()};
147147
else
148148
return {*this, size_t(-1)};
149149
}

‎src/ArduinoJson/Object/MemberProxy.hpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,18 @@ class MemberProxy
1616
public VariantOperators<MemberProxy<TUpstream, AdaptedString>> {
1717
friend class VariantAttorney;
1818

19+
friend class VariantRefBase<MemberProxy<TUpstream, AdaptedString>>;
20+
21+
template <typename, typename>
22+
friend class MemberProxy;
23+
24+
template <typename>
25+
friend class ElementProxy;
26+
1927
public:
2028
MemberProxy(TUpstream upstream, AdaptedString key)
2129
: upstream_(upstream), key_(key) {}
2230

23-
MemberProxy(const MemberProxy& src)
24-
: upstream_(src.upstream_), key_(src.key_) {}
25-
2631
MemberProxy& operator=(const MemberProxy& src) {
2732
this->set(src);
2833
return *this;
@@ -41,6 +46,9 @@ class MemberProxy
4146
}
4247

4348
private:
49+
MemberProxy(const MemberProxy& src)
50+
: upstream_(src.upstream_), key_(src.key_) {}
51+
4452
ResourceManager* getResourceManager() const {
4553
return VariantAttorney::getResourceManager(upstream_);
4654
}

0 commit comments

Comments
 (0)