Commit 47c3c1ec authored by Philipp A. Hartmann's avatar Philipp A. Hartmann

Improved handling of NULL strings

 * Safely assert upon passing NULL string without length
   (requires usage of RAPIDJSON_ASSERT within an expression)
 * Allow using a NULL string together with an explicit length 0
   (GenericStringRef, GenericValue::SetString, ...), see #817
 * Add GenericValue::SetString(StringRefType, Allocator&) overload
 * Add tests for the various cases
parent f1ba61c7
......@@ -308,7 +308,7 @@ struct GenericStringRef {
*/
#endif
explicit GenericStringRef(const CharType* str)
: s(str), length(internal::StrLen(str)){ RAPIDJSON_ASSERT(s != 0); }
: s(str), length(((RAPIDJSON_ASSERT(str != 0)), internal::StrLen(str))) {}
//! Create constant string reference from pointer and length
#ifndef __clang__ // -Wdocumentation
......@@ -320,7 +320,7 @@ struct GenericStringRef {
*/
#endif
GenericStringRef(const CharType* str, SizeType len)
: s(str), length(len) { RAPIDJSON_ASSERT(s != 0); }
: s(RAPIDJSON_LIKELY(str) ? str : emptyString), length(len) { RAPIDJSON_ASSERT(str != 0 || len == 0u); }
GenericStringRef(const GenericStringRef& rhs) : s(rhs.s), length(rhs.length) {}
......@@ -331,6 +331,9 @@ struct GenericStringRef {
const SizeType length; //!< length of the string (excluding the trailing NULL terminator)
private:
/// Empty string - used when passing in a NULL pointer
static const Ch emptyString[];
//! Disallow construction from non-const array
template<SizeType N>
GenericStringRef(CharType (&str)[N]) /* = delete */;
......@@ -338,6 +341,9 @@ private:
GenericStringRef& operator=(const GenericStringRef& rhs) /* = delete */;
};
template<typename CharType>
const CharType GenericStringRef<CharType>::emptyString[] = { CharType() };
//! Mark a character pointer as constant string
/*! Mark a plain character pointer as a "string literal". This function
can be used to avoid copying a character string to be referenced as a
......@@ -352,7 +358,7 @@ private:
*/
template<typename CharType>
inline GenericStringRef<CharType> StringRef(const CharType* str) {
return GenericStringRef<CharType>(str, internal::StrLen(str));
return GenericStringRef<CharType>(str);
}
//! Mark a character pointer as constant string
......@@ -1762,7 +1768,7 @@ public:
\return The value itself for fluent API.
\post IsString() == true && GetString() != s && strcmp(GetString(),s) == 0 && GetStringLength() == length
*/
GenericValue& SetString(const Ch* s, SizeType length, Allocator& allocator) { this->~GenericValue(); SetStringRaw(StringRef(s, length), allocator); return *this; }
GenericValue& SetString(const Ch* s, SizeType length, Allocator& allocator) { return SetString(StringRef(s, length), allocator); }
//! Set this value as a string by copying from source string.
/*! \param s source string.
......@@ -1770,7 +1776,15 @@ public:
\return The value itself for fluent API.
\post IsString() == true && GetString() != s && strcmp(GetString(),s) == 0 && GetStringLength() == length
*/
GenericValue& SetString(const Ch* s, Allocator& allocator) { return SetString(s, internal::StrLen(s), allocator); }
GenericValue& SetString(const Ch* s, Allocator& allocator) { return SetString(StringRef(s), allocator); }
//! Set this value as a string by copying from source string.
/*! \param s source string reference
\param allocator Allocator for allocating copied buffer. Commonly use GenericDocument::GetAllocator().
\return The value itself for fluent API.
\post IsString() == true && GetString() != s.s && strcmp(GetString(),s) == 0 && GetStringLength() == length
*/
GenericValue& SetString(StringRefType s, Allocator& allocator) { this->~GenericValue(); SetStringRaw(s, allocator); return *this; }
#if RAPIDJSON_HAS_STDSTRING
//! Set this value as a string by copying from source string.
......@@ -1780,7 +1794,7 @@ public:
\post IsString() == true && GetString() != s.data() && strcmp(GetString(),s.data() == 0 && GetStringLength() == s.size()
\note Requires the definition of the preprocessor symbol \ref RAPIDJSON_HAS_STDSTRING.
*/
GenericValue& SetString(const std::basic_string<Ch>& s, Allocator& allocator) { return SetString(s.data(), SizeType(s.size()), allocator); }
GenericValue& SetString(const std::basic_string<Ch>& s, Allocator& allocator) { return SetString(StringRef(s), allocator); }
#endif
//@}
......
......@@ -857,9 +857,46 @@ TEST(Value, String) {
}
// Issue 226: Value of string type should not point to NULL
TEST(Value, SetStringNullException) {
Value v;
EXPECT_THROW(v.SetString(0, 0), AssertException);
TEST(Value, SetStringNull) {
MemoryPoolAllocator<> allocator;
const char* nullPtr = 0;
{
// Construction with string type creates empty string
Value v(kStringType);
EXPECT_NE(v.GetString(), nullPtr); // non-null string returned
EXPECT_EQ(v.GetStringLength(), 0u);
// Construction from/setting to null without length not allowed
EXPECT_THROW(Value(StringRef(nullPtr)), AssertException);
EXPECT_THROW(Value(StringRef(nullPtr), allocator), AssertException);
EXPECT_THROW(v.SetString(nullPtr, allocator), AssertException);
// Non-empty length with null string is not allowed
EXPECT_THROW(v.SetString(nullPtr, 17u), AssertException);
EXPECT_THROW(v.SetString(nullPtr, 42u, allocator), AssertException);
// Setting to null string with empty length is allowed
v.SetString(nullPtr, 0u);
EXPECT_NE(v.GetString(), nullPtr); // non-null string returned
EXPECT_EQ(v.GetStringLength(), 0u);
v.SetNull();
v.SetString(nullPtr, 0u, allocator);
EXPECT_NE(v.GetString(), nullPtr); // non-null string returned
EXPECT_EQ(v.GetStringLength(), 0u);
}
// Construction with null string and empty length is allowed
{
Value v(nullPtr,0u);
EXPECT_NE(v.GetString(), nullPtr); // non-null string returned
EXPECT_EQ(v.GetStringLength(), 0u);
}
{
Value v(nullPtr, 0u, allocator);
EXPECT_NE(v.GetString(), nullPtr); // non-null string returned
EXPECT_EQ(v.GetStringLength(), 0u);
}
}
template <typename T, typename Allocator>
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment