diff --git a/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java b/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java index 48ca592fb..f52857d93 100644 --- a/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java +++ b/src/main/java/com/squareup/okhttp/internal/http/HttpURLConnectionImpl.java @@ -57,10 +57,10 @@ import javax.net.ssl.SSLHandshakeException; */ public class HttpURLConnectionImpl extends HttpURLConnection { /** - * HTTP 1.1 doesn't specify how many redirects to follow, but HTTP/1.0 - * recommended 5. http://www.w3.org/Protocols/HTTP/1.0/spec.html#Code3xx + * How many redirects should we follow? Chrome follows 21; Firefox, curl, + * and wget follow 20; Safari follows 16; and HTTP/1.0 recommends 5. */ - private static final int MAX_REDIRECTS = 5; + private static final int MAX_REDIRECTS = 20; private final int defaultPort; @@ -418,7 +418,7 @@ public class HttpURLConnectionImpl extends HttpURLConnection { return Retry.NONE; } if (++redirectionCount > MAX_REDIRECTS) { - throw new ProtocolException("Too many redirects"); + throw new ProtocolException("Too many redirects: " + redirectionCount); } String location = getHeaderField("Location"); if (location == null) { diff --git a/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java b/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java index 3a19b5ce9..6e41b9da5 100644 --- a/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java @@ -1682,6 +1682,42 @@ public final class URLConnectionTest { assertEquals(1, server.getRequestCount()); } + @Test public void follow20Redirects() throws Exception { + for (int i = 0; i < 20; i++) { + server.enqueue(new MockResponse() + .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .addHeader("Location: /" + (i + 1)) + .setBody("Redirecting to /" + (i + 1))); + } + server.enqueue(new MockResponse().setBody("Success!")); + server.play(); + + HttpURLConnection connection = client.open(server.getUrl("/0")); + assertContent("Success!", connection); + assertEquals(server.getUrl("/20"), connection.getURL()); + } + + @Test public void doesNotFollow21Redirects() throws Exception { + for (int i = 0; i < 21; i++) { + server.enqueue(new MockResponse() + .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .addHeader("Location: /" + (i + 1)) + .setBody("Redirecting to /" + (i + 1))); + } + server.play(); + + HttpURLConnection connection = client.open(server.getUrl("/0")); + try { + connection.getInputStream(); + fail(); + } catch (ProtocolException expected) { + assertEquals(HttpURLConnection.HTTP_MOVED_TEMP, connection.getResponseCode()); + assertEquals("Too many redirects: 21", expected.getMessage()); + assertContent("Redirecting to /21", connection); + assertEquals(server.getUrl("/20"), connection.getURL()); + } + } + @Test public void httpsWithCustomTrustManager() throws Exception { RecordingHostnameVerifier hostnameVerifier = new RecordingHostnameVerifier(); RecordingTrustManager trustManager = new RecordingTrustManager();