From ff68e0f66e6ce04cf4a49ffd836dd785a99787eb Mon Sep 17 00:00:00 2001 From: saga Date: Tue, 23 Jan 2018 00:27:15 +0800 Subject: [PATCH] Prevent RibbonRoutingFilter throws exceptions when Http status codes are non-standard(Against the 1.4.x branches) (#2671) * This PR fixes #2639 and against the 1.4 branches --- .../filters/route/RibbonRoutingFilter.java | 5 +- .../route/RibbonRoutingFilterTests.java | 99 +++++++++++++++++++ 2 files changed, 101 insertions(+), 3 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java index 132dfc7d..eacb8dea 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java @@ -156,8 +156,7 @@ public class RibbonRoutingFilter extends ZuulFilter { RibbonCommand command = this.ribbonCommandFactory.create(context); try { ClientHttpResponse response = command.execute(); - this.helper.appendDebug(info, response.getStatusCode().value(), - response.getHeaders()); + this.helper.appendDebug(info, response.getRawStatusCode(), response.getHeaders()); return response; } catch (HystrixRuntimeException ex) { @@ -225,7 +224,7 @@ public class RibbonRoutingFilter extends ZuulFilter { protected void setResponse(ClientHttpResponse resp) throws ClientException, IOException { RequestContext.getCurrentContext().set("zuulResponse", resp); - this.helper.setResponse(resp.getStatusCode().value(), + this.helper.setResponse(resp.getRawStatusCode(), resp.getBody() == null ? null : resp.getBody(), resp.getHeaders()); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java index 08d3b646..b848981f 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java @@ -17,6 +17,9 @@ package org.springframework.cloud.netflix.zuul.filters.route; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; import java.util.Collections; import com.netflix.zuul.context.RequestContext; @@ -25,7 +28,14 @@ import org.junit.Before; import org.junit.Test; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; +import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpStatus; +import org.springframework.http.MediaType; +import org.springframework.http.client.ClientHttpResponse; import org.springframework.mock.web.MockHttpServletRequest; +import org.springframework.mock.web.MockHttpServletResponse; + +import javax.servlet.http.HttpServletResponse; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.Mockito.mock; @@ -35,6 +45,7 @@ import static org.springframework.cloud.netflix.zuul.filters.support.FilterConst /** * @author Spencer Gibb * @author Yongsung Yoon + * @author Gang Li */ public class RibbonRoutingFilterTests { @@ -74,18 +85,106 @@ public class RibbonRoutingFilterTests { assertThat(commandContext.getLoadBalancerKey()).isNull(); } + @Test + public void testSetResponseWithNonHttpStatusCode() throws Exception { + ClientHttpResponse response = this.createClientHttpResponseWithNonStatus(); + this.filter.setResponse(response); + assertThat(517).isEqualTo(this.requestContext.get("responseStatusCode")); + } + + @Test + public void testSetResponseWithHttpStatusCode() throws Exception { + ClientHttpResponse response = this.createClientHttpResponse(); + this.filter.setResponse(response); + assertThat(200).isEqualTo(this.requestContext.get("responseStatusCode")); + } + private void setUpRequestContext() { requestContext = RequestContext.getCurrentContext(); MockHttpServletRequest mockRequest = new MockHttpServletRequest(); + HttpServletResponse httpServletResponse = new MockHttpServletResponse(); mockRequest.setMethod("GET"); mockRequest.setRequestURI("/foo/bar"); requestContext.setRequest(mockRequest); requestContext.setRequestQueryParams(Collections.EMPTY_MAP); requestContext.set(SERVICE_ID_KEY, "testServiceId"); + requestContext.set("response", httpServletResponse); } private void setupRibbonRoutingFilter() { RibbonCommandFactory factory = mock(RibbonCommandFactory.class); filter = new RibbonRoutingFilter(new ProxyRequestHelper(), factory, Collections.emptyList()); } + + private ClientHttpResponse createClientHttpResponseWithNonStatus() { + return new ClientHttpResponse() { + @Override + public HttpStatus getStatusCode() throws IOException { + return null; + } + + @Override + public int getRawStatusCode() throws IOException { + return 517; + } + + @Override + public String getStatusText() throws IOException { + return "Fail"; + } + + @Override + public void close() { + + } + + @Override + public InputStream getBody() throws IOException { + return new ByteArrayInputStream("Fail".getBytes()); + } + + @Override + public HttpHeaders getHeaders() { + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.setContentType(MediaType.APPLICATION_JSON); + return httpHeaders; + } + }; + } + + private ClientHttpResponse createClientHttpResponse() { + return new ClientHttpResponse() { + @Override + public HttpStatus getStatusCode() throws IOException { + return HttpStatus.OK; + } + + @Override + public int getRawStatusCode() throws IOException { + return 200; + } + + @Override + public String getStatusText() throws IOException { + return "OK"; + } + + @Override + public void close() { + + } + + @Override + public InputStream getBody() throws IOException { + return new ByteArrayInputStream("OK".getBytes()); + } + + @Override + public HttpHeaders getHeaders() { + HttpHeaders httpHeaders = new HttpHeaders(); + httpHeaders.setContentType(MediaType.APPLICATION_JSON); + return httpHeaders; + } + }; + } }