Fix crash in QgsGeometry::splitGeometry

nyalldawson · nyalldawson · commit f77377e6ae3c · 2017-10-19T07:30:50.000+11:00
diff --git a/src/core/geometry/qgsgeos.cpp b/src/core/geometry/qgsgeos.cpp
@@ -888,25 +888,19 @@ QgsGeometryEngine::EngineOperationResult QgsGeos::splitPolygonGeometry( GEOSGeom
   }
   GEOSGeom_destroy_r( geosinit.ctxt, polygons );
 
-  bool splitDone = true;
   int nGeometriesThis = numberOfGeometries( mGeos ); //original number of geometries
-  if ( testedGeometries.size() == nGeometriesThis )
-  {
-    splitDone = false;
-  }
-
-  mergeGeometriesMultiTypeSplit( testedGeometries );
-
-  //no split done, preserve original geometry
-  if ( !splitDone )
+  if ( testedGeometries.empty() || testedGeometries.size() == nGeometriesThis )
   {
+    //no split done, preserve original geometry
     for ( int i = 0; i < testedGeometries.size(); ++i )
     {
       GEOSGeom_destroy_r( geosinit.ctxt, testedGeometries[i] );
     }
     return NothingHappened;
   }
 
+  mergeGeometriesMultiTypeSplit( testedGeometries );
+
   int i;
   for ( i = 0; i < testedGeometries.size() && GEOSisValid_r( geosinit.ctxt, testedGeometries[i] ); ++i )
     ;
diff --git a/tests/src/core/testqgsgeometry.cpp b/tests/src/core/testqgsgeometry.cpp
@@ -136,6 +136,7 @@ class TestQgsGeometry : public QObject
     void createCollectionOfType();
 
     void minimalEnclosingCircle( );
+    void splitGeometry();
 
   private:
     //! A helper method to do a render check to see if the geometry op is as expected
@@ -15553,6 +15554,16 @@ void TestQgsGeometry::minimalEnclosingCircle()
 
 }
 
+void TestQgsGeometry::splitGeometry()
+{
+  QgsGeometry g1 = QgsGeometry::fromWkt( QStringLiteral( "Polygon ((492980.38648063864093274 7082334.45244149677455425, 493082.65415841294452548 7082319.87918917648494244, 492980.38648063858272508 7082334.45244149677455425, 492980.38648063864093274 7082334.45244149677455425))" ) );
+  QList<QgsGeometry> newGeoms;
+  QList<QgsPointXY> testPoints;
+  QCOMPARE( g1.splitGeometry( QList< QgsPointXY >() << QgsPointXY( 493825.46541286131832749, 7082214.02779923938214779 ) << QgsPointXY( 492955.04876351181883365, 7082338.06309300474822521 ),
+                              newGeoms, false, testPoints ), QgsGeometry::NothingHappened );
+  QVERIFY( newGeoms.isEmpty() );
+}
+
 
 QGSTEST_MAIN( TestQgsGeometry )
 #include "testqgsgeometry.moc"

Original file line number	Diff line number	Diff line change
`@@ -888,25 +888,19 @@ QgsGeometryEngine::EngineOperationResult QgsGeos::splitPolygonGeometry( GEOSGeom`
`888`	`888`	`}`
`889`	`889`	`GEOSGeom_destroy_r( geosinit.ctxt, polygons );`
`890`	`890`
`891`		`- bool splitDone = true;`
`892`	`891`	`int nGeometriesThis = numberOfGeometries( mGeos ); //original number of geometries`
`893`		`- if ( testedGeometries.size() == nGeometriesThis )`
`894`		`- {`
`895`		`- splitDone = false;`
`896`		`- }`
`897`		`-`
`898`		`- mergeGeometriesMultiTypeSplit( testedGeometries );`
`899`		`-`
`900`		`- //no split done, preserve original geometry`
`901`		`- if ( !splitDone )`
	`892`	`+ if ( testedGeometries.empty() \|\| testedGeometries.size() == nGeometriesThis )`
`902`	`893`	`{`
	`894`	`+ //no split done, preserve original geometry`
`903`	`895`	`for ( int i = 0; i < testedGeometries.size(); ++i )`
`904`	`896`	`{`
`905`	`897`	`GEOSGeom_destroy_r( geosinit.ctxt, testedGeometries[i] );`
`906`	`898`	`}`
`907`	`899`	`return NothingHappened;`
`908`	`900`	`}`
`909`	`901`
	`902`	`+ mergeGeometriesMultiTypeSplit( testedGeometries );`
	`903`	`+`
`910`	`904`	`int i;`
`911`	`905`	`for ( i = 0; i < testedGeometries.size() && GEOSisValid_r( geosinit.ctxt, testedGeometries[i] ); ++i )`
`912`	`906`	`;`