Merge pull request #5966 from johnhaddon/bogusMultiViewWorkaround

ImageReader : Fix handling of invalid EXR `multiView` attributes

Changes.md

@@ -4,6 +4,7 @@
 Fixes
 -----
 
+- ImageReader : Fixed crash caused by invalid OpenEXR `multiView` attributes.
 - LightEditor, RenderPassEditor : Added missing icon representing use of the `CreateIfMissing` tweak mode in the history window.
 
 1.3.16.6 (relative to 1.3.16.5)

python/GafferImageTest/ImageReaderTest.py

@@ -925,5 +925,34 @@ def testSerialisation( self ) :
 		script2.execute( serialisation )
 		self.assertIsInstance( script2["reader"], GafferImage.ImageReader )
 
+	def testInvalidMultiViewAttribute( self ) :
+
+		# Check that we ignore a `multiView` attribute which has the wrong type
+		# (string rather than string array). We don't know where they came from,
+		# but we have encountered these in the wild.
+		#
+		# This synthetic example was generated by :
+		#
+		# ```
+		# > oiiotool python/GafferImageTest/images/rgb.100x100.exr \
+		#   --attrib multiView "main cam1" \
+		#   --nosoftwareattrib \
+		#   -o python/GafferImageTest/images/invalidMultiViewAttribute.exr
+		# ````
+
+		reader = GafferImage.ImageReader()
+		reader["fileName"].setValue( self.imagesPath() / "invalidMultiViewAttribute.exr" )
+
+		expectedReader = GafferImage.ImageReader()
+		expectedReader["fileName"].setValue( self.imagesPath() / "rgb.100x100.exr" )
+
+		with IECore.CapturingMessageHandler() as mh :
+			self.assertImagesEqual( reader["out"], expectedReader["out"], metadataBlacklist = [ "DateTime" ] )
+
+		self.assertEqual( len( mh.messages ), 1 )
+		self.assertIn( 'Ignoring invalid "multiView" attribute', mh.messages[0].message )
+
+		self.assertNotIn( "multiView", reader["out"].metadata() )
+
 if __name__ == "__main__":
 	unittest.main()

src/GafferImage/OpenImageIOReader.cpp

@@ -299,30 +299,31 @@ class File
 					ParamValue *multiViewAttr = currentSpec.find_attribute( "multiView" );
 					if( multiViewAttr )
 					{
-						singlePartMultiView = true;
-
-						if( multiViewAttr->type().basetype != TypeDesc::STRING )
+						if( multiViewAttr->type().basetype != TypeDesc::STRING || multiViewAttr->type().arraylen <= 0 )
 						{
 							IECore::msg(
 								IECore::Msg::Warning, "OpenImageIOReader",
 								fmt::format(
-									"Ignoring \"multiView\" attribute of invalid type in \"{}\".",
+									"Ignoring invalid \"multiView\" attribute in \"{}\".",
 									infoFileName
 								)
 							);
 						}
-
-						for( int i = 0; i < multiViewAttr->type().arraylen; i++ )
+						else
 						{
-							viewNames.push_back( *((&multiViewAttr->get< char* >())+i) );
-						}
+							singlePartMultiView = true;
 
-						for( const std::string &view : viewNames )
-						{
-							m_views[view] = std::make_unique<View>( currentSpec, 0 );
+							for( int i = 0; i < multiViewAttr->type().arraylen; i++ )
+							{
+								viewNames.push_back( *((&multiViewAttr->get< char* >())+i) );
+							}
+
+							for( const std::string &view : viewNames )
+							{
+								m_views[view] = std::make_unique<View>( currentSpec, 0 );
+							}
 						}
 					}
-
 				}
 
 				View *currentView = nullptr;