diff --git a/okhttp/src/main/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt b/okhttp/src/main/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt index b939048a62af..35451796f1e9 100644 --- a/okhttp/src/main/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt +++ b/okhttp/src/main/kotlin/okhttp3/internal/http/RetryAndFollowUpInterceptor.kt @@ -222,16 +222,7 @@ class RetryAndFollowUpInterceptor(private val client: OkHttpClient) : Intercepto HTTP_UNAUTHORIZED -> return client.authenticator.authenticate(route, userResponse) - HTTP_PERM_REDIRECT, HTTP_TEMP_REDIRECT -> { - // "If the 307 or 308 status code is received in response to a request other than GET - // or HEAD, the user agent MUST NOT automatically redirect the request" - if (method != "GET" && method != "HEAD") { - return null - } - return buildRedirectRequest(userResponse, method) - } - - HTTP_MULT_CHOICE, HTTP_MOVED_PERM, HTTP_MOVED_TEMP, HTTP_SEE_OTHER -> { + HTTP_PERM_REDIRECT, HTTP_TEMP_REDIRECT, HTTP_MULT_CHOICE, HTTP_MOVED_PERM, HTTP_MOVED_TEMP, HTTP_SEE_OTHER -> { return buildRedirectRequest(userResponse, method) } @@ -312,8 +303,11 @@ class RetryAndFollowUpInterceptor(private val client: OkHttpClient) : Intercepto // Most redirects don't include a request body. val requestBuilder = userResponse.request.newBuilder() if (HttpMethod.permitsRequestBody(method)) { - val maintainBody = HttpMethod.redirectsWithBody(method) - if (HttpMethod.redirectsToGet(method)) { + val responseCode = userResponse.code + val maintainBody = HttpMethod.redirectsWithBody(method) || + responseCode == HTTP_PERM_REDIRECT || + responseCode == HTTP_TEMP_REDIRECT + if (HttpMethod.redirectsToGet(method) && responseCode != HTTP_PERM_REDIRECT && responseCode != HTTP_TEMP_REDIRECT) { requestBuilder.method("GET", null) } else { val requestBody = if (maintainBody) userResponse.request.body else null diff --git a/okhttp/src/test/java/okhttp3/URLConnectionTest.java b/okhttp/src/test/java/okhttp3/URLConnectionTest.java index 61f00db494fc..3e7d4a046417 100644 --- a/okhttp/src/test/java/okhttp3/URLConnectionTest.java +++ b/okhttp/src/test/java/okhttp3/URLConnectionTest.java @@ -86,6 +86,8 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; +import static java.net.HttpURLConnection.HTTP_MOVED_PERM; +import static java.net.HttpURLConnection.HTTP_MOVED_TEMP; import static java.nio.charset.StandardCharsets.UTF_8; import static java.util.Arrays.asList; import static java.util.Locale.US; @@ -2021,7 +2023,7 @@ private void testSecureStreamingPost(TransferKind streamingMode) throws Exceptio private void testRedirected(TransferKind transferKind, boolean reuse) throws Exception { MockResponse mockResponse = new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /foo"); transferKind.setBody(mockResponse, "This page has moved!", 10); server.enqueue(mockResponse); @@ -2045,7 +2047,7 @@ private void testRedirected(TransferKind transferKind, boolean reuse) throws Exc @Test public void redirectedOnHttps() throws Exception { server.useHttps(handshakeCertificates.sslSocketFactory(), false); server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /foo") .setBody("This page has moved!")); server.enqueue(new MockResponse() @@ -2071,7 +2073,7 @@ private void testRedirected(TransferKind transferKind, boolean reuse) throws Exc @Test public void notRedirectedFromHttpsToHttp() throws Exception { server.useHttps(handshakeCertificates.sslSocketFactory(), false); server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: http://anyhost/foo") .setBody("This page has moved!")); @@ -2088,7 +2090,7 @@ private void testRedirected(TransferKind transferKind, boolean reuse) throws Exc @Test public void notRedirectedFromHttpToHttps() throws Exception { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: https://anyhost/foo") .setBody("This page has moved!")); @@ -2106,7 +2108,7 @@ private void testRedirected(TransferKind transferKind, boolean reuse) throws Exc server.useHttps(handshakeCertificates.sslSocketFactory(), false); server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: " + server2.url("/").url()) .setBody("This page has moved!")); @@ -2127,7 +2129,7 @@ private void testRedirected(TransferKind transferKind, boolean reuse) throws Exc .setBody("This is secure HTTPS!")); server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: " + server2.url("/").url()) .setBody("This page has moved!")); @@ -2167,7 +2169,7 @@ private void redirectToAnotherOriginServer(boolean https) throws Exception { .setBody("This is the 2nd server, again!")); server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: " + server2.url("/").url().toString()) .setBody("This page has moved!")); server.enqueue(new MockResponse() @@ -2213,7 +2215,7 @@ private void redirectToAnotherOriginServer(boolean https) throws Exception { .setBody("This is the 2nd server!")); server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: " + server2.url("/b").toString()) .setBody("This page has moved!")); @@ -2253,7 +2255,7 @@ private void redirectToAnotherOriginServer(boolean https) throws Exception { } @Test public void response302MovedTemporarilyWithPost() throws Exception { - testResponseRedirectedWithPost(HttpURLConnection.HTTP_MOVED_TEMP, TransferKind.END_OF_STREAM); + testResponseRedirectedWithPost(HTTP_MOVED_TEMP, TransferKind.END_OF_STREAM); } @Test public void response303SeeOtherWithPost() throws Exception { @@ -2261,11 +2263,11 @@ private void redirectToAnotherOriginServer(boolean https) throws Exception { } @Test public void postRedirectToGetWithChunkedRequest() throws Exception { - testResponseRedirectedWithPost(HttpURLConnection.HTTP_MOVED_TEMP, TransferKind.CHUNKED); + testResponseRedirectedWithPost(HTTP_MOVED_TEMP, TransferKind.CHUNKED); } @Test public void postRedirectToGetWithStreamedRequest() throws Exception { - testResponseRedirectedWithPost(HttpURLConnection.HTTP_MOVED_TEMP, TransferKind.FIXED_LENGTH); + testResponseRedirectedWithPost(HTTP_MOVED_TEMP, TransferKind.FIXED_LENGTH); } private void testResponseRedirectedWithPost(int redirectCode, TransferKind transferKind) @@ -2294,7 +2296,7 @@ private void testResponseRedirectedWithPost(int redirectCode, TransferKind trans @Test public void redirectedPostStripsRequestBodyHeaders() throws Exception { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /page2")); server.enqueue(new MockResponse() .setBody("Page 2")); @@ -2367,6 +2369,71 @@ private void testResponseRedirectedWithPost(int redirectCode, TransferKind trans testRedirect(false, "POST"); } + class RevertInterceptor implements Interceptor { + @Override public Response intercept(Chain chain) throws IOException { + Response response = chain.proceed(chain.request()); + + return remapResponse(response); + } + + private Response remapResponse(Response response) { + if (response.request().method().equals("POST") && (response.code() == HTTP_TEMP_REDIRECT || response.code() == HTTP_PERM_REDIRECT)) { + // special response code to indicate custom rules + return response.newBuilder().code(response.code() + 10).build(); + } + + return response; + } + } + + @Test public void response307WithPostReverted() throws Exception { + client = client.newBuilder().addNetworkInterceptor(new RevertInterceptor()).build(); + + MockResponse response1 = new MockResponse() + .setResponseCode(HTTP_TEMP_REDIRECT) + .setBody("This page has moved!") + .addHeader("Location: /page2"); + server.enqueue(response1); + + Request.Builder requestBuilder = new Request.Builder() + .url(server.url("/page1")); + requestBuilder.post(RequestBody.create("ABCD", null)); + + Response response = getResponse(requestBuilder.build()); + String responseString = readAscii(response.body().byteStream(), Integer.MAX_VALUE); + + RecordedRequest page1 = server.takeRequest(); + assertThat(page1.getRequestLine()).isEqualTo(("POST /page1 HTTP/1.1")); + + assertThat(page1.getBody().readUtf8()).isEqualTo("ABCD"); + assertThat(server.getRequestCount()).isEqualTo(1); + assertThat(responseString).isEqualTo("This page has moved!"); + } + + @Test public void response308WithPostReverted() throws Exception { + client = client.newBuilder().addNetworkInterceptor(new RevertInterceptor()).build(); + + MockResponse response1 = new MockResponse() + .setResponseCode(HTTP_PERM_REDIRECT) + .setBody("This page has moved!") + .addHeader("Location: /page2"); + server.enqueue(response1); + + Request.Builder requestBuilder = new Request.Builder() + .url(server.url("/page1")); + requestBuilder.post(RequestBody.create("ABCD", null)); + + Response response = getResponse(requestBuilder.build()); + String responseString = readAscii(response.body().byteStream(), Integer.MAX_VALUE); + + RecordedRequest page1 = server.takeRequest(); + assertThat(page1.getRequestLine()).isEqualTo(("POST /page1 HTTP/1.1")); + + assertThat(page1.getBody().readUtf8()).isEqualTo("ABCD"); + assertThat(server.getRequestCount()).isEqualTo(1); + assertThat(responseString).isEqualTo("This page has moved!"); + } + private void testRedirect(boolean temporary, String method) throws Exception { MockResponse response1 = new MockResponse() .setResponseCode(temporary ? HTTP_TEMP_REDIRECT : HTTP_PERM_REDIRECT) @@ -2396,17 +2463,8 @@ private void testRedirect(boolean temporary, String method) throws Exception { assertThat(responseString).isEqualTo("Page 2"); } else if (method.equals("HEAD")) { assertThat(responseString).isEqualTo(""); - } else { - // Methods other than GET/HEAD shouldn't follow the redirect. - if (method.equals("POST")) { - assertThat(page1.getBody().readUtf8()).isEqualTo("ABCD"); - } - assertThat(server.getRequestCount()).isEqualTo(1); - assertThat(responseString).isEqualTo("This page has moved!"); - return; } - // GET/HEAD requests should have followed the redirect with the same method. assertThat(server.getRequestCount()).isEqualTo(2); RecordedRequest page2 = server.takeRequest(); assertThat(page2.getRequestLine()).isEqualTo((method + " /page2 HTTP/1.1")); @@ -2415,7 +2473,7 @@ private void testRedirect(boolean temporary, String method) throws Exception { @Test public void follow20Redirects() throws Exception { for (int i = 0; i < 20; i++) { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /" + (i + 1)) .setBody("Redirecting to /" + (i + 1))); } @@ -2430,7 +2488,7 @@ private void testRedirect(boolean temporary, String method) throws Exception { @Test public void doesNotFollow21Redirects() throws Exception { for (int i = 0; i < 21; i++) { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /" + (i + 1)) .setBody("Redirecting to /" + (i + 1))); } @@ -2653,7 +2711,7 @@ protected ServerSocket configureServerSocket(ServerSocket serverSocket) @Test public void connectionCloseWithRedirect() throws Exception { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /foo") .addHeader("Connection: close")); server.enqueue(new MockResponse() @@ -2675,7 +2733,7 @@ protected ServerSocket configureServerSocket(ServerSocket serverSocket) */ @Test public void sameConnectionRedirectAndReuse() throws Exception { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .setSocketPolicy(SHUTDOWN_INPUT_AT_END) .addHeader("Location: /foo")); server.enqueue(new MockResponse() @@ -3506,7 +3564,7 @@ private void zeroLengthPayload(String method) throws Exception { */ @Test public void gzipWithRedirectAndConnectionReuse() throws Exception { server.enqueue(new MockResponse() - .setResponseCode(HttpURLConnection.HTTP_MOVED_TEMP) + .setResponseCode(HTTP_MOVED_TEMP) .addHeader("Location: /foo") .addHeader("Content-Encoding: gzip") .setBody(gzip("Moved! Moved! Moved!")));