Merge branch 'master' into pr.includes

ASWF/tsc-meetings/2020-02-06.md

@@ -0,0 +1,44 @@
+# 02/06/2020
+
+### Attending:
+
+* Cary Phillips
+* Christina Tempelaar-Lietz
+* Peter Hillman
+* Larry Gritz
+* Joseph Goldstone
+* Kimball Thurston
+* Eskil Steenburg
+
+### Discussion:
+
+* The 2.4.1 release should include only essential bug fixes. Defer
+  everything else to the next minor release.
+
+* Peter will merge recent fixes into the RB-2.4 branch via cherry
+  picking commits. Kimball will look into any cmake changes. Cary will
+  prepare the release notes.
+
+* We'll shoot for making the release Monday or Tuesday of next week. 
+
+* Cary proposes a new label/milestone scheme. Labels will indicate
+  **what** an issue is (Feature Request, Enhancement, Question, Bug,
+  etc), as well as its **status** (Needs Discussion, Awaiting
+  Feedback, etc). Milestones will indicate **when** an issue is likely
+  to be addressed (Next Patch Release, Next Minor Release, Next Major
+  Release, Backlog, etc).
+
+* Briefly discussed #650, Arkell's fix for chromaticity
+  conversion. Joseph agreed to investigate.
+
+* Christina will follow up on the Azure build failures and why the
+  SonarCloud runs don't include coverage reports.
+
+* Christina reported from the ASWF TAC CI meeting that the ASWF
+  intends to migrate from Azure to GitHub Actions.
+
+* Our Google Summer of Code projects ideas are in. Notification of
+  accepted organizations is around Feb 20.
+
+
+

OpenEXR/IlmImf/ImfChromaticities.cpp

@@ -76,7 +76,7 @@ Chromaticities::operator != (const Chromaticities & c) const
 
 
 IMATH_NAMESPACE::M44f
-RGBtoXYZ (const Chromaticities chroma, float Y)
+RGBtoXYZ (const Chromaticities &chroma, float Y)
 {
     //
     // For an explanation of how the color conversion matrix is derived,
@@ -142,7 +142,7 @@ RGBtoXYZ (const Chromaticities chroma, float Y)
 
 
 IMATH_NAMESPACE::M44f
-XYZtoRGB (const Chromaticities chroma, float Y)
+XYZtoRGB (const Chromaticities &chroma, float Y)
 {
     return RGBtoXYZ (chroma, Y).inverse();
 }

OpenEXR/IlmImf/ImfChromaticities.h

@@ -122,11 +122,11 @@ struct Chromaticities
 // 
 // XYZ to RGB:
 // 
-// 	YYZtoRGB(c,Y) returns RGBtoXYZ(c,Y).inverse().
+// 	XYZtoRGB(c,Y) returns RGBtoXYZ(c,Y).inverse().
 // 
 
-IMF_EXPORT IMATH_NAMESPACE::M44f    RGBtoXYZ (const Chromaticities chroma, float Y);
-IMF_EXPORT IMATH_NAMESPACE::M44f    XYZtoRGB (const Chromaticities chroma, float Y);
+IMF_EXPORT IMATH_NAMESPACE::M44f    RGBtoXYZ (const Chromaticities &chroma, float Y);
+IMF_EXPORT IMATH_NAMESPACE::M44f    XYZtoRGB (const Chromaticities &chroma, float Y);
 
 
 OPENEXR_IMF_INTERNAL_NAMESPACE_HEADER_EXIT

OpenEXR/IlmImf/ImfCompositeDeepScanLine.cpp

@@ -44,6 +44,7 @@
 
 #include <Iex.h>
 #include <vector>
+#include <stddef.h>
 OPENEXR_IMF_INTERNAL_NAMESPACE_SOURCE_ENTER
 
 using std::vector;
@@ -179,7 +180,7 @@ CompositeDeepScanLine::Data::handleDeepFrameBuffer (DeepFrameBuffer& buf,
                                                     int start,
                                                     int end)
 {
-    int width=_dataWindow.size().x+1;
+    ptrdiff_t width=_dataWindow.size().x+1;
     size_t pixelcount = width * (end-start+1);
     pointers.resize(_channels.size());
     counts.resize(pixelcount);

OpenEXR/IlmImf/ImfDeepScanLineInputFile.cpp

@@ -919,8 +919,7 @@ void DeepScanLineInputFile::initialize(const Header& header)
     }
     catch (...)
     {
-        delete _data;
-        _data=NULL;
+        // Don't delete _data here, leave that to caller
         throw;
     }
 }
