Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Glide will print Exception when load webp image #4778

Closed
aaronKueng opened this issue Mar 30, 2022 · 0 comments
Closed

Glide will print Exception when load webp image #4778

aaronKueng opened this issue Mar 30, 2022 · 0 comments

Comments

@aaronKueng
Copy link

Invalid image: ExifInterface got an unsupported image format file(ExifInterface supports JPEG and some RAW image formats only) or a corrupted JPEG file to ExifInterface

W/ExifInterface: Invalid image: ExifInterface got an unsupported image format file(ExifInterface supports JPEG and some RAW image formats only) or a corrupted JPEG file to ExifInterface.
java.io.IOException: Invalid byte order: 4646
at androidx.exifinterface.media.ExifInterface.readByteOrder(ExifInterface.java:6581)
at androidx.exifinterface.media.ExifInterface.parseTiffHeaders(ExifInterface.java:6588)
at androidx.exifinterface.media.ExifInterface.getRawAttributes(ExifInterface.java:5617)
at androidx.exifinterface.media.ExifInterface.loadAttributes(ExifInterface.java:4583)
at androidx.exifinterface.media.ExifInterface.(ExifInterface.java:4032)
at androidx.exifinterface.media.ExifInterface.(ExifInterface.java:3983)
at com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser.getOrientation(ExifInterfaceImageHeaderParser.java:40)
at com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser.getOrientation(ExifInterfaceImageHeaderParser.java:53)
at com.bumptech.glide.load.ImageHeaderParserUtils$4.getOrientation(ImageHeaderParserUtils.java:147)
at com.bumptech.glide.load.ImageHeaderParserUtils.getOrientationInternal(ImageHeaderParserUtils.java:222)
at com.bumptech.glide.load.ImageHeaderParserUtils.getOrientation(ImageHeaderParserUtils.java:142)
at com.bumptech.glide.load.resource.bitmap.ImageReader$ByteBufferReader.getImageOrientation(ImageReader.java:166)
at com.bumptech.glide.load.resource.bitmap.Downsampler.decodeFromWrappedStreams(Downsampler.java:330)
at com.bumptech.glide.load.resource.bitmap.Downsampler.decode(Downsampler.java:285)
at com.bumptech.glide.load.resource.bitmap.Downsampler.decode(Downsampler.java:187)
at com.bumptech.glide.load.resource.bitmap.ByteBufferBitmapDecoder.decode(ByteBufferBitmapDecoder.java:28)
at com.bumptech.glide.load.resource.bitmap.ByteBufferBitmapDecoder.decode(ByteBufferBitmapDecoder.java:12)
at com.bumptech.glide.load.engine.DecodePath.decodeResourceWithList(DecodePath.java:92)
at com.bumptech.glide.load.engine.DecodePath.decodeResource(DecodePath.java:70)
at com.bumptech.glide.load.engine.DecodePath.decode(DecodePath.java:59)
at com.bumptech.glide.load.engine.LoadPath.loadWithExceptionList(LoadPath.java:76)
at com.bumptech.glide.load.engine.LoadPath.load(LoadPath.java:57)
at com.bumptech.glide.load.engine.DecodeJob.runLoadPath(DecodeJob.java:535)
at com.bumptech.glide.load.engine.DecodeJob.decodeFromFetcher(DecodeJob.java:499)
at com.bumptech.glide.load.engine.DecodeJob.decodeFromData(DecodeJob.java:485)
at com.bumptech.glide.load.engine.DecodeJob.decodeFromRetrievedData(DecodeJob.java:430)
at com.bumptech.glide.load.engine.DecodeJob.onDataFetcherReady(DecodeJob.java:394)
at com.bumptech.glide.load.engine.DataCacheGenerator.onDataReady(DataCacheGenerator.java:100)
at com.bumptech.glide.load.model.ByteBufferFileLoader$ByteBufferFetcher.loadData(ByteBufferFileLoader.java:62)
at com.bumptech.glide.load.engine.DataCacheGenerator.startNext(DataCacheGenerator.java:77)
at com.bumptech.glide.load.engine.DecodeJob.runGenerators(DecodeJob.java:311)
at com.bumptech.glide.load.engine.DecodeJob.runWrapped(DecodeJob.java:277)
at com.bumptech.glide.load.engine.DecodeJob.run(DecodeJob.java:235)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1162)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:636)
at com.bumptech.glide.load.engine.executor.GlideExecutor$DefaultThreadFactory$1.run(GlideExecutor.java:413)
at java.lang.Thread.run(Thread.java:764)
at com.bumptech.glide.load.engine.executor.GlideExecutor$DefaultPriorityThreadFactory$1.run(GlideExecutor.java:372)

