null in json data should transform to blank string

[rcdailey]

When I do a NVP serialization in a JSON archive, I expect the following:

{"foo":null}

to be treated the exact same as if it were:

{"foo":""}

when serializing the value into a std::string. At the moment this causes an exception due to type mismatch.

Is this an easy change to make? I can't think of any reason why it wouldn't be this flexible. This is kind of getting into the whole discussion of optional values (i.e. if "foo" did not exist, I wouldn't want it to throw either).

[rcdailey]

Here is an example patch that I played with that seems to get me the desired behavior. I tested in very limited cases, so I'm not sure if this is comprehensive or safe. I'll let you decide. I mostly just hacked around. Note that the paths in the diff are specific to a submodule I keep cereal in and does not map directly.

What do you think?

diff --git a/cereal/include/cereal/archives/json.hpp b/cereal/include/cereal/archives/json.hpp
index c4c61d0..afd978f 100644
--- a/cereal/include/cereal/archives/json.hpp
+++ b/cereal/include/cereal/archives/json.hpp
@@ -625,7 +625,7 @@ namespace cereal
       //! Loads a value from the current node - double overload
       void loadValue(double & val)      { search(); val = itsIteratorStack.back().value().GetDouble(); ++itsIteratorStack.back(); }
       //! Loads a value from the current node - string overload
-      void loadValue(std::string & val) { search(); val = itsIteratorStack.back().value().GetString(); ++itsIteratorStack.back(); }
+      void loadValue(std::string & val) { search(); char const* str = itsIteratorStack.back().value().GetString(); val = str ? str : ""; ++itsIteratorStack.back(); }
       //! Loads a nullptr from the current node
       void loadValue(std::nullptr_t&)   { search(); CEREAL_RAPIDJSON_ASSERT(itsIteratorStack.back().value().IsNull()); ++itsIteratorStack.back(); }

diff --git a/cereal/include/cereal/external/rapidjson/document.h b/cereal/include/cereal/external/rapidjson/document.h
index 43e15bc..cab9327 100644
--- a/cereal/include/cereal/external/rapidjson/document.h
+++ b/cereal/include/cereal/external/rapidjson/document.h
@@ -1669,12 +1669,12 @@ public:
     //!@name String
     //@{

-    const Ch* GetString() const { CEREAL_RAPIDJSON_ASSERT(IsString()); return (data_.f.flags & kInlineStrFlag) ? data_.ss.str : GetStringPointer(); }
+    const Ch* GetString() const { CEREAL_RAPIDJSON_ASSERT(IsString() || IsNull()); return (data_.f.flags & kInlineStrFlag) ? data_.ss.str : GetStringPointer(); }

     //! Get the length of string.
     /*! Since rapidjson permits "\\u0000" in the json string, strlen(v.GetString()) may not equal to v.GetStringLength().
     */
-    SizeType GetStringLength() const { CEREAL_RAPIDJSON_ASSERT(IsString()); return ((data_.f.flags & kInlineStrFlag) ? (data_.ss.GetLength()) : data_.s.length); }
+    SizeType GetStringLength() const { CEREAL_RAPIDJSON_ASSERT(IsString() || IsNull()); return ((data_.f.flags & kInlineStrFlag) ? (data_.ss.GetLength()) : data_.s.length); }

     //! Set this value as a string without copying source string.
     /*! This version has better performance with supplied length, and also support string containing null character.
@@ -1980,8 +1980,8 @@ private:

     template <typename SourceAllocator>
     bool StringEqual(const GenericValue<Encoding, SourceAllocator>& rhs) const {
-        CEREAL_RAPIDJSON_ASSERT(IsString());
-        CEREAL_RAPIDJSON_ASSERT(rhs.IsString());
+        CEREAL_RAPIDJSON_ASSERT(IsString() || IsNull());
+        CEREAL_RAPIDJSON_ASSERT(rhs.IsString() || rhs.IsNull());

         const SizeType len1 = GetStringLength();
         const SizeType len2 = rhs.GetStringLength();

[AzothAmmo]

I'm not overly familiar with the JSON spec. We use a very compliant parser internally (rapidjson) so my guess would be that the expectation of null and "" matching is not strictly valid JSON.

There's a related discussion in #310 about chars and JSON. I'm of the opinion that if it isn't strictly valid JSON we won't make changes to the core behavior in cereal. That being said you can definitely modify cereal to change this yourself, though as to whether your changes are safe enough is a better question for rapidjson.

[rcdailey]

The change I made was in cereal code on purpose. I don't think it makes sense to enforce all the unintuitive rules of JSON for what people use your library for. But that's just my opinion. I already think cereal is far too strict which limits its applicability.

On Fri, Jul 29, 2016, 5:20 PM Shane Grant notifications@github.com wrote:

I'm not overly familiar with the JSON spec. We use a very compliant parser internally (rapidjson) so my guess would be that the expectation of null and "" matching is not strictly valid JSON.

There's a related discussion in #310 https://github.com/USCiLab/cereal/issues/310 about chars and JSON. I'm of the opinion that if it isn't strictly valid JSON we won't make changes to the core behavior in cereal. That being said you can definitely modify cereal to change this yourself, though as to whether your changes are safe enough is a better question for rapidjson.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/USCiLab/cereal/issues/318#issuecomment-236308165, or mute the thread https://github.com/notifications/unsubscribe-auth/ABr6dmLBD-0IbONn5OqXo5wHVoXh_FSGks5qanySgaJpZM4JWhT- .