Handle redirects in default url fetcher.

integration/volley/build.gradle

 apply plugin: 'com.android.library'
+apply plugin: 'robolectric'
 
 repositories {
   mavenCentral()
@@ -6,8 +7,15 @@ repositories {
 
 dependencies {
     compile project(':glide')
-
     compile 'com.mcxiaoke.volley:library:1.0.+'
+
+    androidTestCompile 'org.hamcrest:hamcrest-core:1.3'
+    androidTestCompile 'org.hamcrest:hamcrest-library:1.3'
+    androidTestCompile 'junit:junit:4.11'
+    androidTestCompile 'org.mockito:mockito-all:1.9.5'
+    androidTestCompile 'org.robolectric:robolectric:2.4-SNAPSHOT'
+    // TODO: increase this to 2.0.+ when we compile against Java 7.
+    androidTestCompile 'com.squareup.okhttp:mockwebserver:1.2.+'
 }
 
 android {

integration/volley/src/androidTest/java/com/bumptech/glide/integration/volley/VolleyStreamFetcherServerTest.java

+package com.bumptech.glide.integration.volley;
+
+import android.os.SystemClock;
+import com.android.volley.NoConnectionError;
+import com.android.volley.RequestQueue;
+import com.android.volley.ServerError;
+import com.android.volley.VolleyError;
+import com.android.volley.toolbox.Volley;
+import com.bumptech.glide.Priority;
+import com.bumptech.glide.load.data.DataFetcher;
+import com.squareup.okhttp.mockwebserver.MockResponse;
+import com.squareup.okhttp.mockwebserver.MockWebServer;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.Robolectric;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.annotation.Config;
+import org.robolectric.annotation.Implementation;
+import org.robolectric.annotation.Implements;
+import org.robolectric.shadows.ShadowSystemClock;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.ProtocolException;
+import java.net.URL;
+import java.util.concurrent.ExecutionException;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.Assert.fail;
+
+/**
+ * Tests {@link com.bumptech.glide.integration.volley.VolleyStreamFetcher} against server responses.
+ */
+@RunWith(RobolectricTestRunner.class)
+@Config(manifest = Config.NONE, emulateSdk = 18, shadows = VolleyStreamFetcherServerTest.FakeSystemClock.class)
+public class VolleyStreamFetcherServerTest {
+    private static final String DEFAULT_PATH = "/fakepath";
+
+    private MockWebServer mockWebServer;
+    private RequestQueue requestQueue;
+
+    @Before
+    public void setUp() throws IOException {
+        requestQueue = Volley.newRequestQueue(Robolectric.application);
+        mockWebServer = new MockWebServer();
+        mockWebServer.play();
+    }
+
+    @After
+    public void tearDown() throws IOException {
+        mockWebServer.shutdown();
+        requestQueue.stop();
+    }
+
+    @Test
+    public void testReturnsInputStreamOnStatusOk() throws Exception {
+        String expected = "fakedata";
+        mockWebServer.enqueue(new MockResponse()
+                .setBody(expected)
+                .setResponseCode(200));
+        DataFetcher<InputStream> fetcher = getFetcher();
+        InputStream is = fetcher.loadData(Priority.HIGH);
+        assertThat(isToString(is), equalTo(expected));
+    }
+
+    @Test
+    public void testHandlesRedirect301s() throws Exception {
+        String expected = "fakedata";
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(301)
+            .setHeader("Location", mockWebServer.getUrl("/redirect")));
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(200)
+            .setBody(expected));
+        InputStream is = getFetcher().loadData(Priority.LOW);
+        assertThat(isToString(is), equalTo(expected));
+    }
+
+    @Test
+    public void testHandlesRedirect302s() throws Exception {
+        String expected = "fakedata";
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(302)
+            .setHeader("Location", mockWebServer.getUrl("/redirect")));
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(200)
+            .setBody(expected));
+        InputStream is = getFetcher().loadData(Priority.LOW);
+        assertThat(isToString(is), equalTo(expected));
+    }
+
+    @Test
+    public void testHandlesUpToFiveRedirects() throws Exception {
+        int numRedirects = 4;
+        String expected = "redirectedData";
+        String redirectBase = "/redirect";
+        for (int i = 0; i < numRedirects; i++) {
+            mockWebServer.enqueue(new MockResponse()
+                    .setResponseCode(301)
+                    .setHeader("Location", mockWebServer.getUrl(redirectBase + i)));
+        }
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(200).setBody(expected));
+
+        InputStream is = getFetcher().loadData(Priority.NORMAL);
+        assertThat(isToString(is), equalTo(expected));
+
+        assertThat(mockWebServer.takeRequest().getPath(), containsString(DEFAULT_PATH));
+        for (int i = 0; i < numRedirects; i++) {
+            assertThat(mockWebServer.takeRequest().getPath(), containsString(redirectBase + i));
+        }
+    }
+
+    @Test
+    public void testThrowsIfRedirectLocationIsEmpty() throws Exception {
+        for (int i = 0; i < 2; i++) {
+            mockWebServer.enqueue(new MockResponse().setResponseCode(301));
+        }
+
+        try {
+            getFetcher().loadData(Priority.NORMAL);
+            fail("Didn't get expected IOException");
+        } catch (ExecutionException e) {
+            assertThat(e.getCause(), instanceOf(VolleyError.class));
+        }
+    }
+
+    @Test
+    public void testThrowsIfStatusCodeIsNegativeOne() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(-1));
+        try {
+            getFetcher().loadData(Priority.LOW);
+            fail("Failed to get expected exception");
+        } catch (ExecutionException e) {
+            assertThat(e.getCause(), instanceOf(NoConnectionError.class));
+        }
+    }
+
+    @Test
+    public void testThrowsAfterTooManyRedirects() throws Exception {
+        for (int i = 0; i < 20; i++) {
+            mockWebServer.enqueue(new MockResponse()
+                    .setResponseCode(301)
+                    .setHeader("Location", mockWebServer.getUrl("/redirect" + i)));
+        }
+        try {
+            getFetcher().loadData(Priority.NORMAL);
+            fail("Failed to get expected exception");
+        } catch (ExecutionException e) {
+            assertThat(e.getCause(), instanceOf(NoConnectionError.class));
+            assertThat(e.getCause().getCause(), instanceOf(ProtocolException.class));
+        }
+    }
+
+
+    @Test
+    public void testThrowsIfStatusCodeIs500() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(500).setBody("error"));
+        try {
+            getFetcher().loadData(Priority.NORMAL);
+            fail("Failed to get expected exception");
+        } catch (ExecutionException e) {
+            assertThat(e.getCause(), instanceOf(ServerError.class));
+        }
+    }
+
+    @Test
+    public void testThrowsIfStatusCodeIs400() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(400).setBody("error"));
+        try {
+            getFetcher().loadData(Priority.LOW);
+            fail("Failed to get expected exception");
+        } catch (ExecutionException e) {
+            assertThat(e.getCause(), instanceOf(ServerError.class));
+        }
+    }
+
+    private DataFetcher<InputStream> getFetcher() {
+        URL url = mockWebServer.getUrl(DEFAULT_PATH);
+        VolleyRequestFuture<InputStream> requestFuture = new VolleyRequestFuture<InputStream>() {
+            @Override
+            public InputStream get() throws InterruptedException, ExecutionException {
+                for (int i = 0; i < 251 && !isDone(); i++) {
+                    Thread.sleep(10);
+                    Robolectric.runUiThreadTasks();
+                }
+                if (!isDone()) {
+                    fail("Failed to get response from Volley in time");
+                }
+                return super.get();
+            }
+        };
+        return new VolleyStreamFetcher(requestQueue, url.toString(), requestFuture);
+    }
+
+    private static String isToString(InputStream is) throws IOException {
+        ByteArrayOutputStream os = new ByteArrayOutputStream();
+        byte[] buffer = new byte[1024];
+        int read;
+        while ((read = is.read(buffer)) != -1) {
+            os.write(buffer, 0, read);
+        }
+        return new String(os.toByteArray());
+    }
+
+    /** A shadow clock that doesn't rely on running on an Android thread with a Looper. */
+    @Implements(SystemClock.class)
+    public static class FakeSystemClock extends ShadowSystemClock {
+
+        @Implementation
+        public static long elapsedRealtime() {
+            // The default is to return something using the main looper, which doesn't exist on Volley's threads.
+            return System.currentTimeMillis();
+        }
+    }
+}

library/build.gradle

@@ -6,11 +6,14 @@ dependencies {
     compile project(':third_party:gif_decoder')
     compile project(':third_party:disklrucache')
     compile 'com.android.support:support-v4:19.1.+'
+
     androidTestCompile 'org.hamcrest:hamcrest-core:1.3'
     androidTestCompile 'org.hamcrest:hamcrest-library:1.3'
     androidTestCompile 'junit:junit:4.11'
     androidTestCompile 'org.mockito:mockito-all:1.9.5'
     androidTestCompile 'org.robolectric:robolectric:2.4-SNAPSHOT'
+    // TODO: increase this to 2.0.+ when we compile against Java 7.
+    androidTestCompile 'com.squareup.okhttp:mockwebserver:1.2.+'
 }
 
 android {

library/src/androidTest/java/com/bumptech/glide/load/data/HttpUrlFetcherServerTest.java

+package com.bumptech.glide.load.data;
+
+import com.bumptech.glide.Priority;
+import com.bumptech.glide.load.model.GlideUrl;
+import com.squareup.okhttp.mockwebserver.MockResponse;
+import com.squareup.okhttp.mockwebserver.MockWebServer;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.HttpURLConnection;
+import java.net.MalformedURLException;
+import java.net.SocketTimeoutException;
+import java.net.URL;
+import java.util.concurrent.TimeUnit;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.equalTo;
+import static org.junit.Assert.fail;
+
+/**
+ * Tests {@link com.bumptech.glide.load.data.HttpUrlFetcher} against server responses. Tests for behavior
+ * (connection/disconnection/options) should go in {@link com.bumptech.glide.load.data.HttpUrlFetcherTest}, response
+ * handling should go here.
+ */
+@RunWith(RobolectricTestRunner.class)
+public class HttpUrlFetcherServerTest {
+    private static final String DEFAULT_PATH = "/fakepath";
+
+    private MockWebServer mockWebServer;
+    private boolean defaultFollowRedirects;
+
+    @Before
+    public void setUp() throws IOException {
+        defaultFollowRedirects = HttpURLConnection.getFollowRedirects();
+        HttpURLConnection.setFollowRedirects(false);
+        mockWebServer = new MockWebServer();
+        mockWebServer.play();
+    }
+
+    @After
+    public void tearDown() throws IOException {
+        HttpURLConnection.setFollowRedirects(defaultFollowRedirects);
+        mockWebServer.shutdown();
+    }
+
+    @Test
+    public void testReturnsInputStreamOnStatusOk() throws Exception {
+        String expected = "fakedata";
+        mockWebServer.enqueue(new MockResponse()
+                .setBody(expected)
+                .setResponseCode(200));
+        HttpUrlFetcher fetcher = getFetcher();
+        InputStream is = fetcher.loadData(Priority.HIGH);
+        assertThat(isToString(is), equalTo(expected));
+    }
+
+    @Test
+    public void testHandlesRedirect301s() throws Exception {
+        String expected = "fakedata";
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(301)
+            .setHeader("Location", mockWebServer.getUrl("/redirect")));
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(200)
+            .setBody(expected));
+        InputStream is = getFetcher().loadData(Priority.LOW);
+        assertThat(isToString(is), equalTo(expected));
+    }
+
+    @Test
+    public void testHandlesRedirect302s() throws Exception {
+        String expected = "fakedata";
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(302)
+            .setHeader("Location", mockWebServer.getUrl("/redirect")));
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(200)
+            .setBody(expected));
+        InputStream is = getFetcher().loadData(Priority.LOW);
+        assertThat(isToString(is), equalTo(expected));
+    }
+
+    @Test
+    public void testHandlesUpToFiveRedirects() throws Exception {
+        int numRedirects = 4;
+        String expected = "redirectedData";
+        String redirectBase = "/redirect";
+        for (int i = 0; i < numRedirects; i++) {
+            mockWebServer.enqueue(new MockResponse()
+                    .setResponseCode(301)
+                    .setHeader("Location", mockWebServer.getUrl(redirectBase + i)));
+        }
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(200).setBody(expected));
+
+        InputStream is = getFetcher().loadData(Priority.NORMAL);
+        assertThat(isToString(is), equalTo(expected));
+
+        assertThat(mockWebServer.takeRequest().getPath(), containsString(DEFAULT_PATH));
+        for (int i = 0; i < numRedirects; i++) {
+            assertThat(mockWebServer.takeRequest().getPath(), containsString(redirectBase + i));
+        }
+    }
+
+    @Test
+    public void testThrowsOnRedirectLoops() throws Exception {
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(301).setHeader("Location", mockWebServer.getUrl("/redirect")));
+        mockWebServer.enqueue(new MockResponse()
+            .setResponseCode(301).setHeader("Location", mockWebServer.getUrl("/redirect")));
+
+        try {
+            getFetcher().loadData(Priority.IMMEDIATE);
+            fail("Didn't get expected IOException");
+        } catch (SocketTimeoutException e) {
+            fail("Didn't expect SocketTimeoutException");
+        } catch (IOException e) {
+            // Expected.
+        }
+    }
+
+    @Test
+    public void testThrowsIfRedirectLocationIsEmpty() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(301));
+
+        try {
+            getFetcher().loadData(Priority.NORMAL);
+            fail("Didn't get expected IOException");
+        } catch (MalformedURLException e) {
+            // Expected
+        }
+    }
+
+    @Test(expected = IOException.class)
+    public void testThrowsIfStatusCodeIsNegativeOne() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(-1));
+        getFetcher().loadData(Priority.LOW);
+    }
+
+    @Test(expected = IOException.class)
+    public void testThrowsAfterTooManyRedirects() throws Exception {
+        for (int i = 0; i < 10; i++) {
+            mockWebServer.enqueue(new MockResponse()
+                    .setResponseCode(301)
+                    .setHeader("Location", mockWebServer.getUrl("/redirect" + i)));
+        }
+        getFetcher().loadData(Priority.NORMAL);
+    }
+
+
+    @Test(expected = IOException.class)
+    public void testThrowsIfStatusCodeIs500() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(500));
+        getFetcher().loadData(Priority.NORMAL);
+    }
+
+    @Test(expected = IOException.class)
+    public void testThrowsIfStatusCodeIs400() throws Exception {
+        mockWebServer.enqueue(new MockResponse().setResponseCode(400));
+        getFetcher().loadData(Priority.LOW);
+    }
+
+    @Test(expected = SocketTimeoutException.class)
+    public void testSetsReadTimeout() throws Exception {
+        MockWebServer mockWebServer = new MockWebServer();
+        mockWebServer.enqueue(new MockResponse().setBody("test").throttleBody(1, 2501, TimeUnit.SECONDS));
+        mockWebServer.play();
+        try {
+            getFetcher().loadData(Priority.HIGH);
+        } finally {
+            mockWebServer.shutdown();
+        }
+    }
+
+    private HttpUrlFetcher getFetcher() {
+        URL url = mockWebServer.getUrl(DEFAULT_PATH);
+        return new HttpUrlFetcher(new GlideUrl(url));
+    }
+
+    private static String isToString(InputStream is) throws IOException {
+        ByteArrayOutputStream os = new ByteArrayOutputStream();
+        byte[] buffer = new byte[1024];
+        int read;
+        while ((read = is.read(buffer)) != -1) {
+            os.write(buffer, 0, read);
+        }
+        return new String(os.toByteArray());
+    }
+}

