Changeset 25320

Timestamp:

Apr 26, 2021, 4:57:13 PM (3 years ago)

Author:

wraitii

Message:

More actor file validation, preventing crashes.

It's currently possible to trigger ENSUREs in the code if loading empty groups, which is easy enough with LOD and shouldn't happen.

Recover gracefully from these errors instead.

Differential Revision: ​https://code.wildfiregames.com/D3888

Location:

ps/trunk/source/graphics

Files:

• ObjectBase.cpp (modified) (11 diffs)
• ObjectBase.h (modified) (1 diff)

ps/trunk/source/graphics/ObjectBase.cpp

@@ -78 +78 @@
 }
 
-void CObjectBase::Load(const CXeromyces& XeroFile, const XMBElement& root)
+ CObjectBase::Load(const CXeromyces& XeroFile, const XMBElement& root)
 {
     // Define all the elements used in the XML file
@@ -134 +134 @@
                     continue;
 
-                LoadVariant(XeroFile, variant, currentGroup.emplace_back());
+                if (!LoadVariant(XeroFile, variant, currentGroup.emplace_back()))
+                    return false;
             }
 
             if (currentGroup.size() == 0)
+
                 LOGERROR("Actor group has zero variants ('%s')", m_Identifier);
+
+
         }
         else if (child_name == el_castshadow)
@@ -150 +154 @@
     if (m_Material.empty())
         m_Material = VfsPath("art/materials/default.xml");
-}
-
-void CObjectBase::LoadVariant(const CXeromyces& XeroFile, const XMBElement& variant, Variant& currentVariant)
+
+    return true;
+}
+
+bool CObjectBase::LoadVariant(const CXeromyces& XeroFile, const XMBElement& variant, Variant& currentVariant)
 {
     #define EL(x) int el_##x = XeroFile.GetElementID(#x)
@@ -191 +197 @@
     {
         LOGERROR("Invalid variant format (unrecognised root element '%s')", XeroFile.GetElementString(variant.GetNodeName()).c_str());
-        return;
+        return;
     }
 
@@ -206 +212 @@
             {
                 XMBElement variantRoot = XeroVariant.GetRoot();
-                LoadVariant(XeroVariant, variantRoot, currentVariant);
+                if (!LoadVariant(XeroVariant, variantRoot, currentVariant))
+                    return false;
             }
             else
+
                 LOGERROR("Could not open path %s", attr.Value);
+
+
             // Continue loading extra definitions in this variant to allow nested files
         }
@@ -234 +244 @@
             XERO_ITER_EL(option, textures_element)
             {
-                ENSURE(textures_element.GetNodeName() == el_texture);
+                if (textures_element.GetNodeName() != el_texture)
+                {
+                    LOGERROR("<textures> can only contain <texture> elements.");
+                    return false;
+                }
 
                 Samp samp;
@@ -276 +290 @@
             XERO_ITER_EL(option, anim_element)
             {
-                ENSURE(anim_element.GetNodeName() == el_animation);
+                if (anim_element.GetNodeName() != el_animation)
+                {
+                    LOGERROR("<animations> can only contain <animations> elements.");
+                    return false;
+                }
 
                 Anim anim;
@@ -325 +343 @@
         }
     }
+
 }
 
@@ -812 +831 @@
     {
         std::unique_ptr<CObjectBase> base = std::make_unique<CObjectBase>(m_ObjectManager, *this, MAX_QUALITY);
-        base->Load(XeroFile, root);
+        if (!base->Load(XeroFile, root))
+        {
+            LOGERROR("Invalid actor (actor '%s')", pathname.string8());
+            return false;
+        }
         m_ObjectBases.emplace_back(std::move(base));
     }
@@ -877 +900 @@
                     return false;
                 }
-                base->Load(XeroFile, inlineActor);
+                if (!base->Load(XeroFile, inlineActor))
+                {
+                    LOGERROR("Invalid inline actor (actor '%s')", pathname.string8());
+                    return false;
+                }
+
             }
             else if (file.empty())
-                base->Load(XeroFile, actor);
+            {
+                if (!base->Load(XeroFile, actor))
+                {
+                    LOGERROR("Invalid actor (actor '%s')", pathname.string8());
+                    return false;
+                }
+            }
             else
             {
@@ -897 +931 @@
                         return false;
                     }
-                    base->Load(XeroActor, root);
+                    if (!base->Load(XeroActor, root))
+                    {
+                        LOGERROR("Invalid actor (actor '%s' loaded from '%s')", file, pathname.string8());
+                        return false;
+                    }
                 }
                 else

ps/trunk/source/graphics/ObjectBase.h

@@ -196 +196 @@
     void GetQualitySplits(std::vector<u8>& splits) const;
 
-    void Load(const CXeromyces& XeroFile, const XMBElement& base);
-    void LoadVariant(const CXeromyces& XeroFile, const XMBElement& variant, Variant& currentVariant);
+     Load(const CXeromyces& XeroFile, const XMBElement& base);
+     LoadVariant(const CXeromyces& XeroFile, const XMBElement& variant, Variant& currentVariant);
 
 private: