From cb7450bfb08e0cdb22017aecba0840224961d4e0 Mon Sep 17 00:00:00 2001 From: Jordan Bayles Date: Mon, 2 Mar 2026 19:16:53 -0800 Subject: [PATCH 1/5] Fix MSAN issue in #1626 This patch fixes an MSAN issue by changing CZString initialization. --- src/lib_json/json_value.cpp | 44 ++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index a875d28b2..640c7a2d2 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -253,20 +253,29 @@ Value::CZString::CZString(const CZString& other) { cstr_ = (other.storage_.policy_ != noDuplication && other.cstr_ != nullptr ? duplicateStringValue(other.cstr_, other.storage_.length_) : other.cstr_); - storage_.policy_ = - static_cast( - other.cstr_ - ? (static_cast(other.storage_.policy_) == - noDuplication - ? noDuplication - : duplicate) - : static_cast(other.storage_.policy_)) & - 3U; - storage_.length_ = other.storage_.length_; -} - -Value::CZString::CZString(CZString&& other) noexcept - : cstr_(other.cstr_), index_(other.index_) { + if (other.cstr_) { + storage_.policy_ = + static_cast( + other.cstr_ + ? (static_cast(other.storage_.policy_) == + noDuplication + ? noDuplication + : duplicate) + : static_cast(other.storage_.policy_)) & + 3U; + storage_.length_ = other.storage_.length_; + } else { + index_ = other.index_; + } +} + +Value::CZString::CZString(CZString&& other) noexcept : cstr_(other.cstr_) { + if (other.cstr_) { + storage_.policy_ = other.storage_.policy_; + storage_.length_ = other.storage_.length_; + } else { + index_ = other.index_; + } other.cstr_ = nullptr; } @@ -293,7 +302,12 @@ Value::CZString& Value::CZString::operator=(const CZString& other) { Value::CZString& Value::CZString::operator=(CZString&& other) noexcept { cstr_ = other.cstr_; - index_ = other.index_; + if (other.cstr_) { + storage_.policy_ = other.storage_.policy_; + storage_.length_ = other.storage_.length_; + } else { + index_ = other.index_; + } other.cstr_ = nullptr; return *this; } From 047edcb4b2d5655ef40cbf142a1205a404a41c0e Mon Sep 17 00:00:00 2001 From: Jordan Bayles Date: Mon, 2 Mar 2026 19:26:04 -0800 Subject: [PATCH 2/5] add test --- src/test_lib_json/main.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index e20723498..185053a54 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -449,6 +449,24 @@ JSONTEST_FIXTURE_LOCAL(ValueTest, resizeArray) { } } +JSONTEST_FIXTURE_LOCAL(ValueTest, copyMoveArray) { + Json::Value array; + array.append("item1"); + array.append("item2"); + + // Test Copy Constructor (covers CZString(const CZString&) with index) + Json::Value copy(array); + JSONTEST_ASSERT_EQUAL(copy.size(), 2); + JSONTEST_ASSERT_EQUAL(Json::Value("item1"), copy[0]); + JSONTEST_ASSERT_EQUAL(Json::Value("item2"), copy[1]); + + // Test Move Constructor (covers CZString(CZString&&) with index) + Json::Value moved(std::move(copy)); + JSONTEST_ASSERT_EQUAL(moved.size(), 2); + JSONTEST_ASSERT_EQUAL(Json::Value("item1"), moved[0]); + JSONTEST_ASSERT_EQUAL(Json::Value("item2"), moved[1]); +} + JSONTEST_FIXTURE_LOCAL(ValueTest, resizePopulatesAllMissingElements) { Json::ArrayIndex n = 10; Json::Value v; From f49d54769b3702a21131b8bdb42a6165aa10ebef Mon Sep 17 00:00:00 2001 From: Jordan Bayles Date: Mon, 2 Mar 2026 19:44:33 -0800 Subject: [PATCH 3/5] expand tests --- src/test_lib_json/main.cpp | 40 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/test_lib_json/main.cpp b/src/test_lib_json/main.cpp index ca49ed5a0..7ebd46692 100644 --- a/src/test_lib_json/main.cpp +++ b/src/test_lib_json/main.cpp @@ -150,6 +150,8 @@ struct ValueTest : JsonTest::TestCase { /// Normalize the representation of floating-point number by stripped leading /// 0 in exponent. static Json::String normalizeFloatingPointStr(const Json::String& s); + + void runCZStringTests(); }; Json::String ValueTest::normalizeFloatingPointStr(const Json::String& s) { @@ -167,6 +169,44 @@ Json::String ValueTest::normalizeFloatingPointStr(const Json::String& s) { return normalized + exponent; } +void ValueTest::runCZStringTests() { + // 1. Copy Constructor (Index) + Json::Value::CZString idx1(123); + Json::Value::CZString idx2(idx1); + JSONTEST_ASSERT_EQUAL(idx2.index(), 123); + + // 2. Move Constructor (Index) + Json::Value::CZString idx3(std::move(idx1)); + JSONTEST_ASSERT_EQUAL(idx3.index(), 123); + + // 3. Move Assignment (Index) + Json::Value::CZString idx4(456); + idx4 = std::move(idx3); + JSONTEST_ASSERT_EQUAL(idx4.index(), 123); + + // 4. Copy Constructor (String) + Json::Value::CZString str1("param", 5, + Json::Value::CZString::duplicateOnCopy); + Json::Value::CZString str2((str1)); // copy makes it duplicate (owning) + JSONTEST_ASSERT_STRING_EQUAL(str2.data(), "param"); + + // 5. Move Constructor (String) + // Move from Owning string (str2) + Json::Value::CZString str3(std::move(str2)); + JSONTEST_ASSERT_STRING_EQUAL(str3.data(), "param"); + + // 6. Move Assignment (String) + Json::Value::CZString str4("other", 5, + Json::Value::CZString::duplicateOnCopy); + Json::Value::CZString str5((str4)); // owning "other" + // Move-assign owning "param" (str3) into owning "other" (str5) + // This verifies we don't leak "other" (if fixed) and correctly take "param" + str5 = std::move(str3); + JSONTEST_ASSERT_STRING_EQUAL(str5.data(), "param"); +} + +JSONTEST_FIXTURE_LOCAL(ValueTest, CZStringCoverage) { runCZStringTests(); } + JSONTEST_FIXTURE_LOCAL(ValueTest, checkNormalizeFloatingPointStr) { struct TestData { std::string in; From 046c65c2ae93d3f90c44b01b88b5003419c82d95 Mon Sep 17 00:00:00 2001 From: Jordan Bayles Date: Mon, 2 Mar 2026 19:51:21 -0800 Subject: [PATCH 4/5] fix build --- include/json/value.h | 4 ++++ src/lib_json/json_value.cpp | 3 +++ 2 files changed, 7 insertions(+) diff --git a/include/json/value.h b/include/json/value.h index 5f6544329..8657e6cbc 100644 --- a/include/json/value.h +++ b/include/json/value.h @@ -50,6 +50,9 @@ #include #include +// Forward declaration for testing. +struct ValueTest; + #ifdef JSONCPP_HAS_STRING_VIEW #include #endif @@ -201,6 +204,7 @@ class JSON_API StaticString { */ class JSON_API Value { friend class ValueIteratorBase; + friend struct ::ValueTest; public: using Members = std::vector; diff --git a/src/lib_json/json_value.cpp b/src/lib_json/json_value.cpp index 640c7a2d2..74f77896f 100644 --- a/src/lib_json/json_value.cpp +++ b/src/lib_json/json_value.cpp @@ -301,6 +301,9 @@ Value::CZString& Value::CZString::operator=(const CZString& other) { } Value::CZString& Value::CZString::operator=(CZString&& other) noexcept { + if (cstr_ && storage_.policy_ == duplicate) { + releasePrefixedStringValue(const_cast(cstr_)); + } cstr_ = other.cstr_; if (other.cstr_) { storage_.policy_ = other.storage_.policy_; From 4479801cfdd7ee77ec9e9d219cdea45dc257d65d Mon Sep 17 00:00:00 2001 From: Jordan Bayles Date: Sun, 15 Mar 2026 19:12:38 -0700 Subject: [PATCH 5/5] Export CZString with JSON_API to fix Windows DLL linker errors --- include/json/value.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/json/value.h b/include/json/value.h index 8657e6cbc..a7a39a1a7 100644 --- a/include/json/value.h +++ b/include/json/value.h @@ -270,7 +270,7 @@ class JSON_API Value { private: #endif #ifndef JSONCPP_DOC_EXCLUDE_IMPLEMENTATION - class CZString { + class JSON_API CZString { public: enum DuplicationPolicy { noDuplication = 0, duplicate, duplicateOnCopy }; CZString(ArrayIndex index);