From 49082196be780486c97010533d9a1206f999e240 Mon Sep 17 00:00:00 2001 From: jwilson Date: Thu, 31 Jan 2013 00:36:10 -0500 Subject: [PATCH] Follow up to 20 redirects. --- .../internal/http/HttpURLConnectionImpl.java | 8 ++--- .../internal/http/URLConnectionTest.java | 36 +++++++++++++++++++ 2 files changed, 40 insertions(+), 4 deletions(-) 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 a03ef6965..834eceba9 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; @@ -415,7 +415,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 d91a2432e..a1fad1ff7 100644 --- a/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java +++ b/src/test/java/com/squareup/okhttp/internal/http/URLConnectionTest.java @@ -1678,6 +1678,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();