Skip to content

Commit c2fcbed

Browse files
Gui13velo
Gui13
authored and
velo
committed
Add an option to not follow redirects (302) and add a unit test for that (OpenFeign#602)
* Add an option to not follow redirects (302) and add a unit test for that * Implement followRedirect options for Ribbon Client and OkHTTP. Add unit tests for these. * Fix last failing unit test with IClientConfig options handling
1 parent 524c0d9 commit c2fcbed

File tree

8 files changed

+157
-9
lines changed

8 files changed

+157
-9
lines changed

‎core/src/main/java/feign/Client.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ HttpURLConnection convertAndSend(Request request, Options options) throws IOExce
8888
connection.setConnectTimeout(options.connectTimeoutMillis());
8989
connection.setReadTimeout(options.readTimeoutMillis());
9090
connection.setAllowUserInteraction(false);
91-
connection.setInstanceFollowRedirects(true);
91+
connection.setInstanceFollowRedirects(options.isFollowRedirects());
9292
connection.setRequestMethod(request.method());
9393

9494
Collection<String> contentEncodingValues = request.headers().get(CONTENT_ENCODING);

‎core/src/main/java/feign/Request.java‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
*/
1414
package feign;
1515

16+
import java.net.HttpURLConnection;
1617
import java.nio.charset.Charset;
1718
import java.util.Collection;
1819
import java.util.Map;
@@ -103,10 +104,16 @@ public static class Options {
103104

104105
private final int connectTimeoutMillis;
105106
private final int readTimeoutMillis;
107+
private final boolean followRedirects;
106108

107-
public Options(int connectTimeoutMillis, int readTimeoutMillis) {
109+
public Options(int connectTimeoutMillis, int readTimeoutMillis, boolean followRedirects) {
108110
this.connectTimeoutMillis = connectTimeoutMillis;
109111
this.readTimeoutMillis = readTimeoutMillis;
112+
this.followRedirects = followRedirects;
113+
}
114+
115+
public Options(int connectTimeoutMillis, int readTimeoutMillis){
116+
this(connectTimeoutMillis, readTimeoutMillis, true);
110117
}
111118

112119
public Options() {
@@ -130,5 +137,15 @@ public int connectTimeoutMillis() {
130137
public int readTimeoutMillis() {
131138
return readTimeoutMillis;
132139
}
140+
141+
142+
/**
143+
* Defaults to true. {@code false} tells the client to not follow the redirections.
144+
*
145+
* @see HttpURLConnection#getFollowRedirects()
146+
*/
147+
public boolean isFollowRedirects() {
148+
return followRedirects;
149+
}
133150
}
134151
}

‎core/src/test/java/feign/FeignBuilderTest.java‎

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import java.lang.reflect.Method;
2525
import java.lang.reflect.Type;
2626
import java.util.Arrays;
27+
import java.util.Collections;
2728
import java.util.Iterator;
2829
import java.util.List;
2930
import java.util.Map;
@@ -79,6 +80,31 @@ public void testDecode404() throws Exception {
7980
}
8081
}
8182

83+
84+
85+
@Test public void testNoFollowRedirect() {
86+
server.enqueue(new MockResponse().setResponseCode(302).addHeader("Location","/"));
87+
88+
String url = "http://localhost:" + server.getPort();
89+
TestInterface noFollowApi = Feign.builder()
90+
.options(new Request.Options(100, 600, false))
91+
.target(TestInterface.class, url);
92+
93+
Response response = noFollowApi.defaultMethodPassthrough();
94+
assertThat(response.status()).isEqualTo(302);
95+
assertThat(response.headers().getOrDefault("Location", null))
96+
.isNotNull()
97+
.isEqualTo(Collections.singletonList("/"));
98+
99+
server.enqueue(new MockResponse().setResponseCode(302).addHeader("Location","/"));
100+
server.enqueue(new MockResponse().setResponseCode(200));
101+
TestInterface defaultApi = Feign.builder()
102+
.options(new Request.Options(100, 600, true))
103+
.target(TestInterface.class, url);
104+
assertThat(defaultApi.defaultMethodPassthrough().status()).isEqualTo(200);
105+
}
106+
107+
82108
@Test
83109
public void testUrlPathConcatUrlTrailingSlash() throws Exception {
84110
server.enqueue(new MockResponse().setBody("response data"));
@@ -208,7 +234,7 @@ public InvocationHandler create(Target target, Map<Method, MethodHandler> dispat
208234
assertThat(server.takeRequest())
209235
.hasBody("request data");
210236
}
211-
237+
212238
@Test
213239
public void testSlashIsEncodedInPathParams() throws Exception {
214240
server.enqueue(new MockResponse().setBody("response data"));
@@ -318,7 +344,7 @@ interface TestInterface {
318344

319345
@RequestLine("POST /")
320346
Iterator<String> decodedLazyPost();
321-
347+
322348
@RequestLine(value = "GET /api/queues/{vhost}", decodeSlash = false)
323349
byte[] getQueues(@Param("vhost") String vhost);
324350

‎okhttp/src/main/java/feign/okhttp/OkHttpClient.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,7 @@ public feign.Response execute(feign.Request input, feign.Request.Options options
151151
requestScoped = delegate.newBuilder()
152152
.connectTimeout(options.connectTimeoutMillis(), TimeUnit.MILLISECONDS)
153153
.readTimeout(options.readTimeoutMillis(), TimeUnit.MILLISECONDS)
154+
.followRedirects(options.isFollowRedirects())
154155
.build();
155156
} else {
156157
requestScoped = delegate;

‎okhttp/src/test/java/feign/okhttp/OkHttpClientTest.java‎

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@
1515

1616
import feign.Feign.Builder;
1717
import feign.Headers;
18-
import feign.Param;
1918
import feign.RequestLine;
2019
import feign.Response;
20+
import feign.Request;
2121
import feign.Util;
2222
import feign.assertj.MockWebServerAssertions;
2323
import feign.client.AbstractClientTest;
@@ -26,8 +26,6 @@
2626
import okhttp3.mockwebserver.MockResponse;
2727
import org.junit.Test;
2828

29-
import java.util.HashMap;
30-
import java.util.Map;
3129

3230
import static org.junit.Assert.assertEquals;
3331

@@ -57,10 +55,48 @@ public void testContentTypeWithoutCharset() throws Exception {
5755
}
5856

5957

58+
@Test
59+
public void testNoFollowRedirect() throws Exception {
60+
server.enqueue(new MockResponse().setResponseCode(302).addHeader("Location", server.url("redirect")));
61+
62+
OkHttpClientTestInterface api = newBuilder()
63+
.options(new Request.Options(1000, 1000, false))
64+
.target(OkHttpClientTestInterface.class, "http://localhost:" + server.getPort());
65+
66+
Response response = api.get();
67+
// Response length should not be null
68+
assertEquals(302, response.status());
69+
assertEquals(server.url("redirect").toString(), response.headers().get("Location").iterator().next());
70+
71+
}
72+
73+
74+
@Test
75+
public void testFollowRedirect() throws Exception {
76+
String expectedBody = "Hello";
77+
78+
server.enqueue(new MockResponse().setResponseCode(302).addHeader("Location", server.url("redirect")));
79+
server.enqueue(new MockResponse().setBody(expectedBody));
80+
81+
OkHttpClientTestInterface api = newBuilder()
82+
.options(new Request.Options(1000, 1000, true))
83+
.target(OkHttpClientTestInterface.class, "http://localhost:" + server.getPort());
84+
85+
Response response = api.get();
86+
// Response length should not be null
87+
assertEquals(200, response.status());
88+
assertEquals(expectedBody, response.body().toString());
89+
90+
}
91+
92+
6093
public interface OkHttpClientTestInterface {
6194

6295
@RequestLine("GET /")
6396
@Headers({"Accept: text/plain", "Content-Type: text/plain"})
6497
Response getWithContentType();
98+
99+
@RequestLine("GET /")
100+
Response get();
65101
}
66102
}

‎ribbon/src/main/java/feign/ribbon/LBClient.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public final class LBClient extends
4444
private final int readTimeout;
4545
private final IClientConfig clientConfig;
4646
private final Set<Integer> retryableStatusCodes;
47+
private final Boolean followRedirects;
4748

4849
public static LBClient create(ILoadBalancer lb, IClientConfig clientConfig) {
4950
return new LBClient(lb, clientConfig);
@@ -66,6 +67,7 @@ static Set<Integer> parseStatusCodes(String statusCodesString) {
6667
connectTimeout = clientConfig.get(CommonClientConfigKey.ConnectTimeout);
6768
readTimeout = clientConfig.get(CommonClientConfigKey.ReadTimeout);
6869
retryableStatusCodes = parseStatusCodes(clientConfig.get(LBClientFactory.RetryableStatusCodes));
70+
followRedirects = clientConfig.get(CommonClientConfigKey.FollowRedirects);
6971
}
7072

7173
@Override
@@ -76,7 +78,8 @@ public RibbonResponse execute(RibbonRequest request, IClientConfig configOverrid
7678
options =
7779
new Request.Options(
7880
configOverride.get(CommonClientConfigKey.ConnectTimeout, connectTimeout),
79-
(configOverride.get(CommonClientConfigKey.ReadTimeout, readTimeout)));
81+
(configOverride.get(CommonClientConfigKey.ReadTimeout, readTimeout)),
82+
configOverride.get(CommonClientConfigKey.FollowRedirects,followRedirects));
8083
} else {
8184
options = new Request.Options(connectTimeout, readTimeout);
8285
}

‎ribbon/src/main/java/feign/ribbon/RibbonClient.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ static class FeignOptionsClientConfig extends DefaultClientConfigImpl {
107107
public FeignOptionsClientConfig(Request.Options options) {
108108
setProperty(CommonClientConfigKey.ConnectTimeout, options.connectTimeoutMillis());
109109
setProperty(CommonClientConfigKey.ReadTimeout, options.readTimeoutMillis());
110+
setProperty(CommonClientConfigKey.FollowRedirects, options.isFollowRedirects());
110111
}
111112

112113
@Override

‎ribbon/src/test/java/feign/ribbon/RibbonClientTest.java‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
import static org.assertj.core.api.Assertions.assertThat;
1818
import static org.hamcrest.core.IsEqual.equalTo;
1919
import static org.junit.Assert.assertEquals;
20+
import static org.junit.Assert.assertNotNull;
21+
import static org.junit.Assert.assertFalse;
2022
import static org.junit.Assert.assertThat;
2123
import static org.junit.Assert.assertTrue;
2224
import static org.junit.Assert.fail;
2325

2426
import java.io.IOException;
2527
import java.net.URI;
2628
import java.net.URL;
29+
import java.util.Collection;
2730

2831
import org.junit.After;
2932
import org.junit.AfterClass;
@@ -42,6 +45,7 @@
4245
import feign.Feign;
4346
import feign.Param;
4447
import feign.Request;
48+
import feign.Response;
4549
import feign.RequestLine;
4650
import feign.RetryableException;
4751
import feign.Retryer;
@@ -277,6 +281,62 @@ public void ribbonRetryOnStatusCodes() throws IOException, InterruptedException
277281
assertEquals(1, server1.getRequestCount());
278282
assertEquals(1, server2.getRequestCount());
279283
}
284+
285+
286+
@Test
287+
public void testFeignOptionsFollowRedirect() {
288+
String expectedLocation = server2.url("").url().toString();
289+
server1.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", expectedLocation));
290+
291+
getConfigInstance().setProperty(serverListKey(), hostAndPort(server1.url("").url()));
292+
293+
Request.Options options = new Request.Options(1000, 1000, false);
294+
TestInterface api = Feign.builder()
295+
.options(options)
296+
.client(RibbonClient.create())
297+
.retryer(Retryer.NEVER_RETRY)
298+
.target(TestInterface.class, "http://" + client());
299+
300+
try {
301+
Response response = api.get();
302+
assertEquals(302, response.status());
303+
Collection<String> location = response.headers().get("Location");
304+
assertNotNull(location);
305+
assertFalse(location.isEmpty());
306+
assertEquals(expectedLocation, location.iterator().next());
307+
} catch (Exception ignored) {
308+
ignored.printStackTrace();
309+
fail("Shouldn't throw ");
310+
}
311+
312+
}
313+
314+
@Test
315+
public void testFeignOptionsNoFollowRedirect() {
316+
// 302 will say go to server 2
317+
server1.enqueue(new MockResponse().setResponseCode(302).setHeader("Location", server2.url("").url().toString()));
318+
// server 2 will send back 200 with "Hello" as body
319+
server2.enqueue(new MockResponse().setResponseCode(200).setBody("Hello"));
320+
321+
getConfigInstance().setProperty(serverListKey(), hostAndPort(server1.url("").url()) + "," + hostAndPort(server2.url("").url()));
322+
323+
Request.Options options = new Request.Options(1000, 1000, true);
324+
TestInterface api = Feign.builder()
325+
.options(options)
326+
.client(RibbonClient.create())
327+
.retryer(Retryer.NEVER_RETRY)
328+
.target(TestInterface.class, "http://" + client());
329+
330+
try {
331+
Response response = api.get();
332+
assertEquals(200, response.status());
333+
assertEquals("Hello", response.body().toString());
334+
} catch (Exception ignored) {
335+
ignored.printStackTrace();
336+
fail("Shouldn't throw ");
337+
}
338+
339+
}
280340

281341
@Test
282342
public void testFeignOptionsClientConfig() {
@@ -285,7 +345,8 @@ public void testFeignOptionsClientConfig() {
285345
assertThat(config.get(CommonClientConfigKey.ConnectTimeout),
286346
equalTo(options.connectTimeoutMillis()));
287347
assertThat(config.get(CommonClientConfigKey.ReadTimeout), equalTo(options.readTimeoutMillis()));
288-
assertEquals(2, config.getProperties().size());
348+
assertThat(config.get(CommonClientConfigKey.FollowRedirects), equalTo(options.isFollowRedirects()));
349+
assertEquals(3, config.getProperties().size());
289350
}
290351

291352
@Test
@@ -320,5 +381,8 @@ interface TestInterface {
320381

321382
@RequestLine("GET /?a={a}")
322383
void getWithQueryParameters(@Param("a") String a);
384+
385+
@RequestLine("GET /")
386+
Response get();
323387
}
324388
}

0 commit comments

Comments
 (0)