From c8829f2aa4ffac27772c4e960ced51a582f38d00 Mon Sep 17 00:00:00 2001 From: Max Ishchenko Date: Tue, 11 Oct 2016 17:19:33 +0300 Subject: [PATCH 1/4] Prevent double url encoding for secure ribbon urls (#1389) Secure ribbon urls were forced to use https scheme via UriComponentsBuilder, that was created from original uri. This transformation url encoded previously encoded url parts that were used to create builder. This was introduced in c883495. This change fixes double url encoding using RibbonUtils.updateToHttpsIfNeeded that fixes double escaping case and corner case with '+' in url as well. Fixes gh-1382 --- .../ribbon/RibbonLoadBalancerClient.java | 9 +++---- .../ribbon/RibbonLoadBalancerClientTests.java | 26 +++++++++++++++++++ 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java index 4d00a9fb..c9261db5 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClient.java @@ -52,11 +52,10 @@ public class RibbonLoadBalancerClient implements LoadBalancerClient { RibbonLoadBalancerContext context = this.clientFactory .getLoadBalancerContext(serviceId); Server server = new Server(instance.getHost(), instance.getPort()); - boolean secure = isSecure(server, serviceId); - URI uri = original; - if (secure) { - uri = UriComponentsBuilder.fromUri(uri).scheme("https").build().toUri(); - } + IClientConfig clientConfig = clientFactory.getClientConfig(serviceId); + ServerIntrospector serverIntrospector = serverIntrospector(serviceId); + URI uri = RibbonUtils.updateToHttpsIfNeeded(original, clientConfig, + serverIntrospector, server); return context.reconstructURIWithServer(server, uri); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java index e224a134..f2e540cd 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java @@ -38,6 +38,7 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerStats; import lombok.SneakyThrows; +import org.springframework.web.util.DefaultUriTemplateHandler; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; @@ -107,6 +108,31 @@ public class RibbonLoadBalancerClientTests { assertEquals(server.getPort(), uri.getPort()); } + @Test + public void testReconstructSecureUriWithSpecialCharsPath() { + testReconstructUriWithPath("https", "/foo=|"); + } + + @Test + public void testReconstructUnsecureUriWithSpecialCharsPath() { + testReconstructUriWithPath("http", "/foo=|"); + } + + private void testReconstructUriWithPath(String scheme, String path) { + RibbonServer server = getRibbonServer(); + IClientConfig config = mock(IClientConfig.class); + when(config.get(CommonClientConfigKey.IsSecure)).thenReturn(true); + when(clientFactory.getClientConfig(server.getServiceId())).thenReturn(config); + + RibbonLoadBalancerClient client = getRibbonLoadBalancerClient(server); + ServiceInstance serviceInstance = client.choose(server.getServiceId()); + + URI expanded = new DefaultUriTemplateHandler() + .expand(scheme + "://" + server.getServiceId() + path); + URI reconstructed = client.reconstructURI(serviceInstance, expanded); + assertEquals(expanded.getPath(), reconstructed.getPath()); + } + @Test @SneakyThrows public void testReconstructUriWithSecureClientConfig() { From aa160940df8901bde2d30ad4ac48e868e765e939 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Tue, 11 Oct 2016 10:25:02 -0600 Subject: [PATCH 2/4] import order --- .../cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java index f2e540cd..905cc6ac 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ribbon/RibbonLoadBalancerClientTests.java @@ -29,6 +29,7 @@ import org.mockito.MockitoAnnotations; import org.springframework.cloud.client.ServiceInstance; import org.springframework.cloud.client.loadbalancer.LoadBalancerRequest; import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancerClient.RibbonServer; +import org.springframework.web.util.DefaultUriTemplateHandler; import com.netflix.client.config.CommonClientConfigKey; import com.netflix.client.config.IClientConfig; @@ -38,7 +39,6 @@ import com.netflix.loadbalancer.Server; import com.netflix.loadbalancer.ServerStats; import lombok.SneakyThrows; -import org.springframework.web.util.DefaultUriTemplateHandler; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; From 5a127d07002daa6e7cb8ef8d4e9ce302aa49e84c Mon Sep 17 00:00:00 2001 From: mg Date: Sun, 2 Oct 2016 13:44:51 -0500 Subject: [PATCH 3/4] Warning for GC Overhead Limit Reached with servo. Fixes gh-947 --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 1194d049..13d3d4b0 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1758,6 +1758,18 @@ If Spring AOP is enabled and `org.aspectj:aspectjweaver` is present on your runt 3. URI, sanitized for Atlas 4. Client name +WARNING: Avoid using hardcoded url parameters within `RestTemplate`. When targeting dynamic endpoints use URL variables. This will avoid potential "GC Overhead Limit Reached" issues where `ServoMonitorCache` treats each url as a unique key. + +[source,java,indent=0] +---- +// recommended +String orderid = "1"; +restTemplate.getForObject("http://testeurekabrixtonclient/orders/{orderid}", String.class, orderid) + +// avoid +restTemplate.getForObject("http://testeurekabrixtonclient/orders/1", String.class) +---- + [[netflix-metrics-spectator]] === Metrics Collection: Spectator From 18c4bc060b2b3c19f1ce9b0b1d6f3f1b6cbf85dd Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 11 Oct 2016 13:41:31 -0400 Subject: [PATCH 4/4] Upgrade hystrix 1.5.6 (#1388) Fixes gh-1374 --- .../zuul/filters/pre/FormBodyWrapperFilter.java | 13 ++++++------- spring-cloud-netflix-dependencies/pom.xml | 2 +- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java index 22681762..500ee5a2 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/FormBodyWrapperFilter.java @@ -22,16 +22,14 @@ import java.io.OutputStream; import java.lang.reflect.Field; import java.util.HashSet; import java.util.List; -import java.util.Map.Entry; import java.util.Set; +import java.util.Map.Entry; import javax.servlet.ServletInputStream; import javax.servlet.ServletRequest; import javax.servlet.ServletRequestWrapper; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.Part; -import org.apache.commons.lang3.StringUtils; import org.springframework.core.io.InputStreamResource; import org.springframework.core.io.Resource; import org.springframework.http.HttpEntity; @@ -44,6 +42,7 @@ import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; import org.springframework.web.multipart.MultipartFile; import org.springframework.web.multipart.MultipartRequest; import org.springframework.web.servlet.DispatcherServlet; @@ -65,7 +64,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { this.requestField = ReflectionUtils.findField(HttpServletRequestWrapper.class, "req", HttpServletRequest.class); this.servletRequestField = ReflectionUtils.findField(ServletRequestWrapper.class, - "request", ServletRequest.class); + "request", ServletRequest.class); Assert.notNull(this.requestField, "HttpServletRequestWrapper.req field not found"); Assert.notNull(this.servletRequestField, @@ -121,7 +120,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { .getField(this.requestField, request); wrapper = new FormBodyRequestWrapper(wrapped); ReflectionUtils.setField(this.requestField, request, wrapper); - if(request instanceof ServletRequestWrapper) { + if (request instanceof ServletRequestWrapper) { ReflectionUtils.setField(this.servletRequestField, request, wrapper); } } @@ -170,7 +169,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { } return this.contentLength; } - + @Override public long getContentLengthLong() { return getContentLength(); @@ -233,7 +232,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { Set result = new HashSet<>(); String query = this.request.getQueryString(); if (query != null) { - for (String value : StringUtils.split(query, "&")) { + for (String value : StringUtils.tokenizeToStringArray(query, "&")) { if (value.contains("=")) { value = value.substring(0, value.indexOf("=")); } diff --git a/spring-cloud-netflix-dependencies/pom.xml b/spring-cloud-netflix-dependencies/pom.xml index 99e3cb2d..094bee0e 100644 --- a/spring-cloud-netflix-dependencies/pom.xml +++ b/spring-cloud-netflix-dependencies/pom.xml @@ -17,7 +17,7 @@ 0.7.4 1.4.11 9.3.1 - 1.5.5 + 1.5.6 2.2.0 0.10.1 1.2.2