Actually,It is not a bug from Android ExifInterface , but it is a bug of Glide, and happened in Webp image.
1, Why the exception happened ?
Before the exception happened, I debug the code and find that the image mime type check result is "unknown".
I found that , the ExifInterfaceImageHeaderParser get wrong header data which like this " [70, 70, -86, 8, 0, 0, 87, 69, 66, 80, 86, ......." .
Webp file starts with "RIFFxxxxWebpxxxx", must start with "RIFF" four characters , but the start characters begin with "FF", and "RI" is lost.

2, Why "FF" and not "RIFF"
First, I check the cache file, and the header start with "RIFF" , so it is not the problem of image file.
Then, check the code of getOrientation function , We know that there are two image header parsers do the work:
DefaultImageHeaderParser
ExifInterfaceImageHeaderParser
Two parsers share one image file input stream object.
The first parser DefaultImageHeaderParser call imageReader obj to get getUInt16, and check image number;
Because it is webp file, so it is not DefaultImageHeaderParser's work, so return unknown and the next parser continue the work.

While ExifInterfaceImageHeaderParser is working, but it does not do reset for the shared stream obj.
Because, the first parser read one UInt16, 2 bytes, so the second parser read header buffer from position = 2, not 0.
So, The begin 2 characters lost.
Based on webp header def:
RIFF Header - 32 bits representing the ASCII characters ‘R’ ‘I’ ‘F’ ‘F’
File Size - 32 bits (uint32) representing the size of the file in bytes starting at offset 8. The maximum value of this field is 2^32 minus 10 bytes and thus the size of the whole file is at most 4GiB minus 2 bytes.
‘WEBP’ - 32 bits representing the ASCII characters ‘W’ ‘E’ ‘B’ ‘P’

We know that, webp file type check, the first four characters are the key point. So the mime type check failed.
Then, exif info check run into unknown file type, and parser header based on JPEG, so throw the exception like :
"W/ExifInterface: Invalid image: ExifInterface got an unsupported image format file(ExifInterface supports JPEG and some RAW image formats only) or a corrupted JPEG file to ExifInterface. "

Glide provide wrong stream byte data is the root cause. It is not the problem of ExifInterface.
Of course, It is based on if you refer ExifInterface version = 1.2.0.
Because ExifInterface support webp header parse from 1.2.0, maybe, I found webp parsing code in 1.2.0 and not check earlier version.

So the suggestion is that adding reset code for input stream before ExifInterfaceImageHeaderParser do any parsing action in new version.

I did this check with glide 4.13.1 and ExifInterface 1.2.0.

Originally posted by @aaronKueng in #4451 (comment)

copybara-service bot pushed a commit that referenced this issue Mar 31, 2022
Previously we were rewinding InputStreams reliably, but not rewinding ByteBuffers. This could mean that only the first parser had the chance to read the data correctly, even if multiple were specified.

ParcelFileDescriptor is pretty confusing. Prior to this change we were closing the InputStream. In a Robolectric environment, closing the InputStream seems to close the file descriptor, causing subsequent parsers to throw IOExceptions when they tried to read data. However on an emulator, closing the stream doesn't seem to close the file descriptor and things seem to work. I have made a change to explicitly avoid closing the stream when we create one from a ParcelFileDescriptor because that seems to align with the expected behavior in the method. The caller should be responsible for creating and closing the resource. All that said though, I'm not sure this makes a difference outside of Robolectric.

I believe this fixes #4778

PiperOrigin-RevId: 438609502
copybara-service bot pushed a commit that referenced this issue Mar 31, 2022
Previously we were rewinding InputStreams reliably, but not rewinding ByteBuffers. This could mean that only the first parser had the chance to read the data correctly, even if multiple were specified.

ParcelFileDescriptor is pretty confusing. Prior to this change we were closing the InputStream. In a Robolectric environment, closing the InputStream seems to close the file descriptor, causing subsequent parsers to throw IOExceptions when they tried to read data. However on an emulator, closing the stream doesn't seem to close the file descriptor and things seem to work. I have made a change to explicitly avoid closing the stream when we create one from a ParcelFileDescriptor because that seems to align with the expected behavior in the method. The caller should be responsible for creating and closing the resource. All that said though, I'm not sure this makes a difference outside of Robolectric.

I believe this fixes #4778

PiperOrigin-RevId: 438609502
copybara-service bot pushed a commit that referenced this issue Mar 31, 2022
Previously we were rewinding InputStreams reliably, but not rewinding ByteBuffers. This could mean that only the first parser had the chance to read the data correctly, even if multiple were specified.

ParcelFileDescriptor is pretty confusing. Prior to this change we were closing the InputStream. In a Robolectric environment, closing the InputStream seems to close the file descriptor, causing subsequent parsers to throw IOExceptions when they tried to read data. However on an emulator, closing the stream doesn't seem to close the file descriptor and things seem to work. I have made a change to explicitly avoid closing the stream when we create one from a ParcelFileDescriptor because that seems to align with the expected behavior in the method. The caller should be responsible for creating and closing the resource. All that said though, I'm not sure this makes a difference outside of Robolectric.

I believe this fixes #4778

PiperOrigin-RevId: 438609502
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant