Inconsistent behavior with WebP images #392

dekuashe · 2015-04-03T19:44:20Z

Glide Version/Integration library (if any): 3.5.2
Device/Android Version: 4.2.0/4.2.2 (from what I've tested)
Issue details/Repro steps:
In short, WebP images loaded from the network just show up as black, even if the placeholder and error images are set to something else. However, if I use DiskCacheStrategy.SOURCE, they're fine. I haven't tested 4.3, but everything KitKat and above works fine. 4.2.x doesn't (both in 4.2.0 emulator and on a 4.2.2 Verizon Galaxy Nexus). I've done quite a bit of testing with this, and I've examined libraries that appear to work fine with WebP images such as Volley, and the only real difference I can spot for now is that the broken images are loaded from a network stream, and the working images are loaded from a byte array or from disk.

So I set up a test.

The first image uses URL to open an InputStream, which is thrown directly into BitmapFactory.decodeStream(). It's black or is only partially loaded on 4.2.x.
return BitmapFactory.decodeStream(url.openConnection().getInputStream());

The second image first writes the InputStream to a byte array, which is then sent to BitmapFactory.decodeStream with a ByteArrayInputStream. It loads fine.

InputStream is = url.openConnection().getInputStream();
ByteArrayOutputStream bo = new ByteArrayOutputStream();
int nRead;
byte[] data = new byte[1024];
while ((nRead = is.read(data, 0, data.length)) != -1) {
    bo.write(data, 0, nRead);
}
bo.flush();
byte[] imageBytes = bo.toByteArray();

return BitmapFactory.decodeStream(new ByteArrayInputStream(imageBytes));

The last image first writes the InputStream to a File, which is then sent to BitmapFactory.decodeStream with a FileInputStream. It also loads fine.

InputStream is = url.openConnection().getInputStream();
FileOutputStream bo = new FileOutputStream(cacheFile);
int nRead;
byte[] data = new byte[1024];
while ((nRead = is.read(data, 0, data.length)) != -1) {
    bo.write(data, 0, nRead);
}
bo.flush();
bo.close();

return BitmapFactory.decodeStream(new FileInputStream(cacheFile));

My Google-fu is failing me at the moment for finding what exactly this particular bug is and why it's only seemingly specific to WebP, but they've clearly fixed it in later versions of Android. I assume a wrapped InputStream of some kind can fix the issue?

Glide load line:

Glide.with(activity) in onAttach of Fragment.

The text was updated successfully, but these errors were encountered:

dekuashe · 2015-04-03T20:11:06Z

I found something in the AOSP issue tracker: https://code.google.com/p/android/issues/detail?id=52709

sjudd · 2015-04-06T01:21:59Z

Great report, thanks for the link as well. I'll push a fix at least for 4.2.2 shortly based on that link.

Fixes bumptech#392. Conflicts: integration/okhttp/src/main/java/com/bumptech/glide/integration/okhttp/OkHttpStreamFetcher.java library/src/androidTest/java/com/bumptech/glide/load/data/HttpUrlFetcherTest.java library/src/main/java/com/bumptech/glide/load/data/HttpUrlFetcher.java

mufumbo · 2015-04-08T05:24:05Z

I see Picasso had the same issue:
https://www.dropbox.com/s/mwynrgblys9vrce/Screenshot%202015-04-07%2022.23.26.png?dl=0
https://github.com/square/picasso/blob/master/CHANGELOG.md

"Fix: Detect and decode WebP streams from byte array."

dekuashe · 2015-05-22T15:21:47Z

Edit: I just checked, and disabling OkHttp works fine, as well as the DiskCacheStrategy.SOURCE workaround if any of that helps you at all.

I know this is marked as "closed", and I see the commit for the fix... but there is still an issue with the OkHttp integration library release (1.3.0) corresponding to 3.6.0. I haven't checked the other integration libraries.

I see in the commit that a Content-Length header is being read to find the length in OkHttpStreamFetcher:

String contentLength = response.header(CONTENT_LENGTH_HEADER);
stream = ContentLengthInputStream.obtain(responseBody.byteStream(), contentLength);

but in the 1.3.0 release, it's still trying to reading the length based on the size of the response body, which was part of the problem this commit was trying to solve in the first place, right?:

long contentLength = responseBody.contentLength();
stream = ContentLengthInputStream.obtain(responseBody.byteStream(), contentLength);

However, it's still using the new ContentLengthInputStream, so clearly parts of the commit were taken, but not all.

TWiStErRob · 2015-05-22T15:54:06Z

Looking at 3.6.0 tag blame it looks ok: 64be9d3 introduced ContentLengthInputStream and 02c4246 modifies how length is read from the response. I think the idea here is that okhttp integration lib polyfills the stream length so the webp decoder works working around http://b.android.com/52709. I guess Sam chose this solution because the okhttp internal stream didn't report length correctly and the header is more trustworthy.

sjudd · 2015-05-22T15:57:02Z

It's a little hard to tell for sure, now that you bring it up, but my assumption was that the content lengths parsed here are identical.

OkHttp's ResponseBody content length seems to be based either on a buffer size, or on the content length header. There are a couple of implementations of the abstract class, but at least this one just parses the length from the header: https://github.com/square/okhttp/blob/master/okhttp/src/main/java/com/squareup/okhttp/internal/http/RealResponseBody.java#L37.

Do you have a case where the content length from the header doesn't match the one in the response body? And to be clear, can you still reproduce the issue with the OkHttp integration library on 3.6/1.3?

dekuashe · 2015-05-22T16:00:54Z

I'll first answer you real quickly that yes, the issue is with Glide 3.6 and OkHttp integration lib 1.3.

I'll get back to you with the rest. I'm loading WebP images from mostly 2 sources, and I just want to be sure the server isn't to blame.

dekuashe · 2015-05-22T18:34:15Z

The headers don't appear to be all that reliable. As soon as a "Vary" header is included in the response, the content length is reported as 0 even if the Content-Length header is included in the response as well. I get the same results using java.net.URL. As soon as "Vary" is removed it gets the right content length and WebP decoding works again.

After a quick chat with the sys admin, the core of the issue was the server had the deflate module enabled, but wasn't excluding webp from the list. It wasn't being compressed or anything, it was just adding the Vary header is all. I could see this being a somewhat common issue.

sjudd · 2015-05-25T06:42:45Z

Thanks for the feedback and follow up. I'm surprised that just including the Vary header causes OkHttp to return 0 as the content length. I'd guess they have a reason for doing so, but you might want to open an issue with them just in case.

That said, I think we can still do better here too. I filed #470 to track a more robust solution that will function, albiet with worse performance, when the content length header isn't available.

dekuashe · 2015-05-25T07:20:59Z

Thanks. I figured they had a reason for doing so as well. I'll be taking a look through the spec before I go ahead and file an issue though. I'll follow up here letting you know if I did or didn't (and why if I didn't).

