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

Handle WebP images without content length headers. #470

Closed
sjudd opened this issue May 25, 2015 · 6 comments
Closed

Handle WebP images without content length headers. #470

sjudd opened this issue May 25, 2015 · 6 comments

Comments

@sjudd
Copy link
Collaborator

sjudd commented May 25, 2015

If we don't get a valid content length, we could check if the image is a WebP image on a version of Android affected by the framework bug and read the contents into a byte array. Although it's a bit of a hack, it would be a more robust solution than just checking content length headers and it wouldn't sacrifice too much performance on platforms not affected by the framework bug.

We could also log an error when reading into a byte array indicating that we think there's a problem with the headers so that developers who are able to do so can fix the root cause.

@sjudd sjudd added this to the 4.0 milestone May 25, 2015
@dekuashe
Copy link

Just a note, if the content is gzipped (that is, the request contains Accept-Encoding: gzip and the response contains Content-Encoding: gzip), Content-Length isn't available and we're supposed to get the actual length by consuming the entire stream first, so that's one scenario where this could happen, even though images shouldn't be gzipped on a well-configured server anyway.

@sjudd
Copy link
Collaborator Author

sjudd commented May 27, 2015

So we also ought to be setting Accept-Encoding everywhere when downloading images, not just in the HttpUrlConnection fetcher.

@dekuashe
Copy link

Seems like that should cover it by itself?

@sjudd
Copy link
Collaborator Author

sjudd commented May 27, 2015

Yes, though it depends on how paranoid we want to be. If someone adds a new http library (in addition to HttpUrlConnection, OkHttp, or Volley), they may not add the correct headers. Or someone could always decide to try enabling gzip manually, which would replace our attempt to set the header.

Probably though you're right, the first thing to do is to add the headers everywhere (probably by making them defaults in LazyHeaders), then we can worry about other cases.

sjudd added a commit to sjudd/glide that referenced this issue Jun 24, 2015
sjudd added a commit to sjudd/glide that referenced this issue Jun 24, 2015
@sjudd sjudd closed this as completed in b8b3550 Jun 26, 2015
@mufumbo
Copy link

mufumbo commented Sep 30, 2015

@sjudd can you please tell me till what android sdk version glide supports webp now? is it well defined or the bugs are still randomly versioned?

@TWiStErRob
Copy link
Collaborator

As far as I know, WEBP was and is supported where Android SDK can load webp by itself. This issue is about servers who respond with gzipped webp content, which is usually pointless, because they're already compressed. The problem is that they also don't send a length, which is required by some Android versions, see #392 for more.

@TWiStErRob TWiStErRob added the bug label Oct 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants