Don't store string literals by pointer anymore

Fixes #2189
This commit is contained in:
Benoit Blanchon
2025-08-28 10:04:11 +02:00
parent 509807d3c2
commit dddc4912c4
30 changed files with 174 additions and 331 deletions

View File

@@ -54,7 +54,7 @@ TEST_CASE("BasicJsonDocument") {
doc["hello"] = "world";
auto copy = doc;
REQUIRE(copy.as<std::string>() == "{\"hello\":\"world\"}");
REQUIRE(allocatorLog == "AA");
REQUIRE(allocatorLog == "AAAAAA");
}
SECTION("capacity") {

View File

@@ -56,6 +56,7 @@ TEST_CASE("JsonArray::add(T)") {
REQUIRE(array[0].is<int>() == false);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}

View File

@@ -104,6 +104,8 @@ TEST_CASE("deserializeJson(MemberProxy)") {
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\",\"value\":[42]}");
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofString("value")),
});
}
}

View File

@@ -31,6 +31,7 @@ TEST_CASE("ElementProxy::add()") {
REQUIRE(doc.as<std::string>() == "[[\"world\"]]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}

View File

@@ -25,6 +25,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[42]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}
@@ -34,6 +35,8 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}
@@ -44,6 +47,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}
@@ -55,6 +59,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
@@ -71,6 +76,7 @@ TEST_CASE("MemberProxy::add()") {
REQUIRE(doc.as<std::string>() == "{\"hello\":[\"world\"]}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}

View File

@@ -32,6 +32,7 @@ TEST_CASE("JsonDocument::add(T)") {
REQUIRE(doc.as<std::string>() == "[\"hello\"]");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}

View File

@@ -64,6 +64,8 @@ TEST_CASE("JsonDocument constructor") {
REQUIRE(doc2.as<std::string>() == "{\"hello\":\"world\"}");
REQUIRE(spyingAllocator.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}
@@ -87,6 +89,7 @@ TEST_CASE("JsonDocument constructor") {
REQUIRE(doc2.as<std::string>() == "[\"hello\"]");
REQUIRE(spyingAllocator.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}

View File

@@ -37,7 +37,9 @@ TEST_CASE("JsonDocument::set()") {
doc.set("example");
REQUIRE(doc.as<const char*>() == "example"_s);
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofString("example")),
});
}
SECTION("const char*") {

View File

@@ -69,17 +69,8 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
REQUIRE(spyingAllocator.log() == AllocatorLog{});
}
SECTION("linked string") {
doc.set("hello");
doc.shrinkToFit();
REQUIRE(doc.as<std::string>() == "hello");
REQUIRE(spyingAllocator.log() == AllocatorLog{});
}
SECTION("owned string") {
doc.set("abcdefg"_s);
SECTION("string") {
doc.set("abcdefg");
REQUIRE(doc.as<std::string>() == "abcdefg");
doc.shrinkToFit();
@@ -101,20 +92,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
});
}
SECTION("linked key") {
doc["key"] = 42;
doc.shrinkToFit();
REQUIRE(doc.as<std::string>() == "{\"key\":42}");
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Reallocate(sizeofPool(), sizeofObject(1)),
});
}
SECTION("owned key") {
SECTION("object key") {
doc["abcdefg"_s] = 42;
doc.shrinkToFit();
@@ -128,20 +106,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
});
}
SECTION("linked string in array") {
doc.add("hello");
doc.shrinkToFit();
REQUIRE(doc.as<std::string>() == "[\"hello\"]");
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Reallocate(sizeofPool(), sizeofArray(1)),
});
}
SECTION("owned string in array") {
SECTION("string in array") {
doc.add("abcdefg"_s);
doc.shrinkToFit();
@@ -155,20 +120,7 @@ TEST_CASE("JsonDocument::shrinkToFit()") {
});
}
SECTION("linked string in object") {
doc["key"] = "hello";
doc.shrinkToFit();
REQUIRE(doc.as<std::string>() == "{\"key\":\"hello\"}");
REQUIRE(spyingAllocator.log() ==
AllocatorLog{
Allocate(sizeofPool()),
Reallocate(sizeofPool(), sizeofObject(1)),
});
}
SECTION("owned string in object") {
SECTION("string in object") {
doc["key"] = "abcdefg"_s;
doc.shrinkToFit();

View File

@@ -114,6 +114,7 @@ TEST_CASE("JsonDocument::operator[] key storage") {
REQUIRE(doc.as<std::string>() == "{\"hello\":0}");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}

View File

@@ -16,44 +16,18 @@ TEST_CASE("JsonObject::set()") {
JsonObject obj1 = doc1.to<JsonObject>();
JsonObject obj2 = doc2.to<JsonObject>();
SECTION("doesn't copy static string in key or value") {
SECTION("copy key and string value") {
obj1["hello"] = "world";
spy.clearLog();
bool success = obj2.set(obj1);
REQUIRE(success == true);
REQUIRE(obj2["hello"] == "world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}
SECTION("copy local string value") {
obj1["hello"] = "world"_s;
spy.clearLog();
bool success = obj2.set(obj1);
REQUIRE(success == true);
REQUIRE(obj2["hello"] == "world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}
SECTION("copy local key") {
obj1["hello"_s] = "world";
spy.clearLog();
bool success = obj2.set(obj1);
REQUIRE(success == true);
REQUIRE(obj2["hello"] == "world"_s);
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}
@@ -110,7 +84,7 @@ TEST_CASE("JsonObject::set()") {
}
SECTION("copy fails in the middle of an array") {
TimebombAllocator timebomb(1);
TimebombAllocator timebomb(2);
JsonDocument doc3(&timebomb);
JsonObject obj3 = doc3.to<JsonObject>();

View File

@@ -101,30 +101,8 @@ TEST_CASE("JsonObject::operator[]") {
obj[key] = 42;
REQUIRE(42 == obj[key]);
}
SECTION("should not duplicate const char*") {
SECTION("should duplicate key and value strings") {
obj["hello"] = "world";
REQUIRE(spy.log() == AllocatorLog{Allocate(sizeofPool())});
}
SECTION("should duplicate char* value") {
obj["hello"] = const_cast<char*>("world");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}
SECTION("should duplicate char* key") {
obj[const_cast<char*>("hello")] = "world";
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}
SECTION("should duplicate char* key&value") {
obj[const_cast<char*>("hello")] = const_cast<char*>("world");
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
@@ -132,46 +110,6 @@ TEST_CASE("JsonObject::operator[]") {
});
}
SECTION("should duplicate std::string value") {
obj["hello"] = "world"_s;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("world")),
});
}
SECTION("should duplicate std::string key") {
obj["hello"_s] = "world";
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}
SECTION("should duplicate std::string key&value") {
obj["hello"_s] = "world"_s;
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
Allocate(sizeofString("world")),
});
}
SECTION("should duplicate a non-static JsonString key") {
obj[JsonString("hello", false)] = "world";
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
Allocate(sizeofString("hello")),
});
}
SECTION("should not duplicate a static JsonString key") {
obj[JsonString("hello", true)] = "world";
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofPool()),
});
}
SECTION("should ignore null key") {
// object must have a value to make a call to strcmp()
obj["dummy"] = 42;

View File

@@ -185,7 +185,6 @@ TEST_CASE("JsonVariant::as()") {
REQUIRE(variant.as<long>() == 42L);
REQUIRE(variant.as<double>() == 42);
REQUIRE(variant.as<JsonString>() == "42");
REQUIRE(variant.as<JsonString>().isStatic() == true);
}
SECTION("set(\"hello\")") {
@@ -208,7 +207,6 @@ TEST_CASE("JsonVariant::as()") {
REQUIRE(variant.as<const char*>() == "4.2"_s);
REQUIRE(variant.as<std::string>() == "4.2"_s);
REQUIRE(variant.as<JsonString>() == "4.2");
REQUIRE(variant.as<JsonString>().isStatic() == false);
}
SECTION("set(std::string(\"123.45\"))") {
@@ -220,7 +218,6 @@ TEST_CASE("JsonVariant::as()") {
REQUIRE(variant.as<const char*>() == "123.45"_s);
REQUIRE(variant.as<std::string>() == "123.45"_s);
REQUIRE(variant.as<JsonString>() == "123.45");
REQUIRE(variant.as<JsonString>().isStatic() == false);
}
SECTION("set(\"true\")") {

View File

@@ -38,13 +38,15 @@ TEST_CASE("JsonVariant::set(JsonVariant)") {
REQUIRE(var1.as<std::string>() == "{\"value\":[42]}");
}
SECTION("stores const char* by reference") {
SECTION("stores string literals by copy") {
var1.set("hello!!");
spyingAllocator.clearLog();
var2.set(var1);
REQUIRE(spyingAllocator.log() == AllocatorLog{});
REQUIRE(spyingAllocator.log() == AllocatorLog{
Allocate(sizeofString("hello!!")),
});
}
SECTION("stores char* by copy") {

View File

@@ -9,6 +9,7 @@
#include "Literals.hpp"
using ArduinoJson::detail::sizeofObject;
using ArduinoJson::detail::sizeofString;
enum ErrorCode { ERROR_01 = 1, ERROR_10 = 10 };
@@ -21,9 +22,10 @@ TEST_CASE("JsonVariant::set() when there is enough memory") {
bool result = variant.set("hello\0world");
REQUIRE(result == true);
CHECK(variant ==
"hello"_s); // linked string cannot contain '\0' at the moment
CHECK(spy.log() == AllocatorLog{});
REQUIRE(variant == "hello\0world"_s); // stores by copy
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofString(11)),
});
}
SECTION("const char*") {
@@ -140,19 +142,7 @@ TEST_CASE("JsonVariant::set() when there is enough memory") {
});
}
SECTION("static JsonString") {
char str[16];
strcpy(str, "hello");
bool result = variant.set(JsonString(str, true));
strcpy(str, "world");
REQUIRE(result == true);
REQUIRE(variant == "world"); // stores by pointer
REQUIRE(spy.log() == AllocatorLog{});
}
SECTION("non-static JsonString") {
SECTION("JsonString") {
char str[16];
strcpy(str, "hello");

View File

@@ -13,7 +13,6 @@ TEST_CASE("JsonString") {
CHECK(s.isNull() == true);
CHECK(s.c_str() == 0);
CHECK(s.isStatic() == true);
CHECK(s == JsonString());
CHECK(s != "");
}
@@ -96,7 +95,6 @@ TEST_CASE("JsonString") {
JsonString s("hello world", 5);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
CHECK(s == "hello");
CHECK(s != "hello world");
}

View File

@@ -22,7 +22,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 11);
CHECK(s.isStatic() == true);
}
SECTION("null const char*") {
@@ -38,7 +37,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
CHECK(s.data() == p);
}
@@ -46,7 +44,6 @@ TEST_CASE("adaptString()") {
auto s = adaptString(static_cast<const char*>(0), 10);
CHECK(s.isNull() == true);
CHECK(s.isStatic() == false);
}
SECTION("non-null const char* + size") {
@@ -54,7 +51,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
}
SECTION("null Flash string") {
@@ -62,7 +58,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == true);
CHECK(s.size() == 0);
CHECK(s.isStatic() == false);
}
SECTION("non-null Flash string") {
@@ -70,7 +65,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
}
SECTION("std::string") {
@@ -79,7 +73,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
}
SECTION("Arduino String") {
@@ -88,7 +81,6 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
}
SECTION("custom_string") {
@@ -97,25 +89,14 @@ TEST_CASE("adaptString()") {
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
}
SECTION("JsonString linked") {
JsonString orig("hello", true);
SECTION("JsonString") {
JsonString orig("hello");
auto s = adaptString(orig);
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == true);
}
SECTION("JsonString copied") {
JsonString orig("hello", false);
auto s = adaptString(orig);
CHECK(s.isNull() == false);
CHECK(s.size() == 5);
CHECK(s.isStatic() == false);
}
}

View File

@@ -89,58 +89,71 @@ TEST_CASE("ARDUINOJSON_STRING_LENGTH_SIZE == 4") {
REQUIRE(err != DeserializationError::Ok);
}
SECTION("bin 32") {
auto str = std::string(65536, '?');
auto input = "\xc6\x00\x01\x00\x00"_s + str;
auto err = deserializeMsgPack(doc, input);
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.is<MsgPackBinary>());
auto binary = doc.as<MsgPackBinary>();
REQUIRE(binary.size() == 65536);
REQUIRE(binary.data() != nullptr);
REQUIRE(std::string(reinterpret_cast<const char*>(binary.data()),
binary.size()) == str);
}
SECTION("ext 32 deserialization") {
auto str = std::string(65536, '?');
auto input = "\xc9\x00\x01\x00\x00\x2a"_s + str;
auto err = deserializeMsgPack(doc, input);
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.is<MsgPackExtension>());
auto value = doc.as<MsgPackExtension>();
REQUIRE(value.type() == 42);
REQUIRE(value.size() == 65536);
REQUIRE(value.data() != nullptr);
REQUIRE(std::string(reinterpret_cast<const char*>(value.data()),
value.size()) == str);
}
}
SECTION("bin 32 deserialization") {
auto str = std::string(65536, '?');
auto input = "\xc6\x00\x01\x00\x00"_s + str;
SECTION("serializeMsgPack()") {
SECTION("bin 32 serialization") {
auto str = std::string(65536, '?');
doc.set(MsgPackBinary(str.data(), str.size()));
auto err = deserializeMsgPack(doc, input);
std::string output;
auto result = serializeMsgPack(doc, output);
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.is<MsgPackBinary>());
auto binary = doc.as<MsgPackBinary>();
REQUIRE(binary.size() == 65536);
REQUIRE(binary.data() != nullptr);
REQUIRE(std::string(reinterpret_cast<const char*>(binary.data()),
binary.size()) == str);
}
REQUIRE(result == 5 + str.size());
REQUIRE(output == "\xc6\x00\x01\x00\x00"_s + str);
}
SECTION("bin 32 serialization") {
auto str = std::string(65536, '?');
doc.set(MsgPackBinary(str.data(), str.size()));
SECTION("ext 32 serialization") {
auto str = std::string(65536, '?');
doc.set(MsgPackExtension(42, str.data(), str.size()));
std::string output;
auto result = serializeMsgPack(doc, output);
std::string output;
auto result = serializeMsgPack(doc, output);
REQUIRE(result == 5 + str.size());
REQUIRE(output == "\xc6\x00\x01\x00\x00"_s + str);
}
REQUIRE(result == 6 + str.size());
REQUIRE(output == "\xc9\x00\x01\x00\x00\x2a"_s + str);
}
SECTION("ext 32 deserialization") {
auto str = std::string(65536, '?');
auto input = "\xc9\x00\x01\x00\x00\x2a"_s + str;
SECTION("str 32 serialization") {
auto str = std::string(65536, '?');
doc.set(str);
auto err = deserializeMsgPack(doc, input);
std::string output;
auto result = serializeMsgPack(doc, output);
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.is<MsgPackExtension>());
auto value = doc.as<MsgPackExtension>();
REQUIRE(value.type() == 42);
REQUIRE(value.size() == 65536);
REQUIRE(value.data() != nullptr);
REQUIRE(std::string(reinterpret_cast<const char*>(value.data()),
value.size()) == str);
}
SECTION("ext 32 serialization") {
auto str = std::string(65536, '?');
doc.set(MsgPackExtension(42, str.data(), str.size()));
std::string output;
auto result = serializeMsgPack(doc, output);
REQUIRE(result == 6 + str.size());
REQUIRE(output == "\xc9\x00\x01\x00\x00\x2a"_s + str);
REQUIRE(result == 5 + str.size());
REQUIRE(output == "\xDB\x00\x01\x00\x00"_s + str);
}
}
}

