[c++] Fix base to derived deserialization

With this change deserialization will now fail by throwing `bond::CoreException` when `BT_STOP` is encountered while deserializing a base struct.

Fixes #742

CHANGELOG.md

@@ -48,6 +48,8 @@ different versioning scheme, following the Haskell community's
 * gRPC v1.7.1 is now required to use Bond-over-gRPC.
 * Fixed includes for gRPC services with events or parameterless methods.
   [Issue #735](https://github.com/Microsoft/bond/issues/735)
+* Fixed a bug which would read an unrelated struct's field(s) when deserializing a
+  base struct. [Issue #742](https://github.com/Microsoft/bond/issues/742)
 
 ### C# ###
 

cpp/inc/bond/core/detail/inheritance.h

@@ -101,10 +101,17 @@ class ParserInheritance
         // First we recurse into base structs (serialized data starts at the top of the hierarchy)
         // and then we read to the transform the fields of the top level struct.
         transform.Begin(T::metadata);
-        ReadBase(base_class<T>(), transform);
-        bool result = static_cast<Parser*>(this)->ReadFields(typename boost::mpl::begin<typename T::fields>::type(), transform);
+
+        bool done = ReadBase(base_class<T>(), transform);
+
+        if (!done)
+        {
+            done = static_cast<Parser*>(this)->ReadFields(typename boost::mpl::begin<typename T::fields>::type(), transform);
+        }
+
         transform.End();
-        return result;
+
+        return done;
     }
 
 
@@ -138,7 +145,7 @@ class ParserInheritance
     bool Read(const RuntimeSchema& schema, const Transform& transform)
     {
         // The logic is the same as for compile-time schemas, described in the comments above.
-        bool result;
+        bool done;
 
         typename base_input<Input>::type base(base_input<Input>::from(_input));
 
@@ -148,7 +155,7 @@ class ParserInheritance
 
             detail::StructBegin(_input, true);
 
-            result = Parser(base, _base).Read(schema.GetBaseSchema(), transform);
+            done = Parser(base, _base).Read(schema.GetBaseSchema(), transform);
 
             detail::StructEnd(_input, true);
 
@@ -158,14 +165,17 @@ class ParserInheritance
         {
             transform.Begin(schema.GetStruct().metadata);
 
-            if (schema.HasBase())
-                transform.Base(bonded<void, Input>(base, schema.GetBaseSchema(), true));
+            done = schema.HasBase() && transform.Base(bonded<void, Input>(base, schema.GetBaseSchema(), true));
+
+            if (!done)
+            {
+                done = static_cast<Parser*>(this)->ReadFields(schema, transform);
+            }
 
-            result = static_cast<Parser*>(this)->ReadFields(schema, transform);
             transform.End();
         }
 
-        return result;
+        return done;
     }
 
     Input _input;

cpp/inc/bond/core/parser.h

@@ -271,6 +271,8 @@ class DynamicParser
 
         ReadFields(fields, id, type, transform);
 
+        bool done;
+
         if (!_base)
         {
             // If we are not parsing a base class, and we still didn't get to
@@ -292,11 +294,17 @@ class DynamicParser
                 else
                     UnknownField(id, type, transform);
             }
+
+            done = false;
+        }
+        else
+        {
+            done = (type == bond::BT_STOP);
         }
 
         _input.ReadFieldEnd();
 
-        return false;
+        return done;
     }
 
 
@@ -469,6 +477,8 @@ class DynamicParser
             UnknownField(id, type, transform);
         }
 
+        bool done;
+
         if (!_base)
         {
             // If we are not parsing a base class, and we still didn't get to
@@ -494,11 +504,17 @@ class DynamicParser
                     UnknownField(id, type, transform);
                 }
             }
+
+            done = false;
+        }
+        else
+        {
+            done = (type == bond::BT_STOP);
         }
 
         _input.ReadFieldEnd();
 
-        return false;
+        return done;
     }
 
 

cpp/inc/bond/core/transforms.h

