From 504720e3662d3362bbd2f8fbb54b4ec20d122edb Mon Sep 17 00:00:00 2001 From: Craig Rueda Date: Fri, 15 Dec 2017 11:07:34 -0800 Subject: [PATCH 1/4] InputStream should always be closed after reading contents from upstreams (#2501) * InputStream should always be closed after reading contents from upstreams --- .../zuul/filters/post/SendResponseFilter.java | 20 ++++++++++--- .../filters/post/SendResponseFilterTests.java | 30 +++++++++++++++++-- 2 files changed, 44 insertions(+), 6 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilter.java index 7c178a62..4a9958a7 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilter.java @@ -191,12 +191,24 @@ public class SendResponseFilter extends ZuulFilter { } finally { /** - * Closing the wrapping InputStream itself has no effect on closing the underlying tcp connection since it's a wrapped stream. I guess for http - * keep-alive. When closing the wrapping stream it tries to reach the end of the current request, which is impossible for infinite http streams. So - * instead of closing the InputStream we close the HTTP response. + * We must ensure that the InputStream provided by our upstream pooling mechanism is ALWAYS closed + * even in the case of wrapped streams, which are supplied by pooled sources such as Apache's + * PoolingHttpClientConnectionManager. In that particular case, the underlying HTTP connection will + * be returned back to the connection pool iif either close() is explicitly called, a read + * error occurs, or the end of the underlying stream is reached. If, however a write error occurs, we will + * end up leaking a connection from the pool without an explicit close() * * @author Johannes Edmeier */ + if (is != null) { + try { + is.close(); + } + catch (Exception ex) { + log.warn("Error while closing upstream input stream", ex); + } + } + try { Object zuulResponse = RequestContext.getCurrentContext() .get("zuulResponse"); @@ -207,7 +219,7 @@ public class SendResponseFilter extends ZuulFilter { // The container will close the stream for us } catch (IOException ex) { - log.warn("Error while sending response to client: " + ex.getMessage()); + log.warn("Error while sending response to client: " + ex.getMessage()); } } } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilterTests.java index 871514bb..b28b1879 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/post/SendResponseFilterTests.java @@ -19,6 +19,7 @@ package org.springframework.cloud.netflix.zuul.filters.post; import java.io.ByteArrayInputStream; import java.io.Closeable; import java.io.IOException; +import java.io.InputStream; import java.lang.reflect.UndeclaredThrowableException; import javax.servlet.ServletOutputStream; @@ -48,6 +49,7 @@ import static org.mockito.Matchers.anyInt; import static org.mockito.Matchers.isA; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.X_ZUUL_DEBUG_HEADER; @@ -115,13 +117,14 @@ public class SendResponseFilterTests { } @Test - public void closeResponseOutpusStreamError() throws Exception { + public void closeResponseOutputStreamError() throws Exception { HttpServletResponse response = mock(HttpServletResponse.class); + InputStream mockStream = spy(new ByteArrayInputStream("Hello\n".getBytes("UTF-8"))); RequestContext context = new RequestContext(); context.setRequest(new MockHttpServletRequest()); context.setResponse(response); - context.setResponseDataStream(new ByteArrayInputStream("Hello\n".getBytes("UTF-8"))); + context.setResponseDataStream(mockStream); Closeable zuulResponse = mock(Closeable.class); context.set("zuulResponse", zuulResponse); RequestContext.testSetCurrentContext(context); @@ -140,6 +143,29 @@ public class SendResponseFilterTests { } verify(zuulResponse).close(); + verify(mockStream).close(); + } + + @Test + public void testCloseResponseDataStream() throws Exception { + HttpServletResponse response = mock(HttpServletResponse.class); + InputStream mockStream = spy(new ByteArrayInputStream("Hello\n".getBytes("UTF-8"))); + + RequestContext context = new RequestContext(); + context.setRequest(new MockHttpServletRequest()); + context.setResponse(response); + context.setResponseDataStream(mockStream); + Closeable zuulResponse = mock(Closeable.class); + context.set("zuulResponse", zuulResponse); + RequestContext.testSetCurrentContext(context); + + when(response.getOutputStream()).thenReturn(mock(ServletOutputStream.class)); + + SendResponseFilter filter = new SendResponseFilter(); + + filter.run(); + + verify(mockStream).close(); } private void runFilter(String characterEncoding, String content, boolean streamContent) throws Exception { From 94358489ef79c6ac17a4b077e5ddef7cba5c7f89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=91=A8=E7=AB=8B?= Date: Mon, 18 Dec 2017 22:05:30 +0800 Subject: [PATCH 2/4] Change spring-cloud-starter-hystrix-dashboard to spring-cloud-starter-netflix-hystrix-dashboard (#2561) Change spring-cloud-starter-hystrix-dashboard to spring-cloud-starter-netflix-hystrix-dashboard --- spring-cloud-starter-hystrix-dashboard/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-starter-hystrix-dashboard/pom.xml b/spring-cloud-starter-hystrix-dashboard/pom.xml index d9b5f5d3..b6797c57 100644 --- a/spring-cloud-starter-hystrix-dashboard/pom.xml +++ b/spring-cloud-starter-hystrix-dashboard/pom.xml @@ -10,7 +10,7 @@ spring-cloud-starter-hystrix-dashboard spring-cloud-starter-hystrix-dashboard - Spring Cloud Starter Hystrix Dashboard (deprecated, please use spring-cloud-starter-hystrix-dashboard) + Spring Cloud Starter Hystrix Dashboard (deprecated, please use spring-cloud-starter-netflix-hystrix-dashboard) https://projects.spring.io/spring-cloud Pivotal Software, Inc. From 9deab33dc1fbbcc0601febdaafcc43df9d1b16a0 Mon Sep 17 00:00:00 2001 From: Nastya Smirnova Date: Tue, 2 Jan 2018 22:42:59 +0200 Subject: [PATCH 3/4] fixes #2524 (#2574) set proper status and health eureka urls if prefer ip address is set --- .../eureka/EurekaClientAutoConfiguration.java | 4 ++++ .../EurekaClientAutoConfigurationTests.java | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java index d2c2a61e..c7ba4d8f 100644 --- a/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java +++ b/spring-cloud-netflix-eureka-client/src/main/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfiguration.java @@ -135,6 +135,7 @@ public class EurekaClientAutoConfiguration { String hostname = eurekaPropertyResolver.getProperty("hostname"); boolean preferIpAddress = Boolean.parseBoolean(eurekaPropertyResolver.getProperty("preferIpAddress")); + String ipAddress = eurekaPropertyResolver.getProperty("ipAddress"); boolean isSecurePortEnabled = Boolean.parseBoolean(eurekaPropertyResolver.getProperty("securePortEnabled")); String serverContextPath = propertyResolver.getProperty("server.contextPath", "/"); int serverPort = Integer.valueOf(propertyResolver.getProperty("server.port", propertyResolver.getProperty("port", "8080"))); @@ -147,6 +148,9 @@ public class EurekaClientAutoConfiguration { instance.setNonSecurePort(serverPort); instance.setInstanceId(getDefaultInstanceId(propertyResolver)); instance.setPreferIpAddress(preferIpAddress); + if (StringUtils.hasText(ipAddress)) { + instance.setIpAddress(ipAddress); + } if(isSecurePortEnabled) { instance.setSecurePort(serverPort); diff --git a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java index 0143b7d1..a787475f 100644 --- a/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java +++ b/spring-cloud-netflix-eureka-client/src/test/java/org/springframework/cloud/netflix/eureka/EurekaClientAutoConfigurationTests.java @@ -289,6 +289,25 @@ public class EurekaClientAutoConfigurationTests { assertEquals("statusPageUrl is wrong", "http://" + instance.getIpAddress() + ":9999/info", instance.getStatusPageUrl()); + assertEquals("healthCheckUrl is wrong", "http://" + instance.getIpAddress() + ":9999/health", + instance.getHealthCheckUrl()); + } + + @Test + public void statusPageAndHealthCheckUrlsShouldSetUserDefinedIpAddress() { + addEnvironment(this.context, "server.port=8989", + "management.port=9999", "eureka.instance.hostname=foo", + "eureka.instance.ipAddress:192.168.13.90", + "eureka.instance.preferIpAddress:true"); + + setupContext(RefreshAutoConfiguration.class); + EurekaInstanceConfigBean instance = this.context + .getBean(EurekaInstanceConfigBean.class); + + assertEquals("statusPageUrl is wrong", "http://192.168.13.90:9999/info", + instance.getStatusPageUrl()); + assertEquals("healthCheckUrl is wrong", "http://192.168.13.90:9999/health", + instance.getHealthCheckUrl()); } @Test From a4c995a394d99b3303d6bf3245a99b0fa33390fe Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 3 Jan 2018 14:19:39 -0500 Subject: [PATCH 4/4] Disable following redirects in SimpleHostRoutingFilter. Fixes #2578. (#2608) --- .../route/SimpleHostRoutingFilter.java | 2 +- .../route/SimpleHostRoutingFilterTests.java | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index 58a368d7..db068f5c 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -220,7 +220,7 @@ public class SimpleHostRoutingFilter extends ZuulFilter { .setCookieSpec(CookieSpecs.IGNORE_COOKIES).build(); return httpClientFactory.createBuilder(). setDefaultRequestConfig(requestConfig). - setConnectionManager(this.connectionManager).build(); + setConnectionManager(this.connectionManager).disableRedirectHandling().build(); } private CloseableHttpResponse forward(CloseableHttpClient httpclient, String verb, diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java index 43212d12..df348692 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java @@ -196,6 +196,20 @@ public class SimpleHostRoutingFilterTests { assertTrue("Get 1".equals(responseString)); } + + @Test + public void redirectTest() throws Exception { + setupContext(); + InputStreamEntity inputStreamEntity = new InputStreamEntity(new ByteArrayInputStream(new byte[]{})); + HttpRequest httpRequest = getFilter().buildHttpRequest("GET", "/app/redirect", inputStreamEntity, + new LinkedMultiValueMap(), new LinkedMultiValueMap(), new MockHttpServletRequest()); + + CloseableHttpResponse response = getFilter().newClient().execute(new HttpHost("localhost", this.port), httpRequest); + assertEquals(302, response.getStatusLine().getStatusCode()); + String responseString = copyToString(response.getEntity().getContent(), Charset.forName("UTF-8")); + assertTrue(response.getLastHeader("Location").getValue().contains("/app/get/5")); + } + @Test public void zuulHostKeysUpdateHttpClient() { setupContext(); @@ -259,6 +273,12 @@ class SampleApplication { public String getString(@PathVariable String id, HttpServletResponse response) throws IOException { return "Get " + id; } + + @RequestMapping(value = "/redirect", method = RequestMethod.GET) + public String redirect(HttpServletResponse response) throws IOException { + response.sendRedirect("/app/get/5"); + return null; + } } class GZIPCompression {