Improve error message for serializing objects with recursive references

diff --git a/lib/base/serializer.cpp b/lib/base/serializer.cpp
index 7296ec3ce9da90b65eeb754e625d382462a5e4f8..1bd8b0c8c6bc8006d86c78f30a482d46f0e3b742 100644 (file)
--- a/lib/base/serializer.cpp
+++ b/lib/base/serializer.cpp
@@ -21,10 +21,68 @@
 #include "base/type.hpp"
 #include "base/application.hpp"
 #include "base/objectlock.hpp"
+#include "base/convert.hpp"
+#include "base/exception.hpp"
+#include <boost/algorithm/string/join.hpp>
+#include <deque>
 
 using namespace icinga;
 
-static Array::Ptr SerializeArray(const Array::Ptr& input, int attributeTypes)
+struct SerializeStackEntry
+{
+       String Name;
+       Value Val;
+};
+
+CircularReferenceError::CircularReferenceError(String message, std::vector<String> path)
+       : m_Message(message), m_Path(path)
+{ }
+
+const char *CircularReferenceError::what(void) const throw()
+{
+       return m_Message.CStr();
+}
+
+std::vector<String> CircularReferenceError::GetPath() const
+{
+       return m_Path;
+}
+
+struct SerializeStack
+{
+       std::deque<SerializeStackEntry> Entries;
+
+       inline void Push(const String& name, const Value& val)
+       {
+               Object::Ptr obj;
+
+               if (val.IsObject())
+                       obj = val;
+
+               if (obj) {
+                       for (const auto& entry : Entries) {
+                               if (entry.Val == obj) {
+                                       std::vector<String> path;
+                                       for (const auto& entry : Entries)
+                                               path.push_back(entry.Name);
+                                       path.push_back(name);
+                                       BOOST_THROW_EXCEPTION(CircularReferenceError("Cannot serialize object which recursively refers to itself. Attribute path which leads to the cycle: " + boost::algorithm::join(path, " -> "), path));
+                               }
+                       }
+               }
+
+               Entries.push_back({ name, obj });
+       }
+
+       inline void Pop()
+       {
+               Entries.pop_back();
+       }
+};
+
+static Value SerializeInternal(const Value& value, int attributeTypes, SerializeStack& stack);
+
+static Array::Ptr SerializeArray(const Array::Ptr& input, int attributeTypes, SerializeStack& stack)
 {
        ArrayData result;
 
@@ -32,14 +90,19 @@ static Array::Ptr SerializeArray(const Array::Ptr& input, int attributeTypes)
 
        ObjectLock olock(input);
 
+       int index = 0;
+
        for (const Value& value : input) {
-               result.emplace_back(Serialize(value, attributeTypes));
+               stack.Push(Convert::ToString(index), value);
+               result.emplace_back(SerializeInternal(value, attributeTypes, stack));
+               stack.Pop();
+               index++;
        }
 
        return new Array(std::move(result));
 }
 
