Force followRedirects on the OkHttpClient when needed (#1130)

OpenFeign · Dec 12, 2019 · 9e23599 · 9e23599
1 parent cca3887
commit 9e23599
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 3 deletions.
diff --git a/okhttp/src/main/java/feign/okhttp/OkHttpClient.java b/okhttp/src/main/java/feign/okhttp/OkHttpClient.java
@@ -154,7 +154,8 @@ public feign.Response execute(feign.Request input, feign.Request.Options options
       throws IOException {
     okhttp3.OkHttpClient requestScoped;
     if (delegate.connectTimeoutMillis() != options.connectTimeoutMillis()
-        || delegate.readTimeoutMillis() != options.readTimeoutMillis()) {
+        || delegate.readTimeoutMillis() != options.readTimeoutMillis()
+        || delegate.followRedirects() != options.isFollowRedirects()) {
       requestScoped = delegate.newBuilder()
           .connectTimeout(options.connectTimeoutMillis(), TimeUnit.MILLISECONDS)
           .readTimeout(options.readTimeoutMillis(), TimeUnit.MILLISECONDS)

diff --git a/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java b/okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java
@@ -23,6 +23,7 @@
 import feign.client.AbstractClientTest;
 import feign.Feign;
 import java.util.Collections;
+import java.util.concurrent.TimeUnit;
 import okhttp3.mockwebserver.MockResponse;
 import org.assertj.core.data.MapEntry;
 import org.junit.Test;
@@ -60,9 +61,14 @@ public void testContentTypeWithoutCharset() throws Exception {
   public void testNoFollowRedirect() throws Exception {
     server.enqueue(
         new MockResponse().setResponseCode(302).addHeader("Location", server.url("redirect")));
+    // Enqueue a response to fail fast if the redirect is followed, instead of waiting for the
+    // timeout
+    server.enqueue(new MockResponse().setBody("Hello"));
 
     OkHttpClientTestInterface api = newBuilder()
-        .options(new Request.Options(1000, 1000, false))
+        // Use the same connect and read timeouts as the OkHttp default
+        .options(new Request.Options(10_000, TimeUnit.MILLISECONDS, 10_000, TimeUnit.MILLISECONDS,
+            false))
         .target(OkHttpClientTestInterface.class, "http://localhost:" + server.getPort());
 
     Response response = api.get();
@@ -83,7 +89,9 @@ public void testFollowRedirect() throws Exception {
     server.enqueue(new MockResponse().setBody(expectedBody));
 
     OkHttpClientTestInterface api = newBuilder()
-        .options(new Request.Options(1000, 1000, true))
+        // Use the same connect and read timeouts as the OkHttp default
+        .options(new Request.Options(10_000, TimeUnit.MILLISECONDS, 10_000, TimeUnit.MILLISECONDS,
+            true))
         .target(OkHttpClientTestInterface.class, "http://localhost:" + server.getPort());
 
     Response response = api.get();