Possible regression reading very small image files using stream API in 3.3.x? #1984
Description
Hello!
I've been digging into what looks to be a possible regression between 3.2.4 and 3.3.0 in the handling of very small OpenEXR images using the stream API.
Piggy-backing off of #1983, I've been testing running exrcheck using the red.exr test image in the attached red.exr.zip against various versions of OpenEXR and am finding that exrcheck red.exr (file API) and exrcheck -s red.exr (stream API) both result in file red.exr OK (success) in 3.2.4, but in 3.3.0 and the current tip of the main branch, exrcheck -s red.exr results in file red.exr bad, while exrcheck red.exr using the file API still succeeds.
I'm still trying to tease apart exactly what's going on, but my suspicion is that stream mode is always trying to read SCRATCH_BUFFER_SIZE (4096) bytes at a time, and if the entirety of the file happens to be less than that, we run into the error checking that tries to prevent reading past the end of files, for example here:
https://github.com/AcademySoftwareFoundation/openexr/blob/main/src/lib/OpenEXRUtil/ImfCheckFile.cpp#L983
Hacking around with this a bit, I was finding that reducing the SCRATCH_BUFFER_SIZE to 1024 resulted in the exrcheck -s red.exr succeeding, seemingly because the size of the data in memory is 1038 bytes, larger than the scratch buffer size.
If anyone is able to reproduce this and can confirm or deny that theory, as well as point to the right approach in addressing this, that would be much appreciated. :)
Thanks!
For some additional context, we relatively recently upgraded OpenEXR from 3.2.1 to 3.3.2 for Unreal Engine, but are now seeing that small images are failing to import. Our import of EXRs leverages the stream-based API, passing the data into OpenEXR after having read it into memory, as opposed to giving it file paths to read.
We have a class derived from Imf::IStream with its own implementation of read(), but it looks fairly similar to PtrIStream in ImfCheckFile.cpp in its attempt to avoid reading past the end of files. After an initial read of the first 8 bytes, I see our derived class' read() function being called with a size of 4096, which similarly throws, since that is larger than the total size of the data.
If you have access to the Unreal Engine source code on GitHub, you can see this class defined here:
https://github.com/EpicGames/UnrealEngine/blob/0c544b150542b59fc87bdcf64caae09f4e3bc11c/Engine/Source/Runtime/ImageWrapper/Private/Formats/ExrImageWrapper.cpp#L81
I'm unclear on whether this end-of-file error checking should be changed in Imf::IStream::read() functions, or whether maybe something should change elsewhere in the OpenEXR core that would ensure any IStream does not get a read() call past the end of its buffer.