TWiStErRob · 2015-05-25T09:24:55Z

Hmm, I went ahead and quickly searched RFC 2068 and couldn't find anything that would warrant this behaviour. As I understood Vary says which fields may change if you make the request again when server dynamically generates content. I can't see anything that would invalidate those header fields for the current response. Curiously waiting for what you find out.

dekuashe · 2015-05-26T15:05:15Z

RFC 2068 definitely doesn't warrant this behavior. It's a bug. I'm submitting a report with the OkHttp project soon, but what I find more interesting is the java.net.URLConnection implementation (from java.net.Url.openConnection()). I did some quick tests, and the Oracle JVMs are all fine (tested 1.6+). Android 4.1 is also OK. It breaks starting on 4.2 and the bug is still present in 5.1, so their implementation changed starting in 4.2. I'll be submitting a bug report to AOSP as well.

What a can of worms this turned out to be.

sjudd · 2015-05-26T15:09:45Z

Somewhere in there AOSP started using OkHttp to implement HttpUrlConnection. I'm not sure if it was as early as 4.2 though.

dekuashe · 2015-05-26T15:37:25Z

It looks like the internal OkHttp implementation started in 4.4: https://twitter.com/JakeWharton/status/482563299511250944

Here's my report for OkHttp: square/okhttp#1675
and another for AOSP, just for good measure: https://code.google.com/p/android/issues/detail?id=174737

dekuashe · 2015-05-27T14:31:26Z

Well, now I feel completely foolish. Turns out the contents were being gzipped after all, and I somehow overlooked that fact in all my test cases. The Vary header has nothing to do with it. And of course, if it's gzipped, you have to measure the content length by consuming the entire stream, so OkHttp is working as intended. I assume URLConnection in 4.2 started adding gzip in requests by default and that's why it breaks there as well.

Palm, meet forehead.

In any case, if Content-Length isn't available, it still needs a workaround. I'll go ahead and add a note to #470 with a couple of these details.

sjudd · 2015-05-27T15:04:26Z

Ahh no worries, thanks for the detailed follow up. I should have mentioned something as well, we specifically work around this in our default fetcher by setting Accept Encoding: https://github.com/bumptech/glide/blob/master/library/src/main/java/com/bumptech/glide/load/data/HttpUrlFetcher.java#L93

sjudd added this to the 3.6.0 milestone Apr 6, 2015

sjudd added the bug label Apr 6, 2015

sjudd mentioned this issue Apr 6, 2015

SIGSEGV on android 4.3 (galaxy nexus) JWR66Y #372

Closed

sjudd closed this as completed in 64be9d3 Apr 6, 2015

sjudd mentioned this issue Apr 13, 2015

Loading .webp images using thumbor #403

Closed

TWiStErRob mentioned this issue Sep 30, 2015

Handle WebP images without content length headers. #470

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Inconsistent behavior with WebP images #392

Inconsistent behavior with WebP images #392

dekuashe commented Apr 3, 2015

dekuashe commented Apr 3, 2015

sjudd commented Apr 6, 2015

mufumbo commented Apr 8, 2015

dekuashe commented May 22, 2015

TWiStErRob commented May 22, 2015

sjudd commented May 22, 2015

dekuashe commented May 22, 2015

dekuashe commented May 22, 2015

sjudd commented May 25, 2015

dekuashe commented May 25, 2015

TWiStErRob commented May 25, 2015

dekuashe commented May 26, 2015

sjudd commented May 26, 2015

dekuashe commented May 26, 2015

dekuashe commented May 27, 2015

sjudd commented May 27, 2015

Inconsistent behavior with WebP images #392

Inconsistent behavior with WebP images #392

Comments

dekuashe commented Apr 3, 2015

dekuashe commented Apr 3, 2015

sjudd commented Apr 6, 2015

mufumbo commented Apr 8, 2015

dekuashe commented May 22, 2015

TWiStErRob commented May 22, 2015

sjudd commented May 22, 2015

dekuashe commented May 22, 2015

dekuashe commented May 22, 2015

sjudd commented May 25, 2015

dekuashe commented May 25, 2015

TWiStErRob commented May 25, 2015

dekuashe commented May 26, 2015

sjudd commented May 26, 2015

dekuashe commented May 26, 2015

dekuashe commented May 27, 2015

sjudd commented May 27, 2015