From 042244428fb56b53de116adb379cf93ee49358c5 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 13 Mar 2018 16:33:59 -0400 Subject: [PATCH] Fix NPE when body is empty. Fixes #12. --- .../ribbon/RetryableFeignLoadBalancer.java | 2 +- .../RetryableFeignLoadBalancerTests.java | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java index 25d5a85c..89db2b11 100644 --- a/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java +++ b/spring-cloud-openfeign-core/src/main/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancer.java @@ -102,7 +102,7 @@ public class RetryableFeignLoadBalancer extends FeignLoadBalancer implements Ser } Response response = request.client().execute(feignRequest, options); if (retryPolicy.retryableStatusCode(response.status())) { - byte[] byteArray = StreamUtils.copyToByteArray(response.body().asInputStream()); + byte[] byteArray = response.body() == null ? new byte[]{} : StreamUtils.copyToByteArray(response.body().asInputStream()); response.close(); throw new RibbonResponseStatusCodeException(RetryableFeignLoadBalancer.this.clientName, response, byteArray, request.getUri()); diff --git a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java index e8bc6c3e..b4fd8afc 100644 --- a/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java +++ b/spring-cloud-openfeign-core/src/test/java/org/springframework/cloud/openfeign/ribbon/RetryableFeignLoadBalancerTests.java @@ -76,6 +76,7 @@ import static org.junit.Assert.assertEquals; import static org.mockito.Matchers.any; import static org.mockito.Matchers.anyBoolean; import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.anyObject; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.doThrow; @@ -241,6 +242,46 @@ public class RetryableFeignLoadBalancerTests { assertEquals(1, backOffPolicy.getCount()); } + @Test + public void executeRetryOnStatusCodeWithEmptyBody() throws Exception { + int retriesNextServer = 0; + when(this.config.get(MaxAutoRetriesNextServer, + DEFAULT_MAX_AUTO_RETRIES_NEXT_SERVER)).thenReturn(retriesNextServer); + doReturn(new Server("foo", 80)).when(lb).chooseServer(anyObject()); + RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config); + SpringClientFactory clientFactory = mock(SpringClientFactory.class); + IClientConfig config = mock(IClientConfig.class); + doReturn(1).when(config).get(eq(CommonClientConfigKey.MaxAutoRetries), anyInt()); + doReturn(retriesNextServer).when(config).get(eq(CommonClientConfigKey.MaxAutoRetriesNextServer), anyInt()); + doReturn(true).when(config).get(eq(CommonClientConfigKey.OkToRetryOnAllOperations), eq(false)); + doReturn(defaultConnectTimeout).when(config).get(eq(CommonClientConfigKey.ConnectTimeout)); + doReturn(defaultReadTimeout).when(config).get(eq(CommonClientConfigKey.ReadTimeout)); + doReturn("404").when(config).getPropertyAsString(eq(RibbonLoadBalancedRetryPolicy.RETRYABLE_STATUS_CODES),eq("")); + doReturn(config).when(clientFactory).getClientConfig(eq("default")); + doReturn(lbContext).when(clientFactory).getLoadBalancerContext(any(String.class)); + MyBackOffPolicy backOffPolicy = new MyBackOffPolicy(); + RibbonLoadBalancedRetryFactory loadBalancedRetryFactory = new RibbonLoadBalancedRetryFactory(clientFactory){ + @Override + public BackOffPolicy createBackOffPolicy(String service) { + return backOffPolicy; + } + }; + HttpRequest springRequest = mock(HttpRequest.class); + Request feignRequest = Request.create("GET", "http://foo", new HashMap>(), + new byte[]{}, StandardCharsets.UTF_8); + Client client = mock(Client.class); + FeignLoadBalancer.RibbonRequest request = new FeignLoadBalancer.RibbonRequest(client, feignRequest, new URI("http://foo")); + Response response = Response.builder().status(404).headers(new HashMap>()).build(); + Response fourOFourResponse = Response.builder().status(404).headers(new HashMap>()).build(); + doReturn(fourOFourResponse).doReturn(response).when(client).execute(any(Request.class), any(Request.Options.class)); + RetryableFeignLoadBalancer feignLb = new RetryableFeignLoadBalancer(lb, config, inspector, loadBalancedRetryFactory); + FeignLoadBalancer.RibbonResponse ribbonResponse = feignLb.execute(request, null); + assertEquals(404, ribbonResponse.toResponse().status()); + assertEquals(new Integer(0), ribbonResponse.toResponse().body().length()); + verify(client, times(2)).execute(any(Request.class), any(Request.Options.class)); + assertEquals(1, backOffPolicy.getCount()); + } + @Test public void getRequestSpecificRetryHandler() throws Exception { RibbonLoadBalancerContext lbContext = new RibbonLoadBalancerContext(lb, config);