From aef4ecada533c40d41ae4e885eaa39bfe74601cb Mon Sep 17 00:00:00 2001
From: Jeremy Cytryn <jcytryn@fb.com>
Date: Wed, 6 May 2020 23:41:05 -0700
Subject: [PATCH] Fail gltf/gltf2 export whenever invalid / incomplete JSON is
 generated

This can happen currently for example if NaNs are introduced in accessor bounds as rapidjson cannot write NaN/inf floats (see subsequent commit for fix there) and will halt writing to buffer at this point.  Fix here ensures that whenever anything like this happens we throw an exception so this ends up as a registered export failure case, rather than silently exporting the incomplete JSON
---
 code/AssetLib/glTF/glTFAssetWriter.inl           |  12 ++++++++----
 code/AssetLib/glTF2/glTF2AssetWriter.inl         |  10 ++++++----
 .../BoxWithInfinites.glb                         | Bin 0 -> 1900 bytes
 test/unit/utglTF2ImportExport.cpp                |  10 ++++++++++
 4 files changed, 24 insertions(+), 8 deletions(-)
 create mode 100644 test/models/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites.glb

diff --git a/code/AssetLib/glTF/glTFAssetWriter.inl b/code/AssetLib/glTF/glTFAssetWriter.inl
index 5e4416ee9..d8d2556fa 100644
--- a/code/AssetLib/glTF/glTFAssetWriter.inl
+++ b/code/AssetLib/glTF/glTFAssetWriter.inl
@@ -59,7 +59,7 @@ namespace glTF {
     namespace {
 
         template<typename T, size_t N>
-        inline 
+        inline
         Value& MakeValue(Value& val, T(&r)[N], MemoryPoolAllocator<>& al) {
             val.SetArray();
             val.Reserve(N, al);
@@ -70,7 +70,7 @@ namespace glTF {
         }
 
         template<typename T>
-        inline 
+        inline
         Value& MakeValue(Value& val, const std::vector<T> & r, MemoryPoolAllocator<>& al) {
             val.SetArray();
             val.Reserve(static_cast<rapidjson::SizeType>(r.size()), al);
@@ -530,7 +530,9 @@ namespace glTF {
         StringBuffer docBuffer;
 
         PrettyWriter<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         if (jsonOutFile->Write(docBuffer.GetString(), docBuffer.GetSize(), 1) != 1) {
             throw DeadlyExportError("Failed to write scene data!");
@@ -569,7 +571,9 @@ namespace glTF {
 
         StringBuffer docBuffer;
         Writer<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         if (outfile->Write(docBuffer.GetString(), docBuffer.GetSize(), 1) != 1) {
             throw DeadlyExportError("Failed to write scene data!");
diff --git a/code/AssetLib/glTF2/glTF2AssetWriter.inl b/code/AssetLib/glTF2/glTF2AssetWriter.inl
index 798f38c1c..361af40cd 100644
--- a/code/AssetLib/glTF2/glTF2AssetWriter.inl
+++ b/code/AssetLib/glTF2/glTF2AssetWriter.inl
@@ -613,7 +613,9 @@ namespace glTF2 {
         StringBuffer docBuffer;
 
         PrettyWriter<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         if (jsonOutFile->Write(docBuffer.GetString(), docBuffer.GetSize(), 1) != 1) {
             throw DeadlyExportError("Failed to write scene data!");
@@ -664,7 +666,9 @@ namespace glTF2 {
 
         StringBuffer docBuffer;
         Writer<StringBuffer> writer(docBuffer);
-        mDoc.Accept(writer);
+        if (!mDoc.Accept(writer)) {
+            throw DeadlyExportError("Failed to write scene data!");
+        }
 
         uint32_t jsonChunkLength = (docBuffer.GetSize() + 3) & ~3; // Round up to next multiple of 4
         auto paddingLength = jsonChunkLength - docBuffer.GetSize();
@@ -816,5 +820,3 @@ namespace glTF2 {
     }
 
 }
-
-
diff --git a/test/models/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites.glb b/test/models/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites.glb
new file mode 100644
index 0000000000000000000000000000000000000000..ae83f1f06578eb6b6dc74988dc1c52b9dd6477ab
GIT binary patch
literal 1900
zcmb7ES#R1v5T^HiU$1DNTFFJWfdqI#BuXSIjug`N0Y#ND3#{Y=txcLJLgb(2FZ3U!
z-z;lllC-3l@y?8Azi(#t7<v1<w=<cH_aKuwy_?B=>9$)Z%!wi%v&soO<T4Urz$(nn
znap4#9&qWzp@gSB;X$@Wk=eRG31ysZh`w~>arPtT3xAS5EauEy-YFF}HjC~jHZz#x
zx;%<P8L>+5g!QMxA(!6;|HUe%!TQHBZx4rx8m1cpHQ+(Ke>~x=Qnc(1a!vzeW)}<=
zD>wVTvz<p~zH<aj%fNF0OCaaoXUkCkgPE~PX5E0SxFpMiSCe(OYmHiGNoVJmT;>%L
z`vxR`uV@D3MSm8(3LGCzU|ZqQ4-t>+!B7Mu<`Gh%wl#P#ipQ+7X`2@lsj=xsU)SXH
zWV=*Cp^hBU+UE=94P8MumSO;M%f9I26)ZJ5jb;^SjZ$d&f6OlAHCIx|4RWE@J298S
z@m5;Y;RAGK(w9vhJDw-pPB<Nn1MEB6(S0Z4+o2cAUB^}XxM$`{rutb{**42|*)Z{_
zU8Dh$P;5$>U?o0lW-#sxZ1_W-_<96jB0tBQjGbmk&13Do+ShM8$ZQZ!`yTJ2r2|$O
zB6&_r=JRNb<)mYwm%0fL6(WT%VxTWR`U(xLO=?#wA%cOx$-)|eG{$=Mr?fdx?_RrG
z-><h@I5P%owL8sP12bl`10oFow!*0#Kk1|cUW&z$Qq`!Y;54b;sQ1E6eR9Xb8%Rz^
z&QixfSFk{{2#sn1ErKH#cZep{^f)`o2MYR)3u&v~!mX72POFKATWXDddX|b^qp!mv
zq;$+>JN@lC{eN<~$Qqn$@>Y4Mm|IZk{=TC}vT==P?m_NDaILG6?1zv?kjIcGdL;WP
z<Qe2S<b@u|ehGO6c@24^N3!2S-a+0&KIoBb<b948<yD0~PwC``Z|P6#XWCCOi9Xjl
z>?J1Y<ge;@(&1adguUb?{rpmVRmUfOika&e(#eleVUk`=>EwqjF%$hd|9lav;z_TP
F{RglTzqJ4W

literal 0
HcmV?d00001

diff --git a/test/unit/utglTF2ImportExport.cpp b/test/unit/utglTF2ImportExport.cpp
index 55cd2ef6a..99481101e 100644
--- a/test/unit/utglTF2ImportExport.cpp
+++ b/test/unit/utglTF2ImportExport.cpp
@@ -436,6 +436,16 @@ TEST_F(utglTF2ImportExport, error_string_preserved) {
     ASSERT_NE(error.find("BoxTextured0.bin"), std::string::npos) << "Error string should contain an error about missing .bin file";
 }
 
+TEST_F(utglTF2ImportExport, export_bad_accessor_bounds) {
+    Assimp::Importer importer;
+    Assimp::Exporter exporter;
+    const aiScene* scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites.glb", aiProcess_ValidateDataStructure);
+    ASSERT_NE(scene, nullptr);
+
+    EXPECT_EQ(aiReturn_FAILURE, exporter.Export(scene, "glb2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites_out.glb"));
+    EXPECT_EQ(aiReturn_FAILURE, exporter.Export(scene, "gltf2", ASSIMP_TEST_MODELS_DIR "/glTF2/BoxWithInfinites-glTF-Binary/BoxWithInfinites_out.gltf"));
+}
+
 #endif // ASSIMP_BUILD_NO_EXPORT
 
 TEST_F(utglTF2ImportExport, sceneMetadata) {