Fix some crashes in paletted raster renderer

nyalldawson · nyalldawson · commit 603365c38628 · 2017-04-03T12:08:08.000+10:00
- handle negative color values
- don't crash when calculating unique values in bad rasters
diff --git a/src/core/raster/qgspalettedrasterrenderer.cpp b/src/core/raster/qgspalettedrasterrenderer.cpp
@@ -35,12 +35,6 @@ QgsPalettedRasterRenderer::QgsPalettedRasterRenderer( QgsRasterInterface *input,
   updateArrays();
 }
 
-QgsPalettedRasterRenderer::~QgsPalettedRasterRenderer()
-{
-  delete[] mColors;
-  delete[] mIsNoData;
-}
-
 QgsPalettedRasterRenderer *QgsPalettedRasterRenderer::clone() const
 {
   QgsPalettedRasterRenderer *renderer = new QgsPalettedRasterRenderer( nullptr, mBand, mClassData );
@@ -181,15 +175,15 @@ QgsRasterBlock *QgsPalettedRasterRenderer::block( int bandNo, QgsRectangle  cons
       continue;
     }
     int val = ( int ) inputBlock->value( i );
-    if ( val > mMaxColorIndex || mIsNoData[ val ] )
+    if ( !mColors.contains( val ) )
     {
       outputData[i] = myDefaultColor;
       continue;
     }
 
     if ( !hasTransparency )
     {
-      outputData[i] = mColors[val];
+      outputData[i] = mColors.value( val );
     }
     else
     {
@@ -203,7 +197,7 @@ QgsRasterBlock *QgsPalettedRasterRenderer::block( int bandNo, QgsRectangle  cons
         currentOpacity *= alphaBlock->value( i ) / 255.0;
       }
 
-      QRgb c = mColors[val];
+      QRgb c = mColors.value( val );
       outputData[i] = qRgba( currentOpacity * qRed( c ), currentOpacity * qGreen( c ), currentOpacity * qBlue( c ), currentOpacity * qAlpha( c ) );
     }
   }
@@ -415,6 +409,8 @@ QgsPalettedRasterRenderer::ClassData QgsPalettedRasterRenderer::classDataFromRas
   double max = stats.maximumValue;
   // need count of every individual value
   int bins = ceil( max - min ) + 1;
+  if ( bins <= 0 )
+    return ClassData();
 
   QgsRasterHistogram histogram = raster->histogram( bandNumber, bins, min, max, QgsRectangle(), 0, false, feedback );
   if ( feedback && feedback->isCanceled() )
@@ -453,26 +449,12 @@ QgsPalettedRasterRenderer::ClassData QgsPalettedRasterRenderer::classDataFromRas
 
 void QgsPalettedRasterRenderer::updateArrays()
 {
-  // find maximum color index
-  ClassData::const_iterator mIt = mClassData.constBegin();
-  for ( ; mIt != mClassData.constEnd(); ++mIt )
-  {
-    mMaxColorIndex = qMax( mMaxColorIndex, mIt->value );
-  }
-  mMaxColorIndex = qMax( 0, mMaxColorIndex );
-
-  delete [] mColors;
-  delete [] mIsNoData;
-  mColors = new QRgb[mMaxColorIndex + 1];
-  mIsNoData = new bool[mMaxColorIndex + 1];
-  std::fill( mIsNoData, mIsNoData + mMaxColorIndex, true );
-
+  mColors.clear();
   int i = 0;
   ClassData::const_iterator it = mClassData.constBegin();
   for ( ; it != mClassData.constEnd(); ++it )
   {
-    mColors[ it->value] = qPremultiply( it->color.rgba() );
-    mIsNoData[it->value] = false;
+    mColors[it->value] = qPremultiply( it->color.rgba() );
     i++;
   }
 }
diff --git a/src/core/raster/qgspalettedrasterrenderer.h b/src/core/raster/qgspalettedrasterrenderer.h
@@ -60,7 +60,6 @@ class CORE_EXPORT QgsPalettedRasterRenderer: public QgsRasterRenderer
      * Constructor for QgsPalettedRasterRenderer.
      */
     QgsPalettedRasterRenderer( QgsRasterInterface *input, int bandNumber, const ClassData &classes );