@@ -936,8 +935,15 @@ DeepScanLineInputFile::DeepScanLineInputFile(InputPartData* part)
     _data->memoryMapped = _data->_streamData->is->isMemoryMapped();
     _data->version = part->version;
 
-    initialize(part->header);
-
+    try
+    {
+       initialize(part->header);
+    }
+    catch(...)
+    {
+        delete _data;
+        throw;
+    }
     _data->lineOffsets = part->chunkOffsets;
 
     _data->partNumber = part->partNumber;
@@ -949,7 +955,6 @@ DeepScanLineInputFile::DeepScanLineInputFile
 :
      _data (new Data (numThreads))
 {
-    _data->_streamData = new InputStreamMutex();
     _data->_deleteStream = true;
     OPENEXR_IMF_INTERNAL_NAMESPACE::IStream* is = 0;
 
@@ -959,12 +964,29 @@ DeepScanLineInputFile::DeepScanLineInputFile
         readMagicNumberAndVersionField(*is, _data->version);
         //
         // Backward compatibility to read multpart file.
-        //
+        // multiPartInitialize will create _streamData
         if (isMultiPart(_data->version))
         {
             compatibilityInitialize(*is);
             return;
         }
+    }
+    catch (IEX_NAMESPACE::BaseExc &e)
+    {
+        if (is)          delete is;
+        if (_data)       delete _data;
+
+        REPLACE_EXC (e, "Cannot read image file "
+                     "\"" << fileName << "\". " << e.what());
+        throw;
+    }
+
+    // 
+    // not multiPart - allocate stream data and intialise as normal
+    //
+    try
+    { 
+        _data->_streamData = new InputStreamMutex();
         _data->_streamData->is = is;
         _data->memoryMapped = is->isMemoryMapped();
         _data->header.readFrom (*_data->_streamData->is, _data->version);
@@ -980,7 +1002,10 @@ DeepScanLineInputFile::DeepScanLineInputFile
     catch (IEX_NAMESPACE::BaseExc &e)
     {
         if (is)          delete is;
-        if (_data && _data->_streamData) delete _data->_streamData;
+        if (_data && _data->_streamData)
+        {
+            delete _data->_streamData;
+        }
         if (_data)       delete _data;
 
         REPLACE_EXC (e, "Cannot read image file "
@@ -990,7 +1015,10 @@ DeepScanLineInputFile::DeepScanLineInputFile
     catch (...)
     {
         if (is)          delete is;
-        if (_data && _data->_streamData) delete _data->_streamData;
+        if (_data && _data->_streamData)
+        {
+            delete _data->_streamData;
+        }
         if (_data)       delete _data;
 
         throw;
@@ -1014,7 +1042,18 @@ DeepScanLineInputFile::DeepScanLineInputFile
 
     _data->version =version;
 
-    initialize (header);
+    try
+    {
+        initialize (header);
+    }
+    catch (...)
+    {
+        if (_data && _data->_streamData)
+        {
+            delete _data->_streamData;
+        }
+        if (_data)       delete _data;
+   }
 
     readLineOffsets (*_data->_streamData->is,
                      _data->lineOrder,
@@ -1046,8 +1085,9 @@ DeepScanLineInputFile::~DeepScanLineInputFile ()
         //
 
         if (_data->partNumber == -1 && _data->_streamData)
+        {
             delete _data->_streamData;
-
+        }
         delete _data;
     }
 }

OpenEXR/IlmImf/ImfDeepTiledInputFile.cpp

@@ -288,7 +288,8 @@ DeepTiledInputFile::Data::Data (int numThreads):
     multiPartBackwardSupport(false),
     numThreads(numThreads),
     memoryMapped(false),
-    _streamData(NULL),
+    sampleCountTableComp(nullptr),
+    _streamData(nullptr),
     _deleteStream(false)
 {
     //
@@ -313,6 +314,8 @@ DeepTiledInputFile::Data::~Data ()
 
     for (size_t i = 0; i < slices.size(); i++)
         delete slices[i];
+
+    delete sampleCountTableComp;
 }
 
 
@@ -883,7 +886,15 @@ DeepTiledInputFile::DeepTiledInputFile (InputPartData* part) :
     _data (new Data (part->numThreads))
 {
     _data->_deleteStream=false;
-    multiPartInitialize(part);
+    try
+    {
+       multiPartInitialize(part);
+    }
+    catch(...)
+    {
+        delete _data;
+        throw;
+    }
 }
 
 

OpenEXR/IlmImf/ImfDwaCompressor.cpp

@@ -268,8 +268,9 @@ struct DwaCompressor::Classifier
                                 " (truncated rule).");
 
         {
-            char suffix[Name::SIZE];
-            memset (suffix, 0, Name::SIZE);
+            // maximum length of string plus one byte for terminating NULL
+            char suffix[Name::SIZE+1];
+            memset (suffix, 0, Name::SIZE+1);
             Xdr::read<CharPtrIO> (ptr, std::min(size, Name::SIZE-1), suffix);
             _suffix = std::string(suffix);
         }
@@ -2421,7 +2422,7 @@ DwaCompressor::uncompress
         unsigned short ruleSize = 0;
         Xdr::read<CharPtrIO>(dataPtr, ruleSize);
 
-        if (ruleSize < 0) 
+        if (ruleSize < Xdr::size<unsigned short>() ) 
             throw IEX_NAMESPACE::InputExc("Error uncompressing DWA data"
                                 " (corrupt header file).");
 
@@ -2816,6 +2817,14 @@ DwaCompressor::uncompress
                     if (IMATH_NAMESPACE::modp (y, cd->ySampling) != 0)
                         continue;
 
+                    //
+                    // sanity check for buffer data lying within range
+                    //
+                    if (cd->planarUncBufferEnd + dstScanlineSize - _planarUncBuffer[UNKNOWN] > _planarUncBufferSize[UNKNOWN] )
+                    {
+                        throw Iex::InputExc("DWA data corrupt");
+                    }
+
                     memcpy (rowPtrs[chan][row],
                             cd->planarUncBufferEnd,
                             dstScanlineSize);

OpenEXR/IlmImf/ImfFastHuf.cpp

@@ -256,14 +256,29 @@ FastHufDecoder::FastHufDecoder
         int symbol  = *i >> 6;
 
         if (mapping[codeLen] >= static_cast<Int64>(_numSymbols))
+        {
+            delete[] _idToSymbol;
+            _idToSymbol = NULL;
             throw IEX_NAMESPACE::InputExc ("Huffman decode error "
                                            "(Invalid symbol in header).");
-
+        }
         _idToSymbol[mapping[codeLen]] = symbol;
         mapping[codeLen]++;
     }
 
-    buildTables(base, offset);
+    //
+    // exceptions can be thrown whilst building tables. Delete
+    // _idToSynmbol before re-throwing to prevent memory leak
+    //
+    try
+    {
+      buildTables(base, offset);
+    }catch(...)
+    {
+            delete[] _idToSymbol;
+            _idToSymbol = NULL;
+            throw;
+    }
 }
 
 

