Merge pull request #36648 from uclaros/backport-34368

elpaso · web-flow · commit 1a1aa6222fc6 · 2020-05-22T12:28:05.000+02:00
Backport #34368 to 3.10
diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp
@@ -4382,7 +4382,10 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds
   }
   else
   {
-    OGR_L_SetAttributeFilter( layer, encoding->fromUnicode( subsetString ).constData() );
+    if ( OGR_L_SetAttributeFilter( layer, encoding->fromUnicode( subsetString ).constData() ) != OGRERR_NONE )
+    {
+      return nullptr;
+    }
     subsetLayer = layer;
   }
 
diff --git a/tests/src/python/test_provider_shapefile.py b/tests/src/python/test_provider_shapefile.py
@@ -769,6 +769,27 @@ def _lessdigits(s):
         self.assertEqual(_lessdigits(subSet_vl.extent().toString()), filtered_extent)
         self.assertNotEqual(_lessdigits(subSet_vl.extent().toString()), unfiltered_extent)
 
+    def testMalformedSubsetStrings(self):
+        """Test that invalid where clauses always return false"""
+
+        testPath = TEST_DATA_DIR + '/' + 'lines.shp'
+
+        vl = QgsVectorLayer(testPath, 'subset_test', 'ogr')
+        self.assertTrue(vl.isValid())
+        self.assertTrue(vl.setSubsetString(''))
+        self.assertTrue(vl.setSubsetString('"Name" = \'Arterial\''))
+        self.assertTrue(vl.setSubsetString('select * from lines where "Name" = \'Arterial\''))
+        self.assertFalse(vl.setSubsetString('this is invalid sql'))
+        self.assertFalse(vl.setSubsetString('select * from lines where "NonExistentField" = \'someValue\''))
+        self.assertFalse(vl.setSubsetString('select * from lines where "Name" = \'Arte...'))
+        self.assertFalse(vl.setSubsetString('select * from lines where "Name" in (\'Arterial\', \'Highway\' '))
+        self.assertFalse(vl.setSubsetString('select * from NonExistentTable'))
+        self.assertFalse(vl.setSubsetString('select NonExistentField from lines'))
+        self.assertFalse(vl.setSubsetString('"NonExistentField" = \'someValue\''))
+        self.assertFalse(vl.setSubsetString('"Name" = \'Arte...'))
+        self.assertFalse(vl.setSubsetString('"Name" in (\'Arterial\', \'Highway\' '))
+        self.assertTrue(vl.setSubsetString(''))
+
     def testMultipatch(self):
         """Check that we can deal with multipatch shapefiles, returned natively by OGR as GeometryCollection of TIN"""
 

Original file line number	Diff line number	Diff line change
`@@ -4382,7 +4382,10 @@ OGRLayerH QgsOgrProviderUtils::setSubsetString( OGRLayerH layer, GDALDatasetH ds`
`4382`	`4382`	`}`
`4383`	`4383`	`else`
`4384`	`4384`	`{`
`4385`		`- OGR_L_SetAttributeFilter( layer, encoding->fromUnicode( subsetString ).constData() );`
	`4385`	`+ if ( OGR_L_SetAttributeFilter( layer, encoding->fromUnicode( subsetString ).constData() ) != OGRERR_NONE )`
	`4386`	`+ {`
	`4387`	`+ return nullptr;`
	`4388`	`+ }`
`4386`	`4389`	`subsetLayer = layer;`
`4387`	`4390`	`}`
`4388`	`4391`