-    ~QgsPalettedRasterRenderer();
 
     //! QgsPalettedRasterRenderer cannot be copied. Use clone() instead.
     QgsPalettedRasterRenderer( const QgsPalettedRasterRenderer & ) = delete;
@@ -146,15 +145,13 @@ class CORE_EXPORT QgsPalettedRasterRenderer: public QgsRasterRenderer
   private:
 
     int mBand;
-    int mMaxColorIndex = -INT_MAX;
     ClassData mClassData;
 
     //! Source color ramp
     std::unique_ptr<QgsColorRamp> mSourceColorRamp;
 
-    //! Premultiplied color array
-    QRgb *mColors = nullptr;
-    bool *mIsNoData = nullptr;
+    //! Premultiplied color map
+    QMap< int, QRgb > mColors;
     void updateArrays();
 };
 
diff --git a/tests/src/python/test_qgsrasterlayer.py b/tests/src/python/test_qgsrasterlayer.py
@@ -589,8 +589,7 @@ def testPalettedClassDataFromLayer(self):
         self.assertEqual(classes[9].label, '10')
 
         # bad band
-        with self.assertRaises(Exception):
-            classes = QgsPalettedRasterRenderer.classDataFromRaster(layer10.dataProvider(), 10101010)
+        self.assertFalse(QgsPalettedRasterRenderer.classDataFromRaster(layer10.dataProvider(), 10101010))
 
         # with ramp
         r = QgsGradientColorRamp(QColor(200, 0, 0, 100), QColor(0, 200, 0, 200))
@@ -618,6 +617,32 @@ def testPalettedClassDataFromLayer(self):
         expected = [11, 21, 22, 24, 31, 82, 2002, 2004, 2014, 2019, 2027, 2029, 2030, 2080, 2081, 2082, 2088, 2092, 2097, 2098, 2099, 2105, 2108, 2110, 2114, 2118, 2126, 2152, 2184, 2220]
         self.assertEqual([c.value for c in classes], expected)
 
+        # bad layer
+        path = os.path.join(unitTestDataPath('raster'),
+                            'hub13263.vrt')
+        info = QFileInfo(path)
+        base_name = info.baseName()
+        layer = QgsRasterLayer(path, base_name)
+        classes = QgsPalettedRasterRenderer.classDataFromRaster(layer.dataProvider(), 1)
+        self.assertFalse(classes)
+
+    def testPalettedRendererWithNegativeColorValue(self):
+        """ test paletted raster renderer with negative values in color table"""
+
+        path = os.path.join(unitTestDataPath('raster'),
+                            'hub13263.vrt')
+        info = QFileInfo(path)
+        base_name = info.baseName()
+        layer = QgsRasterLayer(path, base_name)
+        self.assertTrue(layer.isValid(), 'Raster not loaded: {}'.format(path))
+
+        renderer = QgsPalettedRasterRenderer(layer.dataProvider(), 1,
+                                             [QgsPalettedRasterRenderer.Class(-1, QColor(0, 255, 0), 'class 2'),
+                                              QgsPalettedRasterRenderer.Class(3, QColor(255, 0, 0), 'class 1')])
+
+        self.assertEqual(renderer.nColors(), 2)
+        self.assertEqual(renderer.usesBands(), [1])
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/tests/testdata/raster/hub13263.vrt b/tests/testdata/raster/hub13263.vrt

Original file line number	Diff line number	Diff line change
`@@ -35,12 +35,6 @@ QgsPalettedRasterRenderer::QgsPalettedRasterRenderer( QgsRasterInterface *input,`
`35`	`35`	`updateArrays();`
`36`	`36`	`}`
`37`	`37`
`38`		`-QgsPalettedRasterRenderer::~QgsPalettedRasterRenderer()`
`39`		`-{`
`40`		`- delete[] mColors;`
`41`		`- delete[] mIsNoData;`
`42`		`-}`
`43`		`-`
`44`	`38`	`QgsPalettedRasterRenderer *QgsPalettedRasterRenderer::clone() const`
`45`	`39`	`{`
`46`	`40`	`QgsPalettedRasterRenderer *renderer = new QgsPalettedRasterRenderer( nullptr, mBand, mClassData );`
`@@ -181,15 +175,15 @@ QgsRasterBlock *QgsPalettedRasterRenderer::block( int bandNo, QgsRectangle cons`
`181`	`175`	`continue;`
`182`	`176`	`}`
`183`	`177`	`int val = ( int ) inputBlock->value( i );`
`184`		`- if ( val > mMaxColorIndex \|\| mIsNoData[ val ] )`
	`178`	`+ if ( !mColors.contains( val ) )`
