XFileParser: release x-file-based scene when throwing an exception.

code/3DSLoader.cpp

@@ -113,22 +113,24 @@ Discreet3DSImporter::Discreet3DSImporter()
 , mScene()
 , mMasterScale()
 , bHasBG()
-, bIsPrj()
+, bIsPrj() {
-{}
+    // empty
+}
 
 // ------------------------------------------------------------------------------------------------
 // Destructor, private as well
-Discreet3DSImporter::~Discreet3DSImporter()
+Discreet3DSImporter::~Discreet3DSImporter() {
-{}
+    // empty
+}
 
 // ------------------------------------------------------------------------------------------------
 // Returns whether the class can handle the format of the given file.
-bool Discreet3DSImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const
+bool Discreet3DSImporter::CanRead( const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const {
-{
     std::string extension = GetExtension(pFile);
     if(extension == "3ds" || extension == "prj" ) {
         return true;
     }
+
     if (!extension.length() || checkSig) {
         uint16_t token[3];
         token[0] = 0x4d4d;
@@ -210,7 +212,7 @@ void Discreet3DSImporter::InternReadFile( const std::string& pFile,
     ConvertScene(pScene);
 
     // Generate the node graph for the scene. This is a little bit
-    // tricky since we'll need to split some meshes into submeshes
+    // tricky since we'll need to split some meshes into sub-meshes
     GenerateNodeGraph(pScene);
 
     // Now apply the master scaling factor to the scene

code/PlyLoader.cpp

@@ -106,24 +106,24 @@ PLYImporter::~PLYImporter() {
 
 // ------------------------------------------------------------------------------------------------
 // Returns whether the class can handle the format of the given file.
-bool PLYImporter::CanRead(const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const
+bool PLYImporter::CanRead(const std::string& pFile, IOSystem* pIOHandler, bool checkSig) const {
-{
     const std::string extension = GetExtension(pFile);
 
-  if (extension == "ply")
+    if ( extension == "ply" ) {
         return true;
-  else if (!extension.length() || checkSig)
+    } else if (!extension.length() || checkSig) {
-  {
+        if ( !pIOHandler ) {
-    if (!pIOHandler)return true;
+            return true;
-    const char* tokens[] = { "ply" };
+        }
+        static const char* tokens[] = { "ply" };
         return SearchFileHeaderForToken(pIOHandler, pFile, tokens, 1);
     }
+
     return false;
 }
 
 // ------------------------------------------------------------------------------------------------
-const aiImporterDesc* PLYImporter::GetInfo() const
+const aiImporterDesc* PLYImporter::GetInfo() const {
-{
     return &desc;
 }
 
@@ -149,9 +149,7 @@ static bool isBigEndian(const char* szMe) {
 
 // ------------------------------------------------------------------------------------------------
 // Imports the given file into the given scene structure.
-void PLYImporter::InternReadFile(const std::string& pFile,
+void PLYImporter::InternReadFile(const std::string& pFile, aiScene* pScene, IOSystem* pIOHandler) {
-  aiScene* pScene, IOSystem* pIOHandler)
-{
     static const std::string mode = "rb";
     std::unique_ptr<IOStream> fileStream(pIOHandler->Open(pFile, mode));
     if (!fileStream.get()) {
@@ -159,7 +157,7 @@ void PLYImporter::InternReadFile(const std::string& pFile,
     }
 
     // Get the file-size
-  size_t fileSize = fileStream->FileSize();
+    const size_t fileSize( fileStream->FileSize() );
     if ( 0 == fileSize ) {
         throw DeadlyImportError("File " + pFile + " is empty.");
     }
@@ -174,8 +172,7 @@ void PLYImporter::InternReadFile(const std::string& pFile,
     if ((headerCheck.size() < 3) ||
             (headerCheck[0] != 'P' && headerCheck[0] != 'p') ||
             (headerCheck[1] != 'L' && headerCheck[1] != 'l') ||
-      (headerCheck[2] != 'Y' && headerCheck[2] != 'y') )
+            (headerCheck[2] != 'Y' && headerCheck[2] != 'y') ) {
-  {
         streamedBuffer.close();
         throw DeadlyImportError("Invalid .ply file: Magic number \'ply\' is no there");
     }
@@ -194,10 +191,8 @@ void PLYImporter::InternReadFile(const std::string& pFile,
     if (TokenMatch(szMe, "format", 6)) {
         if (TokenMatch(szMe, "ascii", 5)) {
             SkipLine(szMe, (const char**)&szMe);
-      if (!PLY::DOM::ParseInstance(streamedBuffer, &sPlyDom, this))
+            if (!PLY::DOM::ParseInstance(streamedBuffer, &sPlyDom, this)) {
-      {
+                if (mGeneratedMesh != NULL) {
-        if (mGeneratedMesh != NULL)
-        {
                     delete(mGeneratedMesh);
                     mGeneratedMesh = nullptr;
                 }
@@ -205,17 +200,13 @@ void PLYImporter::InternReadFile(const std::string& pFile,
                 streamedBuffer.close();
                 throw DeadlyImportError("Invalid .ply file: Unable to build DOM (#1)");
             }
-    }
+        } else if (!::strncmp(szMe, "binary_", 7)) {
-    else if (!::strncmp(szMe, "binary_", 7))
-    {
             szMe += 7;
             const bool bIsBE(isBigEndian(szMe));
 
             // skip the line, parse the rest of the header and build the DOM
-      if (!PLY::DOM::ParseInstanceBinary(streamedBuffer, &sPlyDom, this, bIsBE))
+            if (!PLY::DOM::ParseInstanceBinary(streamedBuffer, &sPlyDom, this, bIsBE)) {
-      {
+                if (mGeneratedMesh != NULL) {
-        if (mGeneratedMesh != NULL)
-        {
                     delete(mGeneratedMesh);
                     mGeneratedMesh = nullptr;
                 }
@@ -223,11 +214,8 @@ void PLYImporter::InternReadFile(const std::string& pFile,
                 streamedBuffer.close();
                 throw DeadlyImportError("Invalid .ply file: Unable to build DOM (#2)");
             }
-    }
+        } else {
-    else
+            if (mGeneratedMesh != NULL) {
-    {
-      if (mGeneratedMesh != NULL)
-      {
                 delete(mGeneratedMesh);
                 mGeneratedMesh = nullptr;
             }
@@ -235,12 +223,9 @@ void PLYImporter::InternReadFile(const std::string& pFile,
             streamedBuffer.close();
             throw DeadlyImportError("Invalid .ply file: Unknown file format");
         }
-  }
+    } else {
-  else
-  {
         AI_DEBUG_INVALIDATE_PTR(this->mBuffer);
-    if (mGeneratedMesh != NULL)
+        if (mGeneratedMesh != NULL) {
-    {
             delete(mGeneratedMesh);
             mGeneratedMesh = nullptr;
         }
@@ -252,20 +237,16 @@ void PLYImporter::InternReadFile(const std::string& pFile,
     //free the file buffer
     streamedBuffer.close();
 
-  if (mGeneratedMesh == NULL)
+    if (mGeneratedMesh == NULL) {
-  {
         throw DeadlyImportError("Invalid .ply file: Unable to extract mesh data ");
     }
 
     // if no face list is existing we assume that the vertex
     // list is containing a list of points
     bool pointsOnly = mGeneratedMesh->mFaces == NULL ? true : false;
-  if (pointsOnly)
+    if (pointsOnly) {
-  {
+        if (mGeneratedMesh->mNumVertices < 3) {
-    if (mGeneratedMesh->mNumVertices < 3)
+            if (mGeneratedMesh != NULL) {
-    {
-      if (mGeneratedMesh != NULL)
-      {
                 delete(mGeneratedMesh);
                 mGeneratedMesh = nullptr;
             }
@@ -279,8 +260,7 @@ void PLYImporter::InternReadFile(const std::string& pFile,
         mGeneratedMesh->mNumFaces = iNum;
         mGeneratedMesh->mFaces = new aiFace[mGeneratedMesh->mNumFaces];
 
-    for (unsigned int i = 0; i < iNum; ++i)
+        for (unsigned int i = 0; i < iNum; ++i) {
-    {
             mGeneratedMesh->mFaces[i].mNumIndices = 3;
             mGeneratedMesh->mFaces[i].mIndices = new unsigned int[3];
             mGeneratedMesh->mFaces[i].mIndices[0] = (i * 3);
@@ -521,9 +501,7 @@ void PLYImporter::LoadVertex(const PLY::Element* pcElement, const PLY::ElementIn
 
 // ------------------------------------------------------------------------------------------------
 // Convert a color component to [0...1]
-ai_real PLYImporter::NormalizeColorValue(PLY::PropertyInstance::ValueUnion val,
+ai_real PLYImporter::NormalizeColorValue(PLY::PropertyInstance::ValueUnion val, PLY::EDataType eType) {
-  PLY::EDataType eType)
-{
   switch (eType)
   {
   case EDT_Float:

code/PlyLoader.h

@@ -57,7 +57,6 @@ struct aiMesh;
 
 namespace Assimp {
 
-
 using namespace PLY;
 
 // ---------------------------------------------------------------------------

code/PlyParser.h

@@ -39,12 +39,11 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 ----------------------------------------------------------------------
 */
 
-
 /** @file Defines the helper data structures for importing PLY files  */
+#pragma once
 #ifndef AI_PLYFILEHELPER_H_INC
 #define AI_PLYFILEHELPER_H_INC
 
-
 #include <assimp/ParsingUtils.h>
 #include <assimp/IOStreamBuffer.h>
 #include <vector>
@@ -58,8 +57,7 @@ class PLYImporter;
 // http://local.wasp.uwa.edu.au/~pbourke/dataformats/ply/
 // http://w3.impa.br/~lvelho/outgoing/sossai/old/ViHAP_D4.4.2_PLY_format_v1.1.pdf
 // http://www.okino.com/conv/exp_ply.htm
-namespace PLY
+namespace PLY {
-{
 
 // ---------------------------------------------------------------------------------
 /*
@@ -78,8 +76,7 @@ int8
 int16
 uint8 ... forms are also used
 */
-enum EDataType
+enum EDataType {
-{
     EDT_Char = 0x0u,
     EDT_UChar,
     EDT_Short,
@@ -98,8 +95,7 @@ enum EDataType
  *
  * Semantics define the usage of a property, e.g. x coordinate
 */
-enum ESemantic
+enum ESemantic {
-{
     //! vertex position x coordinate
     EST_XCoord = 0x0u,
     //! vertex position x coordinate
@@ -182,15 +178,14 @@ enum ESemantic
  *
  * Semantics define the usage of an element, e.g. vertex or material
 */
-enum EElementSemantic
+enum EElementSemantic {
-{
     //! The element is a vertex
     EEST_Vertex = 0x0u,
 
     //! The element is a face description (index table)
     EEST_Face,
 
-    //! The element is a tristrip description (index table)
+    //! The element is a triangle-strip description (index table)
     EEST_TriStrip,
 
     //! The element is an edge description (ignored)
@@ -211,17 +206,16 @@ enum EElementSemantic
  *
  * This can e.g. be a part of the vertex declaration
  */
-class Property
+class Property {
-{
 public:
-
     //! Default constructor
     Property()
-        : eType (EDT_Int),
+    : eType (EDT_Int)
-        Semantic(),
+    , Semantic()
-        bIsList(false),
+    , bIsList(false)
-        eFirstType(EDT_UChar)
+    , eFirstType(EDT_UChar) {
-    {}
+        // empty
+    }
 
     //! Data type of the property
     EDataType eType;
@@ -260,15 +254,14 @@ public:
  * This can e.g. be the vertex declaration. Elements contain a
  * well-defined number of properties.
  */
-class Element
+class Element {
-{
 public:
-
     //! Default constructor
     Element()
     : eSemantic (EEST_INVALID)
-        ,   NumOccur(0)
+    , NumOccur(0) {
-    {}
+        // empty
+    }
 
     //! List of properties assigned to the element
     //! std::vector to support operator[]

code/XFileParser.cpp

@@ -1047,10 +1047,12 @@ void XFileParser::readHeadOfDataObject( std::string* poName)
         if( poName)
             *poName = nameOrBrace;
 
-        if( GetNextToken() != "{")
+        if ( GetNextToken() != "{" ) {
+            delete mScene;
             ThrowException( "Opening brace expected." );
         }
     }
+}
 
 // ------------------------------------------------------------------------------------------------
 std::string XFileParser::GetNextToken() {
@@ -1224,21 +1226,29 @@ void XFileParser::GetNextTokenAsString( std::string& poString)
     }
 
     FindNextNoneWhiteSpace();
-    if( mP >= mEnd)
+    if ( mP >= mEnd ) {
+        delete mScene;
         ThrowException( "Unexpected end of file while parsing string" );
+    }
 
-    if( *mP != '"')
+    if ( *mP != '"' ) {
+        delete mScene;
         ThrowException( "Expected quotation mark." );
+    }
     ++mP;
 
     while( mP < mEnd && *mP != '"')
         poString.append( mP++, 1);
 
-    if( mP >= mEnd-1)
+    if ( mP >= mEnd - 1 ) {
+        delete mScene;
         ThrowException( "Unexpected end of file while parsing string" );
+    }
 
-    if( mP[1] != ';' || mP[0] != '"')
+    if ( mP[ 1 ] != ';' || mP[ 0 ] != '"' ) {
+        delete mScene;
         ThrowException( "Expected quotation mark and semicolon at the end of a string." );
+    }
     mP+=2;
 }
 
@@ -1449,14 +1459,14 @@ aiColor3D XFileParser::ReadRGB()
 
 // ------------------------------------------------------------------------------------------------
 // Throws an exception with a line number and the given text.
-AI_WONT_RETURN void XFileParser::ThrowException( const std::string& pText)
+AI_WONT_RETURN void XFileParser::ThrowException( const std::string& pText) {
-{
+    delete mScene;
-    if( mIsBinaryFormat)
+    if ( mIsBinaryFormat ) {
         throw DeadlyImportError( pText );
-    else
+    } else {
         throw DeadlyImportError( format() << "Line " << mLineNumber << ": " << pText );
     }
-
+}
 
 // ------------------------------------------------------------------------------------------------
 // Filters the imported hierarchy for some degenerated cases that some exporters produce.

test/unit/utPLYImportExport.cpp

@@ -103,3 +103,28 @@ TEST_F( utPLYImportExport, vertexColorTest ) {
     const aiScene *scene = importer.ReadFile( ASSIMP_TEST_MODELS_DIR "/PLY/float-color.ply", 0 );
     EXPECT_NE( nullptr, scene );
 }
+
+static const char *test_file =
+    "ply\n"
+    "format ascii 1.0\n"
+    "element vertex 4\n"
+    "property float x\n"
+    "property float y\n"
+    "property float z\n"
+    "property uchar red\n"
+    "property uchar green\n"
+    "property uchar blue\n"
+    "property float nx\n"
+    "property float ny\n"
+    "property float nz\n"
+    "end_header\n"
+    "0.0 0.0 0.0 255 255 255 0.0 1.0 0.0\n"
+    "0.0 0.0 1.0 255 0 255 0.0 0.0 1.0\n"
+    "0.0 1.0 0.0 255 255 0 1.0 0.0 0.0\n"
+    "0.0 1.0 1.0 0 255 255 1.0 1.0 0.0\n";
+
+TEST_F( utPLYImportExport, parseErrorTest ) {
+    Assimp::Importer importer;
+    const aiScene *scene = importer.ReadFileFromMemory( test_file, strlen( test_file ), 0 );
+    EXPECT_NE( nullptr, scene );
+}