From 983e84f952392afaa13c200a332c2f9412ec3b16 Mon Sep 17 00:00:00 2001 From: John Haddon Date: Fri, 19 Jul 2024 12:03:05 +0100 Subject: [PATCH] ImageReader : Fix handling of invalid EXR `multiView` attributes There were two problems here : - We weren't checking that the attribute was an array. - When the attribute wasn't the right type, we were still setting the `singlePartMultiView` variable instead of ignoring the metadata as claimed. This later lead to an uninitialised read because `viewNames` was empty. --- Changes.md | 1 + python/GafferImageTest/ImageReaderTest.py | 29 ++++++++++++++++++ .../images/invalidMultiViewAttribute.exr | Bin 0 -> 2515 bytes src/GafferImage/OpenImageIOReader.cpp | 25 +++++++-------- 4 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 python/GafferImageTest/images/invalidMultiViewAttribute.exr diff --git a/Changes.md b/Changes.md index 797da2e9ba8..b18f9629716 100644 --- a/Changes.md +++ b/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) diff --git a/python/GafferImageTest/ImageReaderTest.py b/python/GafferImageTest/ImageReaderTest.py index c28804b6f14..b99da26f9f2 100644 --- a/python/GafferImageTest/ImageReaderTest.py +++ b/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() diff --git a/python/GafferImageTest/images/invalidMultiViewAttribute.exr b/python/GafferImageTest/images/invalidMultiViewAttribute.exr new file mode 100644 index 0000000000000000000000000000000000000000..0a1853d22ac91c125c43eb3efbab2367d87b03b3 GIT binary patch literal 2515 zcmeHHU1-}@6xOvIJ8i|>rlC_`>~t+98)cJBplfi^vUbV#u%%1?vWJ0W$4LZ^9b&Jw zXNVLGO&A70tPJlZ=?Jti25T8~8!YsOwaH7U_c$oTfsB-ahiwe8gJqpv`7h2wU-nk$ zMfd33@1Aq+xkozRjdyrtqA1EEOYU*{Ti?k;e4Ne(&Jahs!rmFIpQodP+C!5QPcy z)K*)Y`IVFKYP^gU(9V2n>WWo#mrIG8!zPo<202m4z3ySbjz3Fsx#Chby51B;(79AJ zoh_CtIw+6QSs0}$kP^{!#vD<-ch_z_E90i*3{G$d-uREU|xbwv0Y$IwPp(%(mwQ zSXKdxF$8MX8gCnBZJMQ>bCh2DBbQefY0#QLE{w0vG;IGC(vUHSv9i&#!dH`n3h4NQ z`7zL!XJ-?3S2(}9+WN{Uu*npcYFDq;VA?|N&w5gVCY~fWQRYRF?^7Qj^@&ATYa7Pc z9Yby!sY;yBavgU~t`}L%u^tUyNpKu{2M?>n&9P1@xEl?ar3z3jQ!|xrCz$4PUM*46L=ii@l8XmxdDWAq{-u|4>1ug$kAl z75rpSCmH4w7pcU{*a)w3hTM0vjGE9=u7XC003o}*_>g1C|10wSO7dW^a-UtSE#}2H z2csS|#~nLOcaH6-_k?HYD`P! zf2Ij5iq5$8SGeD~MR(%}v4e`5;5tsz-CA+s`HrsCMUq)8fCeBy3JR#h#yMb%eC<6C WcSJJEyb2`%2W+AaJK>(U@B9lxQwN{` literal 0 HcmV?d00001 diff --git a/src/GafferImage/OpenImageIOReader.cpp b/src/GafferImage/OpenImageIOReader.cpp index 5d61ade2866..68b8e50b086 100644 --- a/src/GafferImage/OpenImageIOReader.cpp +++ b/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( 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( currentSpec, 0 ); + } } } - } View *currentView = nullptr;