-static Dictionary::Ptr SerializeDictionary(const Dictionary::Ptr& input, int attributeTypes)
+static Dictionary::Ptr SerializeDictionary(const Dictionary::Ptr& input, int attributeTypes, SerializeStack& stack)
 {
        DictionaryData result;
 
@@ -48,13 +111,15 @@ static Dictionary::Ptr SerializeDictionary(const Dictionary::Ptr& input, int att
        ObjectLock olock(input);
 
        for (const Dictionary::Pair& kv : input) {
-               result.emplace_back(kv.first, Serialize(kv.second, attributeTypes));
+               stack.Push(kv.first, kv.second);
+               result.emplace_back(kv.first, SerializeInternal(kv.second, attributeTypes, stack));
+               stack.Pop();
        }
 
        return new Dictionary(std::move(result));
 }
 
-static Object::Ptr SerializeObject(const Object::Ptr& input, int attributeTypes)
+static Object::Ptr SerializeObject(const Object::Ptr& input, int attributeTypes, SerializeStack& stack)
 {
        Type::Ptr type = input->GetReflectionType();
 
@@ -73,7 +138,10 @@ static Object::Ptr SerializeObject(const Object::Ptr& input, int attributeTypes)
                if (strcmp(field.Name, "type") == 0)
                        continue;
 
-               fields.emplace_back(field.Name, Serialize(input->GetField(i), attributeTypes));
+               Value value = input->GetField(i);
+               stack.Push(field.Name, value);
+               fields.emplace_back(field.Name, SerializeInternal(input->GetField(i), attributeTypes, stack));
+               stack.Pop();
        }
 
        fields.emplace_back("type", type->GetName());
@@ -158,7 +226,7 @@ static Object::Ptr DeserializeObject(const Object::Ptr& object, const Dictionary
        return instance;
 }
 
-Value icinga::Serialize(const Value& value, int attributeTypes)
+static Value SerializeInternal(const Value& value, int attributeTypes, SerializeStack& stack)
 {
        if (!value.IsObject())
                return value;
@@ -168,14 +236,20 @@ Value icinga::Serialize(const Value& value, int attributeTypes)
        Array::Ptr array = dynamic_pointer_cast<Array>(input);
 
        if (array)
-               return SerializeArray(array, attributeTypes);
+               return SerializeArray(array, attributeTypes, stack);
 
        Dictionary::Ptr dict = dynamic_pointer_cast<Dictionary>(input);
 
        if (dict)
-               return SerializeDictionary(dict, attributeTypes);
+               return SerializeDictionary(dict, attributeTypes, stack);
+
+       return SerializeObject(input, attributeTypes, stack);
+}
 
-       return SerializeObject(input, attributeTypes);
+Value icinga::Serialize(const Value& value, int attributeTypes)
+{
+       SerializeStack stack;
+       return SerializeInternal(value, attributeTypes, stack);
 }
 
 Value icinga::Deserialize(const Value& value, bool safe_mode, int attributeTypes)

diff --git a/lib/base/serializer.hpp b/lib/base/serializer.hpp
index b5e8cd31a4cbc3683818ca10225195c355922500..85e7d454d4336d54191db1c5f2b4f742fe97cee8 100644 (file)
--- a/lib/base/serializer.hpp
+++ b/lib/base/serializer.hpp
@@ -23,10 +23,25 @@
 #include "base/i2-base.hpp"
 #include "base/type.hpp"
 #include "base/value.hpp"
+#include "base/exception.hpp"
 
 namespace icinga
 {
 
+class CircularReferenceError : virtual public user_error
+{
+public:
+       CircularReferenceError(String message, std::vector<String> path);
+       ~CircularReferenceError() throw() = default;
+
+       const char *what(void) const throw() final;
+       std::vector<String> GetPath() const;
+
+private:
+       String m_Message;
+       std::vector<String> m_Path;
+};
+
 Value Serialize(const Value& value, int attributeTypes = FAState);
 Value Deserialize(const Value& value, bool safe_mode = false, int attributeTypes = FAState);
 Value Deserialize(const Object::Ptr& object, const Value& value, bool safe_mode = false, int attributeTypes = FAState);

diff --git a/lib/config/configitem.cpp b/lib/config/configitem.cpp
index 4e4eacc2929bbc249e677a7cf42fd12fe69ab921..e2061286059c73722ccb38bff387e7cb4a405b17 100644 (file)
--- a/lib/config/configitem.cpp
+++ b/lib/config/configitem.cpp
@@ -36,6 +36,7 @@
 #include "base/json.hpp"
 #include "base/exception.hpp"
 #include "base/function.hpp"
+#include <boost/algorithm/string/join.hpp>
 #include <sstream>
 #include <fstream>
 
@@ -289,6 +290,14 @@ ConfigObject::Ptr ConfigItem::Commit(bool discard)
                throw;
        }
 
+       Value serializedObject;
+
+       try {
+               serializedObject = Serialize(dobj, FAConfig);
+       } catch (const CircularReferenceError& ex) {
+               BOOST_THROW_EXCEPTION(ValidationError(dobj, ex.GetPath(), "Circular references are not allowed"));
+       }
+
        Dictionary::Ptr persistentItem = new Dictionary({
                { "type", type->GetName() },
                { "name", GetName() },