@@ -432,6 +432,9 @@ class RequiredFieldValiadator
 template <typename T>
 void RequiredFieldValiadator<T>::MissingFieldException() const
 {
+    // Force instantiation of template statics
+    (void)typename schema<T>::type();
+
     BOND_THROW(CoreException,
           "De-serialization failed: required field " << _required <<
           " is missing from " << schema<T>::type::metadata.qualified_name);
@@ -530,7 +533,12 @@ class To
     template <typename X>
     bool Base(const X& value) const
     {
-        return AssignToBase(_var, value);
+        if (AssignToBase(_var, value))
+        {
+            UnexpectedStructStopException();
+        }
+
+        return false;
     }
 
 
@@ -593,6 +601,16 @@ class To
     }
 
 private:
+    BOND_NORETURN void UnexpectedStructStopException() const
+    {
+        // Force instantiation of template statics
+        (void)typename schema<T>::type();
+
+        BOND_THROW(CoreException,
+            "De-serialization failed: unexpected struct stop encountered for "
+            << schema<T>::type::metadata.qualified_name);
+    }
+
     T& _var;
 };
 

cpp/test/core/inheritance_test.cpp

@@ -10,6 +10,34 @@ bool Compare(const ListWithBase& left, const ListOfBase& right)
         && Equal<Protocols>(left.v4, right.v4);
 }
 
+template <typename Reader, typename Writer>
+typename boost::enable_if<bond::uses_dynamic_parser<Reader> >::type
+DeserializeBaseToDerived()
+{
+    ListOfBondedBase obj;
+    GetBonded<Reader, Writer, ListOfBondedBase>(InitRandom<ListOfBondedBase>()).Deserialize(obj);
+
+    for (const auto& base : obj.l)
+    {
+        bond::bonded<StructWithBase> derived_bonded(base);
+        StructWithBase derived;
+        BOOST_CHECK_THROW(derived_bonded.Deserialize(derived), bond::CoreException);
+        BOOST_CHECK_THROW(bond::bonded<void>(derived_bonded).Deserialize(derived), bond::CoreException);
+    }
+}
+
+template <typename Reader, typename Writer>
+typename boost::disable_if<bond::uses_dynamic_parser<Reader> >::type
+DeserializeBaseToDerived()
+{}
+
+template <typename Reader, typename Writer>
+TEST_CASE_BEGIN(BaseToDerivedDeserializationTest)
+{
+    DeserializeBaseToDerived<Reader, Writer>();
+}
+TEST_CASE_END
+
 template <uint16_t N, typename Reader, typename Writer>
 void InheritanceTests(const char* name)
 {
@@ -50,6 +78,9 @@ void InheritanceTests(const char* name)
     // Deserialize as containers of base/partial hierarchy
     AddTestCase<TEST_ID(N),
         AllBindingAndMapping2, Reader, Writer, ListWithBase, ListOfBase>(suite, "Containers, partial hierarchy");
+
+    AddTestCase<TEST_ID(N),
+        BaseToDerivedDeserializationTest, Reader, Writer>(suite, "Base to derived deserialization");
 }
 
 

cpp/test/core/unit_test.bond

@@ -97,6 +97,12 @@ struct ListOfBase
 };
 
 
+struct ListOfBondedBase
+{
+    1: list<bonded<SimpleBase>> l;
+}
+
+
 struct NestedStruct1OptionalBondedView
 {
     1: bonded<SimpleStruct> s;

python/test/core/unit_test.py

@@ -389,11 +389,6 @@ def test_Bonded(self):
         src2.n2 = self.randomSimpleWithBase()
         dst2 = test.Bonded()
         Deserialize(Serialize(src), dst2)
-        # downcast bonded<SimpleStruct> to bonded<SimpleWithBase>
-        bonded = test.bonded_unittest_SimpleWithBase_(dst2.n2)
-        obj3 = test.SimpleWithBase()
-        bonded.Deserialize(obj3)
-        self.assertTrue(obj3, src2.n2)
 
     def test_Polymorphism(self):
         src = test.Bonded()