Merge pull request #3070 from ms-maxvollmer/GLTF2_recursive_references_fix

GLTF2: Detect and abort recursive references

code/glTF2/glTF2Asset.h

@@ -58,6 +58,7 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #include <algorithm>
 #include <list>
 #include <map>
+#include <set>
 #include <stdexcept>
 #include <string>
 #include <vector>
@@ -81,14 +82,18 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 #define ASSIMP_GLTF_USE_UNORDERED_MULTIMAP
 #else
 #define gltf_unordered_map map
+#define gltf_unordered_set set
 #endif
 
 #ifdef ASSIMP_GLTF_USE_UNORDERED_MULTIMAP
 #include <unordered_map>
+#include <unordered_set>
 #if _MSC_VER > 1600
 #define gltf_unordered_map unordered_map
+#define gltf_unordered_set unordered_set
 #else
 #define gltf_unordered_map tr1::unordered_map
+#define gltf_unordered_set tr1::unordered_set
 #endif
 #endif
 
@@ -874,6 +879,8 @@ class LazyDict : public LazyDictBase {
     Value *mDict; //! JSON dictionary object
     Asset &mAsset; //! The asset instance
 
+    std::gltf_unordered_set<unsigned int> mRecursiveReferenceCheck;  //! Used by Retrieve to prevent recursive lookups
+
     void AttachToDocument(Document &doc);
     void DetachFromDocument();
 

code/glTF2/glTF2Asset.inl

@@ -280,6 +280,11 @@ Ref<T> LazyDict<T>::Retrieve(unsigned int i) {
         throw DeadlyImportError("GLTF: Object at index \"" + to_string(i) + "\" is not a JSON object");
     }
 
+    if (mRecursiveReferenceCheck.find(i) != mRecursiveReferenceCheck.end()) {
+        throw DeadlyImportError("GLTF: Object at index \"" + to_string(i) + "\" has recursive reference to itself");
+    }
+    mRecursiveReferenceCheck.insert(i);
+
     // Unique ptr prevents memory leak in case of Read throws an exception
     auto inst = std::unique_ptr<T>(new T());
     inst->id = std::string(mDictId) + "_" + to_string(i);
@@ -287,7 +292,9 @@ Ref<T> LazyDict<T>::Retrieve(unsigned int i) {
     ReadMember(obj, "name", inst->name);
     inst->Read(obj, mAsset);
 
-    return Add(inst.release());
+    Ref<T> result = Add(inst.release());
+    mRecursiveReferenceCheck.erase(i);
+    return result;
 }
 
 template <class T>

test/models/glTF2/RecursiveNodes/RecursiveNodes.gltf

@@ -0,0 +1,25 @@
+{
+    "asset": {
+        "version": "2.0"
+    },
+    "scene": 0,
+    "scenes": [
+        {
+            "nodes": [
+                0
+            ]
+        }
+    ],
+    "nodes": [
+        {
+            "children": [
+                1
+            ]
+        },
+        {
+            "children": [
+                0
+            ]
+        }
+    ]
+}

test/unit/utglTF2ImportExport.cpp

@@ -491,6 +491,12 @@ TEST_F(utglTF2ImportExport, texcoords) {
     EXPECT_EQ(uvIndex, 1);
 }
 
+TEST_F(utglTF2ImportExport, recursive_nodes) {
+    Assimp::Importer importer;
+    const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/RecursiveNodes/RecursiveNodes.gltf", aiProcess_ValidateDataStructure);
+    EXPECT_EQ(nullptr, scene);
+}
+
 TEST_F(utglTF2ImportExport, norootnode_noscene) {
     Assimp::Importer importer;
     const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/TestNoRootNode/NoScene.gltf", aiProcess_ValidateDataStructure);