View File

@@ -104,6 +104,8 @@ TEST_CASE("deserializeMsgPack(MemberProxy)") {
REQUIRE(err == DeserializationError::Ok);
REQUIRE(doc.as<std::string>() == "{\"hello\":\"world\",\"value\":[42]}");
REQUIRE(spy.log() == AllocatorLog{});
REQUIRE(spy.log() == AllocatorLog{
Allocate(sizeofString("value")),
});
}
}

View File

@@ -137,11 +137,12 @@ TEST_CASE("serialize MsgPack value") {
checkVariant(longest.c_str(), "\xDA\xFF\xFF"_s + longest);
}
#if ARDUINOJSON_STRING_LENGTH_SIZE > 2
SECTION("str 32") {
std::string shortest(65536, '?');
checkVariant(JsonString(shortest.c_str(), true), // force store by pointer
"\xDB\x00\x01\x00\x00"_s + shortest);
checkVariant(shortest.c_str(), "\xDB\x00\x01\x00\x00"_s + shortest);
}
#endif
SECTION("serialized(const char*)") {
checkVariant(serialized("\xDA\xFF\xFF"), "\xDA\xFF\xFF");

View File

@@ -30,7 +30,7 @@ TEST_CASE("StringBuffer") {
memcpy(ptr, "a\0b", 3);
sb.save(&variant);
REQUIRE(variant.type() == VariantType::OwnedString);
REQUIRE(variant.type() == VariantType::LongString);
auto str = variant.asString();
REQUIRE(str.size() == 3);
@@ -44,7 +44,7 @@ TEST_CASE("StringBuffer") {
strcpy(ptr, "alfa");
sb.save(&variant);
REQUIRE(variant.type() == VariantType::OwnedString);
REQUIRE(variant.type() == VariantType::LongString);
REQUIRE(variant.asString() == "alfa");
}
}