fix: make the JSONCPP_USE_SECURE_MEMORY build compile and pass (#1709)
Building with JSONCPP_USE_SECURE_MEMORY=1 fails in three separate ways,
each hidden behind the previous one. No CI job builds this configuration,
which is why they have accumulated.
1. allocator.h calls RtlSecureZeroMemory on the _WIN32 branch without
including <windows.h>, so MSVC fails with error C3861. Removing that
branch lets the portable volatile std::fill_n path handle Windows.
The same fill was zeroing n bytes rather than n * sizeof(T), so for
any T wider than a char, most of the allocation was never wiped.
2. jsontestrunner declares a std::string where the surrounding code uses
Json::String. The two are the same type only in default builds, so the
test runner does not compile under secure memory.
3. CZString's move-assignment released a key with
releasePrefixedStringValue, but CZString keys come from
duplicateStringValue and carry no length prefix. The destructor
already uses the matching releaseStringValue. Under the default
allocator this mismatch is survivable; under SecureAllocator the
suite segfaults.
Verified on MSVC 19.44, x64, C++17. Before: error C3861. With only the
first two fixed: builds, then jsoncpp_test SEGFAULT. With all three:
secure build compiles and 3/3 ctest suites pass. The default build is
unaffected and also 3/3.
Fixes #1399
Co-authored-by: Jordan Bayles <bayles.jordan@gmail.com>
diff --git a/include/json/allocator.h b/include/json/allocator.h
index 459c34c..fa79d97 100644
--- a/include/json/allocator.h
+++ b/include/json/allocator.h
@@ -43,10 +43,9 @@
// unlike memset.
#if defined(HAVE_MEMSET_S)
memset_s(p, n * sizeof(T), 0, n * sizeof(T));
-#elif defined(_WIN32)
- RtlSecureZeroMemory(p, n * sizeof(T));
#else
- std::fill_n(reinterpret_cast<volatile unsigned char*>(p), n, 0);
+ std::fill_n(reinterpret_cast<volatile unsigned char*>(p), n * sizeof(T),
+ static_cast<unsigned char>(0));
#endif
// free using "global operator delete"
diff --git a/src/jsontestrunner/main.cpp b/src/jsontestrunner/main.cpp
index ab6a800..e499276 100644
--- a/src/jsontestrunner/main.cpp
+++ b/src/jsontestrunner/main.cpp
@@ -328,7 +328,7 @@
return modern_return_code;
}
- const std::string filename =
+ const Json::String filename =
opts.path.substr(opts.path.find_last_of("\\/") + 1);
const bool should_run_legacy = (filename.rfind("legacy_", 0) == 0);
if (should_run_legacy) {
diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp
index 5823bd1..6b14c2d 100644
--- a/src/lib_json/json_value.cpp
+++ b/src/lib_json/json_value.cpp
@@ -324,7 +324,9 @@
Value::CZString& Value::CZString::operator=(CZString&& other) noexcept {
if (cstr_ && storage_.policy_ == duplicate) {
- releasePrefixedStringValue(const_cast<char*>(cstr_));
+ // CZString keys come from duplicateStringValue (no length prefix), so
+ // release with the matching non-prefixed variant, as the destructor does.
+ releaseStringValue(const_cast<char*>(cstr_), storage_.length_ + 1U);
}
cstr_ = other.cstr_;
if (other.cstr_) {
diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp
index 2e106bc..e09b87b 100644
--- a/src/test_lib_json/main.cpp
+++ b/src/test_lib_json/main.cpp
@@ -214,8 +214,8 @@
JSONTEST_FIXTURE_LOCAL(ValueTest, checkNormalizeFloatingPointStr) {
struct TestData {
- std::string in;
- std::string out;
+ Json::String in;
+ Json::String out;
} const testData[] = {
{"0.0", "0.0"},
{"0e0", "0e0"},
@@ -295,7 +295,7 @@
JSONTEST_ASSERT(foundId != nullptr);
JSONTEST_ASSERT_EQUAL(Json::Value(1234), *foundId);
- const std::string stringIdKey = "id";
+ const Json::String stringIdKey = "id";
const Json::Value* stringFoundId = object1_.find(stringIdKey);
JSONTEST_ASSERT(stringFoundId != nullptr);
JSONTEST_ASSERT_EQUAL(Json::Value(1234), *stringFoundId);
@@ -305,7 +305,7 @@
object1_.find(unknownIdKey, unknownIdKey + strlen(unknownIdKey));
JSONTEST_ASSERT_EQUAL(nullptr, foundUnknownId);
- const std::string stringUnknownIdKey = "unknown id";
+ const Json::String stringUnknownIdKey = "unknown id";
const Json::Value* stringFoundUnknownId = object1_.find(stringUnknownIdKey);
JSONTEST_ASSERT_EQUAL(nullptr, stringFoundUnknownId);
@@ -357,7 +357,7 @@
const Json::Value* stringFound = object2_.findString("string");
JSONTEST_ASSERT(stringFound != nullptr);
- JSONTEST_ASSERT_EQUAL(std::string{"string"}, *stringFound);
+ JSONTEST_ASSERT_EQUAL(Json::String{"string"}, *stringFound);
JSONTEST_ASSERT(object3_.findString("string") == nullptr);
const Json::Value* arrayFound = object2_.findArray("array");
@@ -2028,9 +2028,9 @@
JSONTEST_FIXTURE_LOCAL(ValueTest, WideString) {
// https://github.com/open-source-parsers/jsoncpp/issues/756
- const std::string uni =
+ const Json::String uni =
reinterpret_cast<const char*>(u8"\u5f0f\uff0c\u8fdb"); // "式，进"
- std::string styled;
+ Json::String styled;
{
Json::Value v;
v["abc"] = uni;
@@ -2039,7 +2039,7 @@
Json::Value root;
{
JSONCPP_STRING errs;
- std::istringstream iss(styled);
+ Json::IStringStream iss(styled);
bool ok = parseFromStream(Json::CharReaderBuilder(), iss, &root, &errs);
JSONTEST_ASSERT(ok);
if (!ok) {
@@ -2894,7 +2894,7 @@
JSONTEST_FIXTURE_LOCAL(StreamWriterTest, escapeControlCharacters) {
auto uEscape = [](unsigned ch) {
static const char h[] = "0123456789abcdef";
- std::string r = "\\u";
+ Json::String r = "\\u";
r += h[(ch >> (3 * 4)) & 0xf];
r += h[(ch >> (2 * 4)) & 0xf];
r += h[(ch >> (1 * 4)) & 0xf];
@@ -2931,8 +2931,8 @@
if (!emitUTF8 && i >= 0x80)
break; // The algorithm would try to parse UTF-8, so stop here.
- std::string raw({static_cast<char>(i)});
- std::string esc = raw;
+ Json::String raw({static_cast<char>(i)});
+ Json::String esc = raw;
if (i < 0x20)
esc = uEscape(i);
if (const char* shEsc = shortEscape(i))
@@ -2944,7 +2944,7 @@
Json::Value root;
root["test"] = raw;
JSONTEST_ASSERT_STRING_EQUAL(
- std::string("{\n\t\"test\" : \"").append(esc).append("\"\n}"),
+ Json::String("{\n\t\"test\" : \"").append(esc).append("\"\n}"),
Json::writeString(b, root))
<< ", emit=" << emitUTF8 << ", i=" << i << ", raw=\"" << raw << "\""
<< ", esc=\"" << esc << "\"";
@@ -3018,7 +3018,7 @@
template <typename Input>
void checkParse(Input&& input,
const std::vector<Json::Reader::StructuredError>& structured,
- const std::string& formatted) {
+ const Json::String& formatted) {
checkParse(input, structured);
JSONTEST_ASSERT_EQUAL(formatted, reader->getFormattedErrorMessages());
}
@@ -3793,7 +3793,7 @@
return [=](const Value& root) { JSONTEST_ASSERT_EQUAL(root, v); };
}
- static ValueCheck objGetAnd(std::string idx, ValueCheck f) {
+ static ValueCheck objGetAnd(Json::String idx, ValueCheck f) {
return [=](const Value& root) { f(root.get(idx, true)); };
}
@@ -4117,16 +4117,16 @@
j["k1"] = "a";
j["k2"] = "b";
- std::vector<std::string> keys;
- std::vector<std::string> values;
+ std::vector<Json::String> keys;
+ std::vector<Json::String> values;
for (const auto& member : j.members()) {
keys.push_back(member.name);
values.push_back(member.value.asString());
}
- JSONTEST_ASSERT((keys == std::vector<std::string>{"k1", "k2"}));
- JSONTEST_ASSERT((values == std::vector<std::string>{"a", "b"}));
+ JSONTEST_ASSERT((keys == std::vector<Json::String>{"k1", "k2"}));
+ JSONTEST_ASSERT((values == std::vector<Json::String>{"a", "b"}));
// Test modification through value reference
for (const auto& member : j.members()) {
@@ -4145,8 +4145,8 @@
values.push_back(member.value.asString());
}
- JSONTEST_ASSERT((keys == std::vector<std::string>{"k1", "k2"}));
- JSONTEST_ASSERT((values == std::vector<std::string>{"c", "c"}));
+ JSONTEST_ASSERT((keys == std::vector<Json::String>{"k1", "k2"}));
+ JSONTEST_ASSERT((values == std::vector<Json::String>{"c", "c"}));
#if __cplusplus >= 201703L
keys.clear();
@@ -4155,8 +4155,8 @@
keys.push_back(k);
values.push_back(v.asString());
}
- JSONTEST_ASSERT((keys == std::vector<std::string>{"k1", "k2"}));
- JSONTEST_ASSERT((values == std::vector<std::string>{"c", "c"}));
+ JSONTEST_ASSERT((keys == std::vector<Json::String>{"k1", "k2"}));
+ JSONTEST_ASSERT((values == std::vector<Json::String>{"c", "c"}));
#endif
}
@@ -4173,25 +4173,25 @@
Json::Value json;
json["k1"] = "a";
json["k2"] = "b";
- std::vector<std::string> values;
+ std::vector<Json::String> values;
for (auto it = json.end(); it != json.begin();) {
--it;
values.push_back(it->asString());
}
- JSONTEST_ASSERT((values == std::vector<std::string>{"b", "a"}));
+ JSONTEST_ASSERT((values == std::vector<Json::String>{"b", "a"}));
}
JSONTEST_FIXTURE_LOCAL(IteratorTest, reverseIterator) {
Json::Value json;
json["k1"] = "a";
json["k2"] = "b";
- std::vector<std::string> values;
+ std::vector<Json::String> values;
using Iter = decltype(json.begin());
auto re = std::reverse_iterator<Iter>(json.begin());
for (auto it = std::reverse_iterator<Iter>(json.end()); it != re; ++it) {
values.push_back(it->asString());
}
- JSONTEST_ASSERT((values == std::vector<std::string>{"b", "a"}));
+ JSONTEST_ASSERT((values == std::vector<Json::String>{"b", "a"}));
}
JSONTEST_FIXTURE_LOCAL(IteratorTest, distance) {
