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.

Sign up for GitHub

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

Jump to bottom

OkHttpStreamFetcher.loadData should check that response code is successful #315

Closed
bubbleguuum opened this issue Jan 26, 2015 · 2 comments
Closed

OkHttpStreamFetcher.loadData should check that response code is successful #315

bubbleguuum opened this issue Jan 26, 2015 · 2 comments
Labels
enhancement
Milestone
3.6.0

Comments

@bubbleguuum
Copy link

If the http response is not successful (http code not beetween 200 and 300), there's no need to process it further.

Currently, all http responses with an unsuccessful response code and a body are processed, which is unecessary. Some http servers return some descriptive error text in the response body and Glide attempt to decode it as bitmap, which of course will fail (skia error in the logcat).

I suggest making this modification:

  @Override
    public InputStream loadData(Priority priority) throws Exception {

        Request.Builder builder = new Request.Builder().url(url.toString());
        if(userAgent != null) {
            builder.header("User-Agent", userAgent);
        }

        request = builder.build();

        Response response = client.newCall(request).execute();
        if(!response.isSuccessful()) throw new IOException("request failed with code: " + response.code());

        return response.body().byteStream();
    }

This URL can be used for testing (always returns a 503 with an html error body):

http://images.tidalhifi.com/im/im?w=160&h=107&artistid=4920567
@bubbleguuum
Copy link
Author

Additionally, loadData() could abort processing if there's a Content-Type header in the response whose value is not one of the supported image type by Glide.

@sjudd
Copy link
Collaborator

sjudd commented Jan 26, 2015

Thanks for pointing this out, I'd be happy to see a pull request for this.

@sjudd sjudd modified the milestone: 3.5 Jan 26, 2015
@sjudd sjudd added this to the 3.6.0 milestone Apr 6, 2015
@sjudd sjudd closed this as completed in 52c7e92 Apr 6, 2015
sjudd added a commit to sjudd/glide that referenced this issue Apr 6, 2015
@sjudd
Check result status in OkHttpStreamFetcher
976ec37 
Fixes bumptech#315.
Conflicts:
	integration/okhttp/src/main/java/com/bumptech/glide/integration/okhttp/OkHttpStreamFetcher.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
3.6.0
Development

No branches or pull requests
2 participants
@bubbleguuum @sjudd