Guard against some crashes in geometry

nyalldawson · nyalldawson · commit cc1a34fceaff · 2015-07-16T16:00:46.000+10:00
diff --git a/src/core/geometry/qgscircularstringv2.cpp b/src/core/geometry/qgscircularstringv2.cpp
@@ -695,7 +695,7 @@ bool QgsCircularStringV2::insertVertex( const QgsVertexId& position, const QgsPo
 
 bool QgsCircularStringV2::moveVertex( const QgsVertexId& position, const QgsPointV2& newPos )
 {
-  if ( position.vertex < 0 || position.vertex > mX.size() )
+  if ( position.vertex < 0 || position.vertex >= mX.size() )
   {
     return false;
   }
diff --git a/src/core/geometry/qgslinestringv2.cpp b/src/core/geometry/qgslinestringv2.cpp
@@ -358,7 +358,7 @@ bool QgsLineStringV2::insertVertex( const QgsVertexId& position, const QgsPointV
 
 bool QgsLineStringV2::moveVertex( const QgsVertexId& position, const QgsPointV2& newPos )
 {
-  if ( position.vertex < 0 || position.vertex > mCoords.size() )
+  if ( position.vertex < 0 || position.vertex >= mCoords.size() )
   {
     return false;
   }
@@ -378,7 +378,7 @@ bool QgsLineStringV2::moveVertex( const QgsVertexId& position, const QgsPointV2&
 
 bool QgsLineStringV2::deleteVertex( const QgsVertexId& position )
 {
-  if ( position.vertex > ( mCoords.size() - 1 ) || position.vertex < 0 )
+  if ( position.vertex >= mCoords.size() || position.vertex < 0 )
   {
     return false;
   }
diff --git a/tests/src/python/test_qgsgeometry.py b/tests/src/python/test_qgsgeometry.py
@@ -751,6 +751,12 @@ def testMultipoint(self):
 
     def testMoveVertex(self):
         multipoint = QgsGeometry.fromWkt( "MultiPoint ((5 0),(0 0),(0 4),(5 4),(5 1),(1 1),(1 3),(4 3),(4 2),(2 2))" )
+
+        #try moving invalid vertices
+        assert not multipoint.moveVertex( 9, 9, -1 ), "move vertex succeeded when it should have failed"
+        assert not multipoint.moveVertex( 9, 9, 10 ), "move vertex succeeded when it should have failed"
+        assert not multipoint.moveVertex( 9, 9, 11 ), "move vertex succeeded when it should have failed"
+
         for i in range(0,10):
           assert multipoint.moveVertex( i+1, -1-i, i ), "move vertex %d failed" % i
         expwkt = "MultiPoint ((1 -1),(2 -2),(3 -3),(4 -4),(5 -5),(6 -6),(7 -7),(8 -8),(9 -9),(10 -10))"
@@ -767,6 +773,12 @@ def testMoveVertex(self):
         # |
         # 1-+-+-+-+-0 !
         polyline = QgsGeometry.fromWkt( "LineString (5 0, 0 0, 0 4, 5 4, 5 1, 1 1, 1 3, 4 3, 4 2, 2 2)" )
+
+        #try moving invalid vertices
+        assert not polyline.moveVertex( 9, 9, -1 ), "move vertex succeeded when it should have failed"
+        assert not polyline.moveVertex( 9, 9, 10 ), "move vertex succeeded when it should have failed"
+        assert not polyline.moveVertex( 9, 9, 11 ), "move vertex succeeded when it should have failed"
+
         assert polyline.moveVertex( 5.5, 4.5, 3 ), "move vertex failed"
         expwkt = "LineString (5 0, 0 0, 0 4, 5.5 4.5, 5 1, 1 1, 1 3, 4 3, 4 2, 2 2)"
         wkt = polyline.exportToWkt()
@@ -781,6 +793,7 @@ def testMoveVertex(self):
 
         assert not polygon.moveVertex( 3, 4, -10 ), "move vertex unexpectedly succeeded"
         assert not polygon.moveVertex( 3, 4, 7 ), "move vertex unexpectedly succeeded"
+        assert not polygon.moveVertex( 3, 4, 8 ), "move vertex unexpectedly succeeded"
 
         assert polygon.moveVertex( 1, 2, 0 ), "move vertex failed"
         expwkt = "Polygon ((1 2, 1 0, 1 1, 2 1, 2 2, 0 2, 1 2))"
@@ -803,6 +816,11 @@ def testMoveVertex(self):
         # | |     | |
         # 0-1     7-8
         polygon = QgsGeometry.fromWkt( "MultiPolygon (((0 0, 1 0, 1 1, 2 1, 2 2, 0 2, 0 0)),((4 0, 5 0, 5 2, 3 2, 3 1, 4 1, 4 0)))" )
+
+        assert not polygon.moveVertex( 3, 4, -10 ), "move vertex unexpectedly succeeded"
+        assert not polygon.moveVertex( 3, 4, 14 ), "move vertex unexpectedly succeeded"
+        assert not polygon.moveVertex( 3, 4, 15 ), "move vertex unexpectedly succeeded"
+
         assert polygon.moveVertex( 6, 2, 9 ), "move vertex failed"
         expwkt = "MultiPolygon (((0 0, 1 0, 1 1, 2 1, 2 2, 0 2, 0 0)),((4 0, 5 0, 6 2, 3 2, 3 1, 4 1, 4 0)))"
         wkt = polygon.exportToWkt()

Original file line number	Diff line number	Diff line change
`@@ -695,7 +695,7 @@ bool QgsCircularStringV2::insertVertex( const QgsVertexId& position, const QgsPo`
`695`	`695`
`696`	`696`	`bool QgsCircularStringV2::moveVertex( const QgsVertexId& position, const QgsPointV2& newPos )`
`697`	`697`	`{`
`698`		`- if ( position.vertex < 0 \|\| position.vertex > mX.size() )`
	`698`	`+ if ( position.vertex < 0 \|\| position.vertex >= mX.size() )`
`699`	`699`	`{`
`700`	`700`	`return false;`
`701`	`701`	`}`
Original file line number	Diff line number	Diff line change
`@@ -358,7 +358,7 @@ bool QgsLineStringV2::insertVertex( const QgsVertexId& position, const QgsPointV`
`358`	`358`
`359`	`359`	`bool QgsLineStringV2::moveVertex( const QgsVertexId& position, const QgsPointV2& newPos )`
`360`	`360`	`{`
`361`		`- if ( position.vertex < 0 \|\| position.vertex > mCoords.size() )`
	`361`	`+ if ( position.vertex < 0 \|\| position.vertex >= mCoords.size() )`
`362`	`362`	`{`
`363`	`363`	`return false;`
`364`	`364`	`}`
`@@ -378,7 +378,7 @@ bool QgsLineStringV2::moveVertex( const QgsVertexId& position, const QgsPointV2&`
`378`	`378`
`379`	`379`	`bool QgsLineStringV2::deleteVertex( const QgsVertexId& position )`
`380`	`380`	`{`
`381`		`- if ( position.vertex > ( mCoords.size() - 1 ) \|\| position.vertex < 0 )`
	`381`	`+ if ( position.vertex >= mCoords.size() \|\| position.vertex < 0 )`
`382`	`382`	`{`
`383`	`383`	`return false;`
`384`	`384`	`}`