Add class cycle detection during unmarshaling (#946)

Add support for detection of class cycles during unmarshaling in
languages which do no have garbage collection: C++, Swift, and Objective-C.
A `MarshalException` is thrown when a cycle is detected.

The property `Ice.AcceptClassCycles` can be set to a value greater than `0`
to change this behavior.

5 files changed, 26 insertions, 5 deletions

diff --git a/cpp/src/Ice/InputStream.cpp b/cpp/src/Ice/InputStream.cpp
index d0c4c91d9fc..3c4686e979d 100644
--- a/cpp/src/Ice/InputStream.cpp
+++ b/cpp/src/Ice/InputStream.cpp
@@ -1819,12 +1819,17 @@ Ice::InputStream::EncapsDecoder::addPatchEntry(Int index, PatchFunc patchFunc, v
     assert(index > 0);
 
     //
-    // Check if we already unmarshaled the object. If that's the case,
-    // just patch the object smart pointer and we're done.
+    // Check if we already unmarshaled the object. If that's the case, just patch the object smart pointer
+    // and we're done. A null value indicates we've encountered a cycle and Ice.AllowClassCycles is false.
     //
     IndexToPtrMap::iterator p = _unmarshaledMap.find(index);
     if(p != _unmarshaledMap.end())
     {
+        if (p->second == ICE_NULLPTR)
+        {
+            assert(!_stream->_instance->acceptClassCycles());
+            throw MarshalException(__FILE__, __LINE__, "cycle detected during Value unmarshaling");
+        }
         (*patchFunc)(patchAddr, p->second);
         return;
     }
@@ -1862,7 +1867,10 @@ Ice::InputStream::EncapsDecoder::unmarshal(Int index, const Ice::ValuePtr& v)
     // Add the object to the map of unmarshaled instances, this must
     // be done before reading the instances (for circular references).
     //
-    _unmarshaledMap.insert(make_pair(index, v));
+    // If circular references are not allowed we insert null (for cycle detection) and add
+    // the object to the map once it has been fully unmarshaled.
+    //
+    _unmarshaledMap.insert(make_pair(index, _stream->_instance->acceptClassCycles() ? v : Ice::ValuePtr()));
 
     //
     // Read the object.
@@ -1915,6 +1923,13 @@ Ice::InputStream::EncapsDecoder::unmarshal(Int index, const Ice::ValuePtr& v)
             _valueList.clear();
         }
     }
+
+    if(!_stream->_instance->acceptClassCycles())
+    {
+        // This class has been fully unmarshaled without creating any cycles
+        // It can be added to the map now.
+        _unmarshaledMap[index] = v;
+    }
 }
 
 void
diff --git a/cpp/src/Ice/Instance.cpp b/cpp/src/Ice/Instance.cpp
index e0c06b52a9b..7a1b5952157 100644
--- a/cpp/src/Ice/Instance.cpp
+++ b/cpp/src/Ice/Instance.cpp
@@ -950,6 +950,7 @@ IceInternal::Instance::Instance(const CommunicatorPtr& communicator, const Initi
     _classGraphDepthMax(0),
     _collectObjects(false),
     _toStringMode(ICE_ENUM(ToStringMode, Unicode)),
+    _acceptClassCycles(false),
     _implicitContext(0),
     _stringConverter(Ice::getProcessStringConverter()),
     _wstringConverter(Ice::getProcessWstringConverter()),
@@ -1227,6 +1228,8 @@ IceInternal::Instance::Instance(const CommunicatorPtr& communicator, const Initi
             throw InitializationException(__FILE__, __LINE__, "The value for Ice.ToStringMode must be Unicode, ASCII or Compat");
         }
 
+        const_cast<bool&>(_acceptClassCycles) = _initData.properties->getPropertyAsInt("Ice.AcceptClassCycles") > 0;
+
         const_cast<ImplicitContextIPtr&>(_implicitContext) =
             ImplicitContextI::create(_initData.properties->getProperty("Ice.ImplicitContext"));
 
diff --git a/cpp/src/Ice/Instance.h b/cpp/src/Ice/Instance.h
index 70e9c249e42..150167190d8 100644
--- a/cpp/src/Ice/Instance.h
+++ b/cpp/src/Ice/Instance.h
@@ -106,6 +106,7 @@ public:
     size_t classGraphDepthMax() const { return _classGraphDepthMax; }
     bool collectObjects() const { return _collectObjects; }
     Ice::ToStringMode toStringMode() const { return _toStringMode; }
+    bool acceptClassCycles() const { return _acceptClassCycles; }
     const ACMConfig& clientACM() const;
     const ACMConfig& serverACM() const;
 
@@ -175,6 +176,7 @@ private:
     const size_t _classGraphDepthMax; // Immutable, not reset by destroy().
     const bool _collectObjects; // Immutable, not reset by destroy().
     const Ice::ToStringMode _toStringMode; // Immutable, not reset by destroy()
+    const bool _acceptClassCycles; // Immutable, not reset by destroy()
     ACMConfig _clientACM;
     ACMConfig _serverACM;
     RouterManagerPtr _routerManager;
diff --git a/cpp/src/Ice/PropertyNames.cpp b/cpp/src/Ice/PropertyNames.cpp
index 9d332d8618a..da4d0af3534 100644
--- a/cpp/src/Ice/PropertyNames.cpp
+++ b/cpp/src/Ice/PropertyNames.cpp
@@ -1,7 +1,7 @@
 //
 // Copyright (c) ZeroC, Inc. All rights reserved.
 //
-// Generated by makeprops.py from file ./config/PropertyNames.xml, Fri Sep  6 18:11:04 2019
+// Generated by makeprops.py from file ./config/PropertyNames.xml, Thu Jul  2 14:55:02 2020
 
 // IMPORTANT: Do not edit this file -- any edits made here will be lost!
 
@@ -9,6 +9,7 @@
 
 const IceInternal::Property IcePropsData[] =
 {
+    IceInternal::Property("Ice.AcceptClassCycles", false, 0),
     IceInternal::Property("Ice.ACM.Client", true, 0),
     IceInternal::Property("Ice.ACM.Server", true, 0),
     IceInternal::Property("Ice.ACM.Timeout", false, 0),
diff --git a/cpp/src/Ice/PropertyNames.h b/cpp/src/Ice/PropertyNames.h
index 9c057af4afe..8f65258d805 100644
--- a/cpp/src/Ice/PropertyNames.h
+++ b/cpp/src/Ice/PropertyNames.h
@@ -1,7 +1,7 @@
 //
 // Copyright (c) ZeroC, Inc. All rights reserved.
 //
-// Generated by makeprops.py from file ./config/PropertyNames.xml, Fri Sep  6 18:11:04 2019
+// Generated by makeprops.py from file ./config/PropertyNames.xml, Thu Jul  2 14:55:02 2020
 
 // IMPORTANT: Do not edit this file -- any edits made here will be lost!