Fix some crashes in paletted raster renderer

- handle negative color values - don't crash when calculating unique values in bad rasters
qgis · Apr 3, 2017 · 603365c · 603365c
1 parent 1cbe971
commit 603365c
Show file tree

Hide file tree

Showing 4 changed files with 4,530 additions and 32 deletions.
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()