OpenEXR/IlmImf/ImfHeader.cpp

@@ -873,6 +873,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
     }
 
     const std::string & part_type=hasType() ? type() : "";
+
 
     if(part_type!="" && !isSupportedType(part_type))
     {
@@ -882,6 +883,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
         return;
     }
 
+    bool isDeep = isDeepData(part_type);
 
     //
     // If the file is tiled, verify that the tile description has reasonable
@@ -902,7 +904,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
 
 	const TileDescription &tileDesc = tileDescription();
 
-	if (tileDesc.xSize <= 0 || tileDesc.ySize <= 0)
+	if (tileDesc.xSize <= 0 || tileDesc.ySize <= 0 || tileDesc.xSize > INT_MAX || tileDesc.ySize > INT_MAX )
 	    throw IEX_NAMESPACE::ArgExc ("Invalid tile size in image header.");
 
 	if (maxTileWidth > 0 &&
@@ -953,7 +955,8 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
     if (!isValidCompression (this->compression()))
   	throw IEX_NAMESPACE::ArgExc ("Unknown compression type in image header.");
 
-    if(isDeepData(part_type))
+
+    if( isDeep )
     {
         if (!isValidDeepCompression (this->compression()))
             throw IEX_NAMESPACE::ArgExc ("Compression type in header not valid for deep data");
@@ -965,6 +968,8 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
     // If the file is tiled then for each channel, the type must be one of the
     // predefined values, and the x and y sampling must both be 1.
     //
+    // x and y sampling must currently also be 1 for deep scanline images
+    //
     // If the file is not tiled then for each channel, the type must be one
     // of the predefined values, the x and y coordinates of the data window's
     // upper left corner must be divisible by the x and y subsampling factors,
@@ -974,7 +979,7 @@ Header::sanityCheck (bool isTiled, bool isMultipartFile) const
 
     const ChannelList &channels = this->channels();
 
-    if (isTiled)
+    if (isTiled || isDeep)
     {
 	for (ChannelList::ConstIterator i = channels.begin();
 	     i != channels.end();