Check indices before accessing two-dimensional vector

OPM · Dec 12, 2024 · a4b23ab · a4b23ab
1 parent 9254037
commit a4b23ab
Show file tree

Hide file tree

Showing 3 changed files with 89 additions and 9 deletions.
diff --git a/ApplicationLibCode/FileInterface/RifSurfaceImporter.cpp b/ApplicationLibCode/FileInterface/RifSurfaceImporter.cpp
@@ -600,17 +600,34 @@ bool RifSurfaceImporter::generateTriangleIndices( const std::vector<std::vector<
                                                   std::vector<unsigned>&                    triangleIndices,
                                                   unsigned                                  resolution /*= 1 */ )
 {
+    if ( i >= indexToPointData.size() || j >= indexToPointData[i].size() )
+    {
+        return false;
+    }
+
     unsigned topI = unsigned( std::min( indexToPointData.size() - 1, i + resolution ) );
+    if ( topI >= indexToPointData.size() )
+    {
+        return false;
+    }
+
     unsigned topJ = unsigned( std::min( indexToPointData[topI].size() - 1, j + resolution ) );
+    if ( topJ >= indexToPointData[topI].size() || topJ >= indexToPointData[i].size() )
+    {
+        return false;
+    }
 
+    // Prevent degenerate cases
     if ( topI == i || topJ == j ) return false;
 
-    unsigned q1 = indexToPointData[i + 0][j + 0];
-    unsigned q2 = indexToPointData[i + 0][topJ];
-    unsigned q3 = indexToPointData[topI][j + 0];
+    // Retrieve the indices
+    unsigned q1 = indexToPointData[i][j];
+    unsigned q2 = indexToPointData[i][topJ];
+    unsigned q3 = indexToPointData[topI][j];
     unsigned q4 = indexToPointData[topI][topJ];
 
-    if ( q1 != ( (unsigned)-1 ) && q2 != ( (unsigned)-1 ) && q3 != ( (unsigned)-1 ) && q4 != ( (unsigned)-1 ) )
+    // Ensure all points are valid
+    if ( q1 != (unsigned)-1 && q2 != (unsigned)-1 && q3 != (unsigned)-1 && q4 != (unsigned)-1 )
     {
         triangleIndices.push_back( q1 );
         triangleIndices.push_back( q2 );
@@ -620,10 +637,8 @@ bool RifSurfaceImporter::generateTriangleIndices( const std::vector<std::vector<
         triangleIndices.push_back( q3 );
         return true;
     }
-    else
-    {
-        return false;
-    }
+
+    return false; // At least one point is invalid
 }
 
 //--------------------------------------------------------------------------------------------------

diff --git a/ApplicationLibCode/FileInterface/RifSurfaceImporter.h b/ApplicationLibCode/FileInterface/RifSurfaceImporter.h
@@ -36,12 +36,13 @@ class RifSurfaceImporter
     static std::pair<std::vector<cvf::Vec3d>, std::vector<unsigned>> readOpenWorksXyzFile( const QString& filename,
                                                                                            double         preferredPointDistance );
 
-private:
     static bool generateTriangleIndices( const std::vector<std::vector<unsigned>>& indexToPointData,
                                          const size_t&                             i,
                                          const size_t&                             j,
                                          std::vector<unsigned>&                    triangleIndices,
                                          unsigned                                  resolution = 1 );
+
+private:
     static bool
         vectorFuzzyCompare( const cvf::Vec2d& vector1, const cvf::Vec2d& vector2, double epsilon = std::numeric_limits<double>::epsilon() );
 };
diff --git a/ApplicationLibCode/UnitTests/RifSurfaceImporter-Test.cpp b/ApplicationLibCode/UnitTests/RifSurfaceImporter-Test.cpp
@@ -260,3 +260,67 @@ TEST( RifSurfaceImporter, ReadLargeOpenWorksXyzFile )
         }
     }
 }
+
+// Test fixture for RifSurfaceImporter
+class RifSurfaceImporterTest : public ::testing::Test
+{
+protected:
+    RifSurfaceImporter                 rifSurfaceImporter;
+    std::vector<std::vector<unsigned>> indexToPointData;
+    std::vector<unsigned>              triangleIndices;
+};
+
+TEST_F( RifSurfaceImporterTest, HandlesEmptyInput )
+{
+    // Empty indexToPointData
+    indexToPointData = {};
+    EXPECT_FALSE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 0, 0, triangleIndices ) );
+    EXPECT_TRUE( triangleIndices.empty() );
+}
+
+TEST_F( RifSurfaceImporterTest, HandlesOutOfBoundsInput )
+{
+    // Non-empty indexToPointData but invalid indices
+    indexToPointData = { { 0, 1 }, { 2, 3 } };
+    EXPECT_FALSE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 2, 0, triangleIndices ) );
+    EXPECT_FALSE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 0, 2, triangleIndices ) );
+    EXPECT_TRUE( triangleIndices.empty() );
+}
+
+TEST_F( RifSurfaceImporterTest, HandlesSingleResolution )
+{
+    // Valid input with resolution 1
+    indexToPointData = { { 0, 1, 2 }, { 3, 4, 5 }, { 6, 7, 8 } };
+    EXPECT_TRUE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 0, 0, triangleIndices, 1 ) );
+
+    // Check expected triangle indices
+    std::vector<unsigned> expected = { 0, 1, 4, 0, 4, 3 };
+    EXPECT_EQ( triangleIndices, expected );
+}
+
+TEST_F( RifSurfaceImporterTest, HandlesLargerResolution )
+{
+    // Valid input with larger resolution
+    indexToPointData = { { 0, 1, 2 }, { 3, 4, 5 }, { 6, 7, 8 } };
+    EXPECT_TRUE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 0, 0, triangleIndices, 2 ) );
+
+    // Check expected triangle indices
+    std::vector<unsigned> expected = { 0, 2, 8, 0, 8, 6 };
+    EXPECT_EQ( triangleIndices, expected );
+}
+
+TEST_F( RifSurfaceImporterTest, HandlesInvalidPoints )
+{
+    // Input with invalid points (-1 as unsigned max value)
+    indexToPointData = { { 0, 1, 2 }, { 3, (unsigned)-1, 5 }, { 6, 7, 8 } };
+    EXPECT_FALSE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 0, 1, triangleIndices ) );
+    EXPECT_TRUE( triangleIndices.empty() );
+}
+
+TEST_F( RifSurfaceImporterTest, PreventsDegenerateTriangles )
+{
+    // Degenerate case where resolution leads to topI == i or topJ == j
+    indexToPointData = { { 0, 1, 2 }, { 3, 4, 5 }, { 6, 7, 8 } };
+    EXPECT_FALSE( rifSurfaceImporter.generateTriangleIndices( indexToPointData, 2, 2, triangleIndices, 1 ) );
+    EXPECT_TRUE( triangleIndices.empty() );
+}