diff --git a/code/AssetLib/Ply/PlyParser.cpp b/code/AssetLib/Ply/PlyParser.cpp index dbbabc03f..ffdd62efb 100644 --- a/code/AssetLib/Ply/PlyParser.cpp +++ b/code/AssetLib/Ply/PlyParser.cpp @@ -48,10 +48,29 @@ OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. #include #include #include +#include #include namespace Assimp { +std::string to_string(EElementSemantic e) { + + switch (e) { + case EEST_Vertex: + return std::string{ "vertex" }; + case EEST_TriStrip: + return std::string{ "tristrips" }; + case EEST_Edge: + return std::string{ "edge" }; + case EEST_Material: + return std::string{ "material" }; + case EEST_TextureFile: + return std::string{ "TextureFile" }; + default: + return std::string{ "invalid" }; + } +} + // ------------------------------------------------------------------------------------------------ PLY::EDataType PLY::Property::ParseDataType(std::vector &buffer) { ai_assert(!buffer.empty()); @@ -281,6 +300,8 @@ bool PLY::Element::ParseElement(IOStreamBuffer &streamBuffer, std::vector< // if the exact semantic can't be determined, just store // the original string identifier pOut->szName = std::string(&buffer[0], &buffer[0] + strlen(&buffer[0])); + auto pos = pOut->szName.find_last_of(' '); + pOut->szName.erase(pos, pOut->szName.size()); } if (!PLY::DOM::SkipSpaces(buffer)) @@ -413,6 +434,7 @@ bool PLY::DOM::SkipComments(std::vector buffer) { bool PLY::DOM::ParseHeader(IOStreamBuffer &streamBuffer, std::vector &buffer, bool isBinary) { ASSIMP_LOG_VERBOSE_DEBUG("PLY::DOM::ParseHeader() begin"); + std::unordered_set definedAlElements; // parse all elements while (!buffer.empty()) { // skip all comments @@ -421,6 +443,13 @@ bool PLY::DOM::ParseHeader(IOStreamBuffer &streamBuffer, std::vector PLY::Element out; if (PLY::Element::ParseElement(streamBuffer, buffer, &out)) { // add the element to the list of elements + + const auto propertyName = (out.szName.empty()) ? to_string(out.eSemantic) : out.szName; + auto alreadyDefined = definedAlElements.find(propertyName); + if (alreadyDefined != definedAlElements.end()) { + throw DeadlyImportError("Property '" + propertyName + "' in header already defined "); + } + definedAlElements.insert(propertyName); alElements.push_back(out); } else if (TokenMatch(buffer, "end_header", 10)) { // we have reached the end of the header diff --git a/test/unit/utPLYImportExport.cpp b/test/unit/utPLYImportExport.cpp index 54d7e4328..ecbe837d4 100644 --- a/test/unit/utPLYImportExport.cpp +++ b/test/unit/utPLYImportExport.cpp @@ -89,7 +89,7 @@ TEST_F(utPLYImportExport, exportTest_Success) { #endif // ASSIMP_BUILD_NO_EXPORT -//Test issue 1623, crash when loading two PLY files in a row +// Test issue 1623, crash when loading two PLY files in a row TEST_F(utPLYImportExport, importerMultipleTest) { Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/PLY/cube.ply", aiProcess_ValidateDataStructure); @@ -109,7 +109,7 @@ TEST_F(utPLYImportExport, importPLYwithUV) { EXPECT_NE(nullptr, scene); EXPECT_NE(nullptr, scene->mMeshes[0]); - //This test model is using n-gons, so 6 faces instead of 12 tris + // This test model is using n-gons, so 6 faces instead of 12 tris EXPECT_EQ(6u, scene->mMeshes[0]->mNumFaces); EXPECT_EQ(aiPrimitiveType_POLYGON, scene->mMeshes[0]->mPrimitiveTypes); EXPECT_EQ(true, scene->mMeshes[0]->HasTextureCoords(0)); @@ -121,7 +121,7 @@ TEST_F(utPLYImportExport, importBinaryPLY) { EXPECT_NE(nullptr, scene); EXPECT_NE(nullptr, scene->mMeshes[0]); - //This test model is double sided, so 12 faces instead of 6 + // This test model is double sided, so 12 faces instead of 6 EXPECT_EQ(12u, scene->mMeshes[0]->mNumFaces); } @@ -160,7 +160,7 @@ TEST_F(utPLYImportExport, vertexColorTest) { TEST_F(utPLYImportExport, pointcloudTest) { Assimp::Importer importer; - //Could not use aiProcess_ValidateDataStructure since it's missing faces. + // Could not use aiProcess_ValidateDataStructure since it's missing faces. const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/PLY/issue623.ply", 0); EXPECT_NE(nullptr, scene); @@ -192,7 +192,7 @@ static const char *test_file = TEST_F(utPLYImportExport, parseErrorTest) { Assimp::Importer importer; - //Could not use aiProcess_ValidateDataStructure since it's missing faces. + // Could not use aiProcess_ValidateDataStructure since it's missing faces. const aiScene *scene = importer.ReadFileFromMemory(test_file, strlen(test_file), 0); EXPECT_NE(nullptr, scene); } @@ -207,5 +207,57 @@ TEST_F(utPLYImportExport, parseInvalid) { TEST_F(utPLYImportExport, payload_JVN42386607) { Assimp::Importer importer; const aiScene *scene = importer.ReadFile(ASSIMP_TEST_MODELS_DIR "/PLY/payload_JVN42386607", 0); - EXPECT_EQ(nullptr, scene); -} \ No newline at end of file + EXPECT_EQ(nullptr, scene); +} + +// Tests Issue #5729. Test, if properties defined multiple times. Unclear what to do, better to abort than to crash entirely +TEST_F(utPLYImportExport, parseInvalidDoubleProperty) { + const char data[] = "ply\n" + "format ascii 1.0\n" + "element vertex 4\n" + "property float x\n" + "property float y\n" + "property float z\n" + "element vertex 8\n" + "property float x\n" + "property float y\n" + "property float z\n" + "end_header\n" + "0.0 0.0 0.0 0.0 0.0 0.0\n" + "0.0 0.0 1.0 0.0 0.0 1.0\n" + "0.0 1.0 0.0 0.0 1.0 0.0\n" + "0.0 0.0 1.0\n" + "0.0 1.0 0.0 0.0 0.0 1.0\n" + "0.0 1.0 1.0 0.0 1.0 1.0\n"; + + Assimp::Importer importer; + const aiScene *scene = importer.ReadFileFromMemory(data, sizeof(data), 0); + EXPECT_EQ(nullptr, scene); +} + +// Tests Issue #5729. Test, if properties defined multiple times. Unclear what to do, better to abort than to crash entirely +TEST_F(utPLYImportExport, parseInvalidDoubleCustomProperty) { + const char data[] = "ply\n" + "format ascii 1.0\n" + "element vertex 4\n" + "property float x\n" + "property float y\n" + "property float z\n" + "element name 8\n" + "property float x\n" + "element name 5\n" + "property float x\n" + "end_header\n" + "0.0 0.0 0.0 100.0 10.0\n" + "0.0 0.0 1.0 200.0 20.0\n" + "0.0 1.0 0.0 300.0 30.0\n" + "0.0 1.0 1.0 400.0 40.0\n" + "0.0 0.0 0.0 500.0 50.0\n" + "0.0 0.0 1.0 600.0 60.0\n" + "0.0 1.0 0.0 700.0 70.0\n" + "0.0 1.0 1.0 800.0 80.0\n"; + + Assimp::Importer importer; + const aiScene *scene = importer.ReadFileFromMemory(data, sizeof(data), 0); + EXPECT_EQ(nullptr, scene); +}