`185`	`179`	`{`
`186`	`180`	`outputData[i] = myDefaultColor;`
`187`	`181`	`continue;`
`188`	`182`	`}`
`189`	`183`
`190`	`184`	`if ( !hasTransparency )`
`191`	`185`	`{`
`192`		`- outputData[i] = mColors[val];`
	`186`	`+ outputData[i] = mColors.value( val );`
`193`	`187`	`}`
`194`	`188`	`else`
`195`	`189`	`{`
`@@ -203,7 +197,7 @@ QgsRasterBlock *QgsPalettedRasterRenderer::block( int bandNo, QgsRectangle cons`
`203`	`197`	`currentOpacity *= alphaBlock->value( i ) / 255.0;`
`204`	`198`	`}`
`205`	`199`
`206`		`- QRgb c = mColors[val];`
	`200`	`+ QRgb c = mColors.value( val );`
`207`	`201`	`outputData[i] = qRgba( currentOpacity * qRed( c ), currentOpacity * qGreen( c ), currentOpacity * qBlue( c ), currentOpacity * qAlpha( c ) );`
`208`	`202`	`}`
`209`	`203`	`}`
`@@ -415,6 +409,8 @@ QgsPalettedRasterRenderer::ClassData QgsPalettedRasterRenderer::classDataFromRas`
`415`	`409`	`double max = stats.maximumValue;`
`416`	`410`	`// need count of every individual value`
`417`	`411`	`int bins = ceil( max - min ) + 1;`
	`412`	`+ if ( bins <= 0 )`
	`413`	`+ return ClassData();`
`418`	`414`
`419`	`415`	`QgsRasterHistogram histogram = raster->histogram( bandNumber, bins, min, max, QgsRectangle(), 0, false, feedback );`
`420`	`416`	`if ( feedback && feedback->isCanceled() )`
`@@ -453,26 +449,12 @@ QgsPalettedRasterRenderer::ClassData QgsPalettedRasterRenderer::classDataFromRas`
`453`	`449`
`454`	`450`	`void QgsPalettedRasterRenderer::updateArrays()`
`455`	`451`	`{`
`456`		`- // find maximum color index`
`457`		`- ClassData::const_iterator mIt = mClassData.constBegin();`
`458`		`- for ( ; mIt != mClassData.constEnd(); ++mIt )`
`459`		`- {`
`460`		`- mMaxColorIndex = qMax( mMaxColorIndex, mIt->value );`
`461`		`- }`
`462`		`- mMaxColorIndex = qMax( 0, mMaxColorIndex );`
`463`		`-`
`464`		`- delete [] mColors;`
`465`		`- delete [] mIsNoData;`
`466`		`- mColors = new QRgb[mMaxColorIndex + 1];`
`467`		`- mIsNoData = new bool[mMaxColorIndex + 1];`
`468`		`- std::fill( mIsNoData, mIsNoData + mMaxColorIndex, true );`
`469`		`-`
	`452`	`+ mColors.clear();`
`470`	`453`	`int i = 0;`
`471`	`454`	`ClassData::const_iterator it = mClassData.constBegin();`
`472`	`455`	`for ( ; it != mClassData.constEnd(); ++it )`
`473`	`456`	`{`
`474`		`- mColors[ it->value] = qPremultiply( it->color.rgba() );`
`475`		`- mIsNoData[it->value] = false;`
	`457`	`+ mColors[it->value] = qPremultiply( it->color.rgba() );`
`476`	`458`	`i++;`
`477`	`459`	`}`
`478`	`460`	`}`