library/src/androidTest/java/com/bumptech/glide/load/data/HttpUrlFetcherTest.java

@@ -44,7 +44,6 @@ public class HttpUrlFetcherTest {
         assertEquals(expected, fetcher.getId());
     }
 
-
     @Test
     public void testSetsReadTimeout() throws Exception {
         fetcher.loadData(Priority.HIGH);
@@ -57,33 +56,6 @@ public class HttpUrlFetcherTest {
         verify(urlConnection).setConnectTimeout(eq(2500));
     }
 
-    @Test
-    public void testReturnsInputStreamOnStatusOk() throws Exception {
-        InputStream expected = new ByteArrayInputStream(new byte[0]);
-        when(urlConnection.getResponseCode()).thenReturn(200);
-        when(urlConnection.getInputStream()).thenReturn(expected);
-
-        assertEquals(expected, fetcher.loadData(Priority.NORMAL));
-    }
-
-    @Test(expected = IOException.class)
-    public void testThrowsIfStatusCodeIsNegativeOne() throws Exception {
-        when(urlConnection.getResponseCode()).thenReturn(-1);
-        fetcher.loadData(Priority.HIGH);
-    }
-
-    @Test(expected = IOException.class)
-    public void testThrowsIfStatusCodeIs300() throws Exception {
-        when(urlConnection.getResponseCode()).thenReturn(300);
-        fetcher.loadData(Priority.HIGH);
-    }
-
-    @Test(expected = IOException.class)
-    public void testThrowsIfStatusCodeIs500() throws Exception {
-        when(urlConnection.getResponseCode()).thenReturn(500);
-        fetcher.loadData(Priority.HIGH);
-    }
-
     @Test
     public void testReturnsNullIfCancelledBeforeConnects() throws Exception {
         InputStream notExpected = new ByteArrayInputStream(new byte[0]);

library/src/main/java/com/bumptech/glide/load/data/HttpUrlFetcher.java

@@ -12,9 +12,11 @@ import java.net.URL;
  * A DataFetcher that retrieves an {@link java.io.InputStream} for a Url.
  */
 public class HttpUrlFetcher implements DataFetcher<InputStream> {
+    private static final int MAXIMUM_REDIRECTS = 5;
     private static final HttpUrlConnectionFactory DEFAULT_FACTORY = new DefaultHttpUrlConnectionFactory();
-    private GlideUrl glideUrl;
+
     private final HttpUrlConnectionFactory factory;
+    private final GlideUrl glideUrl;
     private HttpURLConnection urlConnection;
     private volatile boolean isCancelled;
 
@@ -29,7 +31,16 @@ public class HttpUrlFetcher implements DataFetcher<InputStream> {
 
     @Override
     public InputStream loadData(Priority priority) throws Exception {
-        urlConnection = factory.build(glideUrl.toURL());
+        return loadDataWithRedirects(glideUrl.toURL(), 0 /*redirects*/, null /*lastUrl*/);
+    }
+
+    private InputStream loadDataWithRedirects(URL url, int redirects, URL lastUrl) throws IOException {
+        if (redirects >= MAXIMUM_REDIRECTS) {
+            throw new IOException("Too many (> " + MAXIMUM_REDIRECTS + ") redirects!");
+        } else if (url.equals(lastUrl)) {
+            throw new IOException("In re-direct loop");
+        }
+        urlConnection = factory.build(url);
         urlConnection.setConnectTimeout(2500);
         urlConnection.setReadTimeout(2500);
         urlConnection.setUseCaches(false);
@@ -43,6 +54,10 @@ public class HttpUrlFetcher implements DataFetcher<InputStream> {
         final int statusCode = urlConnection.getResponseCode();
         if (statusCode / 100 == 2) {
             return urlConnection.getInputStream();
+        } else if (statusCode / 100 == 3) {
+            String redirectUrlString = urlConnection.getHeaderField("Location");
+            URL redirectUrl = new URL(redirectUrlString);
+            return loadDataWithRedirects(redirectUrl, redirects + 1, url);
         } else {
             if (statusCode == -1) {
                 throw new IOException("Unable to retrieve response code from HttpUrlConnection.");
@@ -65,8 +80,8 @@ public class HttpUrlFetcher implements DataFetcher<InputStream> {
 
     @Override
     public void cancel() {
-        // TODO: we should consider disconnecting the url connection here, but we can't do so directly because it
-        // would cause a strict mode violation.
+        // TODO: we should consider disconnecting the url connection here, but we can't do so directly because cancel is
+        // often called on the main thread.
         isCancelled = true;
     }