From f19c7fbbe2333dd474da0a7785bf5359874676e2 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 6 Aug 2018 10:25:36 -0400 Subject: [PATCH 01/11] Exclude Netty dependencies. Fixes #3119 --- pom.xml | 20 +++++++++---------- .../pom.xml | 10 ++++++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/pom.xml b/pom.xml index 9c7ba0c6cc..6fa53c7291 100644 --- a/pom.xml +++ b/pom.xml @@ -120,16 +120,16 @@ pom import - - io.netty - netty-codec-http - ${netty.version} - - - io.netty - netty-transport-native-epoll - ${netty.version} - + + + + + + + + + + com.fasterxml.jackson.dataformat jackson-dataformat-smile diff --git a/spring-cloud-starter-netflix/spring-cloud-starter-netflix-ribbon/pom.xml b/spring-cloud-starter-netflix/spring-cloud-starter-netflix-ribbon/pom.xml index 7ff7f124f5..c07099c671 100644 --- a/spring-cloud-starter-netflix/spring-cloud-starter-netflix-ribbon/pom.xml +++ b/spring-cloud-starter-netflix/spring-cloud-starter-netflix-ribbon/pom.xml @@ -33,6 +33,16 @@ com.netflix.ribbon ribbon + + + io.netty + netty-codec-http + + + io.netty + netty-transport-native-epoll + + com.netflix.ribbon From d98b3ce3d6a4890f94bf1645f76f29765461503a Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 6 Aug 2018 10:27:46 -0400 Subject: [PATCH 02/11] Remove commented out lines --- pom.xml | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/pom.xml b/pom.xml index 6fa53c7291..8dc86a83e2 100644 --- a/pom.xml +++ b/pom.xml @@ -120,16 +120,6 @@ pom import - - - - - - - - - - com.fasterxml.jackson.dataformat jackson-dataformat-smile From 46719c0c313af828297babedac5614f7a728df5d Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Mon, 13 Aug 2018 14:08:55 -0400 Subject: [PATCH 03/11] Fix invalid URI error when Feign request URI does not end in / (#3136) * Fix invalid URI error when Feign request URI does not end in / --- .../netflix/feign/ribbon/LoadBalancerFeignClient.java | 8 +++++++- .../netflix/feign/ribbon/FeignRibbonClientTests.java | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java index 37d7f662ba..0be90266ff 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/feign/ribbon/LoadBalancerFeignClient.java @@ -97,7 +97,13 @@ public Client getDelegate() { } static URI cleanUrl(String originalUrl, String host) { - return URI.create(originalUrl.replaceFirst(host, "")); + String newUrl = originalUrl.replaceFirst(host, ""); + StringBuffer buffer = new StringBuffer(newUrl); + if((newUrl.startsWith("https://") && newUrl.length() == 8) || + (newUrl.startsWith("http://") && newUrl.length() == 7)) { + buffer.append("/"); + } + return URI.create(buffer.toString()); } private FeignLoadBalancer lbClient(String clientName) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java index 676ca30758..aadd8a0ce9 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java @@ -102,6 +102,16 @@ public void remoteRequestIsSent() throws Exception { any(Options.class)); } + @Test + public void remoteRequestIsSentAtRoot() throws Exception { + Request request = new RequestTemplate().method("GET").append("http://foo") + .request(); + this.client.execute(request, new Options()); + RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/"); + verify(this.delegate).execute(argThat(matcher), + any(Options.class)); + } + @Test public void remoteRequestIsSecure() throws Exception { Request request = new RequestTemplate().method("GET").append("https://foo/") From dd19bc5ba3975e2559f933081d0dd0244e3f2ee4 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 4 Sep 2018 14:24:36 -0400 Subject: [PATCH 04/11] Fixing typo --- docs/src/main/asciidoc/spring-cloud-netflix.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index fbbf311474..8318570e31 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -1925,7 +1925,7 @@ It should return a JSON document that resembles the following: } ---- -HThe following application.yml example shows sample configuration for a Sidecar application: +The following application.yml example shows sample configuration for a Sidecar application: .application.yml [source,yaml] From 6e8ef02a145e377d8c7e003af6ffa65607f7e481 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Wed, 5 Sep 2018 20:39:45 -0400 Subject: [PATCH 05/11] Allow the host header to be included if addHostHeader is true. Fixes #3171 (#3177) --- .../zuul/ZuulProxyAutoConfiguration.java | 8 +- .../zuul/filters/ProxyRequestHelper.java | 19 +++++ .../zuul/filters/TraceProxyRequestHelper.java | 9 +++ .../filters/route/RibbonRoutingFilter.java | 2 + .../zuul/filters/ProxyRequestHelperTests.java | 73 +++++++++++++------ .../filters/pre/PreDecorationFilterTests.java | 8 ++ .../route/RibbonRoutingFilterTests.java | 3 +- .../route/SimpleHostRoutingFilterTests.java | 2 +- 8 files changed, 92 insertions(+), 32 deletions(-) diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java index f6e63710e9..172d39f671 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/ZuulProxyAutoConfiguration.java @@ -141,9 +141,7 @@ protected static class NoActuatorConfiguration { @Bean public ProxyRequestHelper proxyRequestHelper(ZuulProperties zuulProperties) { - ProxyRequestHelper helper = new ProxyRequestHelper(); - helper.setIgnoredHeaders(zuulProperties.getIgnoredHeaders()); - helper.setTraceRequestBody(zuulProperties.isTraceRequestBody()); + ProxyRequestHelper helper = new ProxyRequestHelper(zuulProperties); return helper; } @@ -171,12 +169,10 @@ public FiltersEndpoint filtersEndpoint() { @Bean public ProxyRequestHelper proxyRequestHelper(ZuulProperties zuulProperties) { - TraceProxyRequestHelper helper = new TraceProxyRequestHelper(); + TraceProxyRequestHelper helper = new TraceProxyRequestHelper(zuulProperties); if (this.traces != null) { helper.setTraces(this.traces); } - helper.setIgnoredHeaders(zuulProperties.getIgnoredHeaders()); - helper.setTraceRequestBody(zuulProperties.isTraceRequestBody()); return helper; } } diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java index f4504283dc..dc9c163ec4 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java @@ -71,6 +71,18 @@ public class ProxyRequestHelper { private boolean traceRequestBody = true; + private boolean addHostHeader = false; + + @Deprecated + //TODO Remove in 2.1.x + public ProxyRequestHelper() {} + + public ProxyRequestHelper(ZuulProperties zuulProperties) { + this.ignoredHeaders.addAll(zuulProperties.getIgnoredHeaders()); + this.traceRequestBody = zuulProperties.isTraceRequestBody(); + this.addHostHeader = zuulProperties.isAddHostHeader(); + } + public void setWhitelistHosts(Set whitelistHosts) { this.whitelistHosts.addAll(whitelistHosts); } @@ -79,10 +91,14 @@ public void setSensitiveHeaders(Set sensitiveHeaders) { this.sensitiveHeaders.addAll(sensitiveHeaders); } + @Deprecated + //TODO Remove in 2.1.x public void setIgnoredHeaders(Set ignoredHeaders) { this.ignoredHeaders.addAll(ignoredHeaders); } + @Deprecated + //TODO Remove in 2.1.x public void setTraceRequestBody(boolean traceRequestBody) { this.traceRequestBody = traceRequestBody; } @@ -208,6 +224,9 @@ public boolean isIncludedHeader(String headerName) { } switch (name) { case "host": + if(addHostHeader) { + return true; + } case "connection": case "content-length": case "server": diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java index 13bc5d768d..b699664591 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/TraceProxyRequestHelper.java @@ -48,6 +48,15 @@ public class TraceProxyRequestHelper extends ProxyRequestHelper { private HttpTraceRepository traces; + + @Deprecated + //TODO Remove in 2.1.x + public TraceProxyRequestHelper(){} + + public TraceProxyRequestHelper(ZuulProperties zuulProperties) { + super(zuulProperties); + } + private final HttpExchangeTracer tracer = new HttpExchangeTracer( Include.defaultIncludes()); diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java index 5d4d8285c0..d1f182f767 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilter.java @@ -78,6 +78,8 @@ public RibbonRoutingFilter(ProxyRequestHelper helper, } } + @Deprecated + //TODO Remove in 2.1.x public RibbonRoutingFilter(RibbonCommandFactory ribbonCommandFactory) { this(new ProxyRequestHelper(), ribbonCommandFactory, null); } diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java index 3bd28365f4..bb922f2246 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelperTests.java @@ -44,6 +44,7 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.mockito.MockitoAnnotations.initMocks; @@ -82,7 +83,7 @@ public void debug() throws Exception { request.addHeader("multiName", "multiValue2"); RequestContext.getCurrentContext().setRequest(request); - TraceProxyRequestHelper helper = new TraceProxyRequestHelper(); + TraceProxyRequestHelper helper = new TraceProxyRequestHelper(new ZuulProperties()); this.traceRepository = new InMemoryHttpTraceRepository(); helper.setTraces(this.traceRepository); @@ -98,8 +99,9 @@ public void debug() throws Exception { public void shouldDebugBodyDisabled() throws Exception { RequestContext context = RequestContext.getCurrentContext(); - ProxyRequestHelper helper = new ProxyRequestHelper(); - helper.setTraceRequestBody(false); + ZuulProperties zuulProperties = new ZuulProperties(); + zuulProperties.setTraceRequestBody(false); + ProxyRequestHelper helper = new ProxyRequestHelper(zuulProperties); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(false)); } @@ -111,7 +113,7 @@ public void shouldDebugBodyChunked() throws Exception { context.setChunkedRequestBody(); context.setRequest(request); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(false)); } @@ -123,7 +125,7 @@ public void shouldDebugBodyServlet() throws Exception { context.setZuulEngineRan(); context.setRequest(request); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(false)); } @@ -135,7 +137,7 @@ public void shouldDebugBodyNullContentType() throws Exception { RequestContext context = RequestContext.getCurrentContext(); context.setRequest(request); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(true)); } @@ -144,7 +146,7 @@ public void shouldDebugBodyNullContentType() throws Exception { public void shouldDebugBodyNullRequest() throws Exception { RequestContext context = RequestContext.getCurrentContext(); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(true)); } @@ -156,7 +158,7 @@ public void shouldDebugBodyNotMultitypeContentType() throws Exception { RequestContext context = RequestContext.getCurrentContext(); context.setRequest(request); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(true)); } @@ -168,7 +170,7 @@ public void shouldDebugBodyMultitypeContentType() throws Exception { RequestContext context = RequestContext.getCurrentContext(); context.setRequest(request); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); assertThat("shouldDebugBody wrong", helper.shouldDebugBody(context), is(false)); } @@ -180,7 +182,7 @@ public void buildZuulRequestHeadersWork() { request.addHeader("multiName", "multiValue1"); request.addHeader("multiName", "multiValue2"); - TraceProxyRequestHelper helper = new TraceProxyRequestHelper(); + TraceProxyRequestHelper helper = new TraceProxyRequestHelper(new ZuulProperties()); helper.setTraces(this.traceRepository); MultiValueMap headers = helper.buildZuulRequestHeaders(request); @@ -201,7 +203,7 @@ public void buildZuulRequestHeadersWork() { public void buildZuulRequestHeadersRequestsGzipAndOnlyGzip() { MockHttpServletRequest request = new MockHttpServletRequest("", "/"); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); MultiValueMap headers = helper.buildZuulRequestHeaders(request); @@ -215,7 +217,7 @@ public void buildZuulRequestHeadersRequestsContentEncoding() { MockHttpServletRequest request = new MockHttpServletRequest("", "/"); request.addHeader("content-encoding", "identity"); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); MultiValueMap headers = helper.buildZuulRequestHeaders(request); @@ -229,7 +231,7 @@ public void buildZuulRequestHeadersRequestsAcceptEncoding() { MockHttpServletRequest request = new MockHttpServletRequest("", "/"); request.addHeader("accept-encoding", "identity"); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); MultiValueMap headers = helper.buildZuulRequestHeaders(request); @@ -238,6 +240,29 @@ public void buildZuulRequestHeadersRequestsAcceptEncoding() { assertThat(acceptEncodings, contains("identity")); } + @Test + public void addHostHeader() { + MockHttpServletRequest request = new MockHttpServletRequest("", "/"); + request.addHeader("host", "foo.com"); + + ZuulProperties zuulProperties = new ZuulProperties(); + zuulProperties.setAddHostHeader(true); + ProxyRequestHelper helper = new ProxyRequestHelper(zuulProperties); + + MultiValueMap headers = helper.buildZuulRequestHeaders(request); + + List acceptEncodings = headers.get("host"); + assertThat(acceptEncodings, hasSize(1)); + assertThat(acceptEncodings, contains("foo.com")); + + zuulProperties.setAddHostHeader(false); + helper = new ProxyRequestHelper(zuulProperties); + headers = helper.buildZuulRequestHeaders(request); + + acceptEncodings = headers.get("host"); + assertNull(acceptEncodings); + } + @Test public void setResponseLowercase() throws IOException { MockHttpServletRequest request = new MockHttpServletRequest("POST", "/"); @@ -247,7 +272,7 @@ public void setResponseLowercase() throws IOException { context.setRequest(request); context.setResponse(response); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); MultiValueMap headers = new HttpHeaders(); headers.add(HttpHeaders.CONTENT_ENCODING.toLowerCase(), "gzip"); @@ -265,7 +290,7 @@ public void setResponseShouldSetOriginResponseHeaders() throws IOException { context.setRequest(request); context.setResponse(response); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); MultiValueMap headers = new HttpHeaders(); headers.add(HttpHeaders.CONTENT_TYPE, "text/plain"); @@ -287,7 +312,7 @@ public void setResponseUppercase() throws IOException { context.setRequest(request); context.setResponse(response); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); MultiValueMap headers = new HttpHeaders(); headers.add(HttpHeaders.CONTENT_ENCODING, "gzip"); @@ -302,7 +327,7 @@ public void getQueryString() { params.add("a", "1234"); params.add("b", "5678"); - String queryString = new ProxyRequestHelper().getQueryString(params); + String queryString = new ProxyRequestHelper(new ZuulProperties()).getQueryString(params); assertThat(queryString, is("?a=1234&b=5678")); } @@ -312,7 +337,7 @@ public void getQueryStringWithEmptyParam() { MultiValueMap params = new LinkedMultiValueMap<>(); params.add("wsdl", ""); - String queryString = new ProxyRequestHelper().getQueryString(params); + String queryString = new ProxyRequestHelper(new ZuulProperties()).getQueryString(params); assertThat(queryString, is("?wsdl")); } @@ -322,7 +347,7 @@ public void getQueryStringEncoded() { MultiValueMap params = new LinkedMultiValueMap<>(); params.add("foo", "weird#chars"); - String queryString = new ProxyRequestHelper().getQueryString(params); + String queryString = new ProxyRequestHelper(new ZuulProperties()).getQueryString(params); assertThat(queryString, is("?foo=weird%23chars")); } @@ -334,7 +359,7 @@ public void getQueryParamNameWithColon() { params.add("foobar", "bam"); params.add("foo\fbar", "bat"); // form feed is the colon replacement char - String queryString = new ProxyRequestHelper().getQueryString(params); + String queryString = new ProxyRequestHelper(new ZuulProperties()).getQueryString(params); assertThat(queryString, is("?foo:bar=baz&foobar=bam&foo%0Cbar=bat")); } @@ -350,7 +375,7 @@ public void buildZuulRequestURIWithUTF8() throws Exception { context.setRequest(request); context.set(REQUEST_URI_KEY, decodedURI); - final String requestURI = new ProxyRequestHelper().buildZuulRequestURI(request); + final String requestURI = new ProxyRequestHelper(new ZuulProperties()).buildZuulRequestURI(request); assertThat(requestURI, equalTo(encodedURI)); } @@ -364,7 +389,7 @@ public void buildZuulRequestURIWithDefaultEncoding() { context.setRequest(request); context.set(REQUEST_URI_KEY, decodedURI); - final String requestURI = new ProxyRequestHelper().buildZuulRequestURI(request); + final String requestURI = new ProxyRequestHelper(new ZuulProperties()).buildZuulRequestURI(request); assertThat(requestURI, equalTo(encodedURI)); } @@ -378,7 +403,7 @@ public void getUTF8Url() { RequestContext context = RequestContext.getCurrentContext(); context.set(REQUEST_URI_KEY, requestURI); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); String uri = helper.buildZuulRequestURI(request); @@ -394,7 +419,7 @@ public void getDefaultEncodingUrl() { RequestContext context = RequestContext.getCurrentContext(); context.set(REQUEST_URI_KEY, requestURI); - ProxyRequestHelper helper = new ProxyRequestHelper(); + ProxyRequestHelper helper = new ProxyRequestHelper(new ZuulProperties()); String uri = helper.buildZuulRequestURI(request); diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java index 314abf441b..ad10339012 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilterTests.java @@ -28,6 +28,7 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.junit.runner.RunWith; import org.mockito.Mock; import org.springframework.cloud.client.discovery.DiscoveryClient; @@ -35,6 +36,8 @@ import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.cloud.netflix.zuul.filters.ZuulProperties.ZuulRoute; import org.springframework.cloud.netflix.zuul.filters.discovery.DiscoveryClientRouteLocator; +import org.springframework.cloud.test.ClassPathExclusions; +import org.springframework.cloud.test.ModifiedClassPathRunner; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.util.MultiValueMap; @@ -50,6 +53,10 @@ /** * @author Dave Syer */ +@RunWith(ModifiedClassPathRunner.class) +//This is needed for sensitiveHeadersOverrideEmpty, if Spring Security is on the classpath +//then sensitive headers will always be present. +@ClassPathExclusions({"spring-security-*.jar"}) public class PreDecorationFilterTests { private PreDecorationFilter filter; @@ -69,6 +76,7 @@ public class PreDecorationFilterTests { public void init() { initMocks(this); this.properties = new ZuulProperties(); + this.proxyRequestHelper = new ProxyRequestHelper(properties); this.routeLocator = new DiscoveryClientRouteLocator("/", this.discovery, this.properties); this.filter = new PreDecorationFilter(this.routeLocator, "/", this.properties, diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java index 7a783d308e..568fcdb4df 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/RibbonRoutingFilterTests.java @@ -29,6 +29,7 @@ import org.springframework.cloud.netflix.ribbon.support.RibbonCommandContext; import org.springframework.cloud.netflix.ribbon.support.RibbonRequestCustomizer; import org.springframework.cloud.netflix.zuul.filters.ProxyRequestHelper; +import org.springframework.cloud.netflix.zuul.filters.ZuulProperties; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.http.MediaType; @@ -114,7 +115,7 @@ private void setUpRequestContext() { private void setupRibbonRoutingFilter() { RibbonCommandFactory factory = mock(RibbonCommandFactory.class); - filter = new RibbonRoutingFilter(new ProxyRequestHelper(), factory, Collections.emptyList()); + filter = new RibbonRoutingFilter(new ProxyRequestHelper(new ZuulProperties()), factory, Collections.emptyList()); } private ClientHttpResponse createClientHttpResponseWithNonStatus() { diff --git a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java index 3d667623ca..bb5817d776 100644 --- a/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java +++ b/spring-cloud-netflix-zuul/src/test/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilterTests.java @@ -652,7 +652,7 @@ ZuulProperties zuulProperties() { SimpleHostRoutingFilter simpleHostRoutingFilter(ZuulProperties zuulProperties, ApacheHttpClientConnectionManagerFactory connectionManagerFactory, ApacheHttpClientFactory clientFactory) { - return new SimpleHostRoutingFilter(new ProxyRequestHelper(), zuulProperties, connectionManagerFactory, clientFactory); + return new SimpleHostRoutingFilter(new ProxyRequestHelper(zuulProperties), zuulProperties, connectionManagerFactory, clientFactory); } } From cd70fb5023dca9a27a38eb3c640cf31a55023e4c Mon Sep 17 00:00:00 2001 From: durigon Date: Thu, 20 Sep 2018 09:40:39 +0900 Subject: [PATCH 06/11] Refactor String#replaceAll (#3199) If we repeatedly call String#replaceAll, we internally repeatedly call the regular expression pattern compilation every time as following: ```java public String replaceAll(String regex, String replacement) { return Pattern.compile(regex).matcher(this).replaceAll(replacement); } ``` The modifications are to keep the compiled pattern. Therefore, compiling a relatively expensive regular expression pattern does not have to be done every time. --- .../cloud/netflix/zuul/filters/ProxyRequestHelper.java | 9 +++++++-- .../filters/discovery/PatternServiceRouteMapper.java | 4 +++- .../netflix/zuul/filters/pre/PreDecorationFilter.java | 5 ++++- .../zuul/filters/route/SimpleHostRoutingFilter.java | 7 +++++-- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java index dc9c163ec4..0a5fd7cdb9 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/ProxyRequestHelper.java @@ -27,6 +27,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.regex.Pattern; import java.util.Set; import javax.servlet.http.HttpServletRequest; @@ -63,6 +64,10 @@ public class ProxyRequestHelper { */ public static final String IGNORED_HEADERS = "ignoredHeaders"; + public static final Pattern FORM_FEED_PATTERN = Pattern.compile("\f"); + + public static final Pattern COLON_PATTERN = Pattern.compile(":"); + private Set ignoredHeaders = new LinkedHashSet<>(); private Set sensitiveHeaders = new LinkedHashSet<>(); @@ -283,11 +288,11 @@ public String getQueryString(MultiValueMap params) { // if form feed is already part of param name double // since form feed is used as the colon replacement below if (key.contains("\f")) { - key = (key.replaceAll("\f", "\f\f")); + key = (FORM_FEED_PATTERN.matcher(key).replaceAll("\f\f")); } // colon is special to UriTemplate if (key.contains(":")) { - key = key.replaceAll(":", "\f"); + key = COLON_PATTERN.matcher(key).replaceAll("\f"); } key = key + i; singles.put(key, value); diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/discovery/PatternServiceRouteMapper.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/discovery/PatternServiceRouteMapper.java index 1b6de7183f..7e80ca5c86 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/discovery/PatternServiceRouteMapper.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/discovery/PatternServiceRouteMapper.java @@ -21,6 +21,8 @@ */ public class PatternServiceRouteMapper implements ServiceRouteMapper { + private static final Pattern MULTIPLE_SLASH_PATTERN = Pattern.compile("/{2,}"); + /** * A RegExp Pattern that extract needed information from a service ID. Ex : * "(?.*)-(?v.*$)" @@ -60,7 +62,7 @@ public String apply(String serviceId) { * @return */ private String cleanRoute(final String route) { - String routeToClean = route.replaceAll("/{2,}", "/"); + String routeToClean = MULTIPLE_SLASH_PATTERN.matcher(route).replaceAll("/"); if (routeToClean.startsWith("/")) { routeToClean = routeToClean.substring(1); } diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java index 846535713c..5ff66507f1 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/pre/PreDecorationFilter.java @@ -18,6 +18,7 @@ import java.net.MalformedURLException; import java.net.URL; +import java.util.regex.Pattern; import javax.servlet.http.HttpServletRequest; @@ -70,6 +71,8 @@ public class PreDecorationFilter extends ZuulFilter { @Deprecated public static final int FILTER_ORDER = PRE_DECORATION_FILTER_ORDER; + public static final Pattern DOUBLE_SLASH = Pattern.compile("//"); + private RouteLocator routeLocator; private String dispatcherServletPath; @@ -185,7 +188,7 @@ else if (!xforwardedfor.contains(remoteAddr)) { // Prevent duplicates fallBackUri = "/" + fallBackUri; } String forwardURI = fallbackPrefix + fallBackUri; - forwardURI = forwardURI.replaceAll("//", "/"); + forwardURI = DOUBLE_SLASH.matcher(forwardURI).replaceAll("/"); ctx.set(FORWARD_TO_KEY, forwardURI); } return null; diff --git a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java index 4ccc585297..161b8991e0 100644 --- a/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java +++ b/spring-cloud-netflix-zuul/src/main/java/org/springframework/cloud/netflix/zuul/filters/route/SimpleHostRoutingFilter.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.List; import java.util.Map; +import java.util.regex.Pattern; import java.util.Timer; import java.util.TimerTask; @@ -85,6 +86,8 @@ public class SimpleHostRoutingFilter extends ZuulFilter { private static final Log log = LogFactory.getLog(SimpleHostRoutingFilter.class); + private static final Pattern MULTIPLE_SLASH_PATTERN = Pattern.compile("/{2,}"); + private final Timer connectionManagerTimer = new Timer( "SimpleHostRoutingFilter.connectionManagerTimer", true); @@ -286,7 +289,7 @@ private CloseableHttpResponse forward(CloseableHttpClient httpclient, String ver requestEntity); URL host = RequestContext.getCurrentContext().getRouteHost(); HttpHost httpHost = getHttpHost(host); - uri = StringUtils.cleanPath((host.getPath() + uri).replaceAll("/{2,}", "/")); + uri = StringUtils.cleanPath(MULTIPLE_SLASH_PATTERN.matcher(host.getPath() + uri).replaceAll("/")); long contentLength = getContentLength(request); ContentType contentType = null; @@ -449,4 +452,4 @@ protected long getContentLength(HttpServletRequest request) { } return request.getContentLength(); } -} \ No newline at end of file +} From 81579f522faf00bf41ae8d043e0a121b8822adae Mon Sep 17 00:00:00 2001 From: Dennis Effing Date: Wed, 26 Sep 2018 22:22:40 +0200 Subject: [PATCH 07/11] Fix for #1251 (#3211) Example uses the same value of eureka.client.serviceUrl.defaultZone for all peers. The eureka server knows who it is and doesn't attempt to replicate to itself [1], so this simplification will work. [1]https://github.com/Netflix/eureka/blob/v1.4.10/eureka-core/src/main/java/com/netflix/eureka/registry/PeerAwareInstanceRegistryImpl.java#L616-L619 Cherry picked from commit 4b40a45 --- .../main/asciidoc/spring-cloud-netflix.adoc | 35 ++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/src/main/asciidoc/spring-cloud-netflix.adoc b/docs/src/main/asciidoc/spring-cloud-netflix.adoc index 186bec5200..0dc794d790 100644 --- a/docs/src/main/asciidoc/spring-cloud-netflix.adoc +++ b/docs/src/main/asciidoc/spring-cloud-netflix.adoc @@ -507,10 +507,37 @@ on a machine that knows its own hostname (it is looked up using `java.net.InetAddress` by default). You can add multiple peers to a system, and as long as they are all -connected to each other by at least one edge, they will synchronize -the registrations amongst themselves. If the peers are physically -separated (inside a data centre or between multiple data centres) then -the system can in principle survive split-brain type failures. +directly connected to each other, they will synchronize +the registrations amongst themselves. + +.application.yml (Three Peer Aware Eureka Servers) +---- +eureka: + client: + serviceUrl: + defaultZone: http://peer1/eureka/,http://peer2/eureka/,http://peer3/eureka/ + +--- +spring: + profiles: peer1 +eureka: + instance: + hostname: peer1 + +--- +spring: + profiles: peer2 +eureka: + instance: + hostname: peer2 + +--- +spring: + profiles: peer3 +eureka: + instance: + hostname: peer3 +---- === Prefer IP Address From 482deb04859affa881f904378eef34f5e7c02d90 Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 2 Oct 2018 09:43:06 -0400 Subject: [PATCH 08/11] Adds additional-spring-configuration-metadata. Fixes #3164 --- ...itional-spring-configuration-metadata.json | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json diff --git a/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 0000000000..5f29c879ef --- /dev/null +++ b/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,64 @@ +{ + "properties": [ + { + "defaultValue": "true", + "name": "archaius.propagate.environmentChangedEvent", + "description": "Propagates EnvironmentChanged events to Archaius ConfigurationManager.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": true, + "name": "eureka.client.healthcheck.enabled", + "description": "Enables the Eureka health check handler.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": "true", + "name": "management.metrics.binders.hystrix.enabled", + "description": "Enables creation of OK Http Client factory beans.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "hystrix.shareSecurityContext", + "description": "Enables auto-configuration of the Hystrix concurrency strategy plugin hook who will transfer the `SecurityContext` from your main thread to the one used by the Hystrix command.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.restclient.enabled", + "description": "Enables the use of the deprecated Ribbon RestClient.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.http.client.enabled", + "description": "Deprecated property to enable Ribbon RestClient.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": true, + "name": "ribbon.eureka.enabled", + "description": "Enables the use of Eureka with Ribbon.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.okhttp.enabled", + "description": "Enables the use of the OK HTTP Client with Ribbon.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": true, + "name": "turbine.stream.enabled", + "description": "Enables Autoconfiguration for a Spring Cloud Turbine using Spring Cloud Stream.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "zuul.ribbon.eager-load.enabled", + "description": "Enables eager loading of Ribbon clients on startup.", + "type": "java.lang.Boolean" + } + ] +} \ No newline at end of file From a0041e1d63592e175b1a1a5464afc80fb5c5e20a Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 2 Oct 2018 10:05:13 -0400 Subject: [PATCH 09/11] Adding additional-spring-configuration-metadata for different modules --- ...itional-spring-configuration-metadata.json | 10 ++++ ...itional-spring-configuration-metadata.json | 48 ------------------- ...itional-spring-configuration-metadata.json | 16 +++++++ ...itional-spring-configuration-metadata.json | 28 +++++++++++ ...itional-spring-configuration-metadata.json | 22 +++++++++ 5 files changed, 76 insertions(+), 48 deletions(-) create mode 100644 spring-cloud-netflix-archaius/src/main/resources/META-INF/additional-spring-configuration-metadata.json create mode 100644 spring-cloud-netflix-eureka-client/src/main/resources/META-INF/additional-spring-configuration-metadata.json create mode 100644 spring-cloud-netflix-ribbon/src/main/resources/META-INF/additional-spring-configuration-metadata.json create mode 100644 spring-cloud-netflix-zuul/src/main/resources/META-INF/additional-spring-configuration-metadata.json diff --git a/spring-cloud-netflix-archaius/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-netflix-archaius/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 0000000000..0edf137940 --- /dev/null +++ b/spring-cloud-netflix-archaius/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,10 @@ +{ + "properties": [ + { + "defaultValue": "true", + "name": "archaius.propagate.environmentChangedEvent", + "description": "Propagates EnvironmentChanged events to Archaius ConfigurationManager.", + "type": "java.lang.Boolean" + } + ] +} \ No newline at end of file diff --git a/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 5f29c879ef..1dd3a52d7a 100644 --- a/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-cloud-netflix-core/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -1,17 +1,5 @@ { "properties": [ - { - "defaultValue": "true", - "name": "archaius.propagate.environmentChangedEvent", - "description": "Propagates EnvironmentChanged events to Archaius ConfigurationManager.", - "type": "java.lang.Boolean" - }, - { - "defaultValue": true, - "name": "eureka.client.healthcheck.enabled", - "description": "Enables the Eureka health check handler.", - "type": "java.lang.Boolean" - }, { "defaultValue": "true", "name": "management.metrics.binders.hystrix.enabled", @@ -23,42 +11,6 @@ "name": "hystrix.shareSecurityContext", "description": "Enables auto-configuration of the Hystrix concurrency strategy plugin hook who will transfer the `SecurityContext` from your main thread to the one used by the Hystrix command.", "type": "java.lang.Boolean" - }, - { - "defaultValue": false, - "name": "ribbon.restclient.enabled", - "description": "Enables the use of the deprecated Ribbon RestClient.", - "type": "java.lang.Boolean" - }, - { - "defaultValue": false, - "name": "ribbon.http.client.enabled", - "description": "Deprecated property to enable Ribbon RestClient.", - "type": "java.lang.Boolean" - }, - { - "defaultValue": true, - "name": "ribbon.eureka.enabled", - "description": "Enables the use of Eureka with Ribbon.", - "type": "java.lang.Boolean" - }, - { - "defaultValue": false, - "name": "ribbon.okhttp.enabled", - "description": "Enables the use of the OK HTTP Client with Ribbon.", - "type": "java.lang.Boolean" - }, - { - "defaultValue": true, - "name": "turbine.stream.enabled", - "description": "Enables Autoconfiguration for a Spring Cloud Turbine using Spring Cloud Stream.", - "type": "java.lang.Boolean" - }, - { - "defaultValue": false, - "name": "zuul.ribbon.eager-load.enabled", - "description": "Enables eager loading of Ribbon clients on startup.", - "type": "java.lang.Boolean" } ] } \ No newline at end of file diff --git a/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 0000000000..aea1bb6497 --- /dev/null +++ b/spring-cloud-netflix-eureka-client/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,16 @@ +{ + "properties": [ + { + "defaultValue": true, + "name": "eureka.client.healthcheck.enabled", + "description": "Enables the Eureka health check handler.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": true, + "name": "ribbon.eureka.enabled", + "description": "Enables the use of Eureka with Ribbon.", + "type": "java.lang.Boolean" + } + ] +} \ No newline at end of file diff --git a/spring-cloud-netflix-ribbon/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-netflix-ribbon/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 0000000000..a98ad55cff --- /dev/null +++ b/spring-cloud-netflix-ribbon/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,28 @@ +{ + "properties": [ + { + "defaultValue": true, + "name": "eureka.client.healthcheck.enabled", + "description": "Enables the Eureka health check handler.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.restclient.enabled", + "description": "Enables the use of the deprecated Ribbon RestClient.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.http.client.enabled", + "description": "Deprecated property to enable Ribbon RestClient.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.okhttp.enabled", + "description": "Enables the use of the OK HTTP Client with Ribbon.", + "type": "java.lang.Boolean" + } + ] +} \ No newline at end of file diff --git a/spring-cloud-netflix-zuul/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-cloud-netflix-zuul/src/main/resources/META-INF/additional-spring-configuration-metadata.json new file mode 100644 index 0000000000..61ec669690 --- /dev/null +++ b/spring-cloud-netflix-zuul/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -0,0 +1,22 @@ +{ + "properties": [ + { + "defaultValue": false, + "name": "ribbon.restclient.enabled", + "description": "Enables the use of the deprecated Ribbon RestClient.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "ribbon.okhttp.enabled", + "description": "Enables the use of the OK HTTP Client with Ribbon.", + "type": "java.lang.Boolean" + }, + { + "defaultValue": false, + "name": "zuul.ribbon.eager-load.enabled", + "description": "Enables eager loading of Ribbon clients on startup.", + "type": "java.lang.Boolean" + } + ] +} \ No newline at end of file From 8125edd772be840bd2e64dde28992f2e3cfefb0e Mon Sep 17 00:00:00 2001 From: Ryan Baxter Date: Tue, 2 Oct 2018 14:34:36 -0400 Subject: [PATCH 10/11] Removing test that does not belong in 2.0.x branch --- .../feign/ribbon/FeignRibbonClientTests.java | 140 ------------------ 1 file changed, 140 deletions(-) delete mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java deleted file mode 100644 index aadd8a0ce9..0000000000 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/feign/ribbon/FeignRibbonClientTests.java +++ /dev/null @@ -1,140 +0,0 @@ -/* - * Copyright 2013-2015 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.cloud.netflix.feign.ribbon; - -import com.netflix.client.config.CommonClientConfigKey; -import com.netflix.client.config.DefaultClientConfigImpl; -import com.netflix.client.config.IClientConfig; -import com.netflix.loadbalancer.AbstractLoadBalancer; -import com.netflix.loadbalancer.ILoadBalancer; -import com.netflix.loadbalancer.LoadBalancerStats; -import com.netflix.loadbalancer.Server; -import com.netflix.loadbalancer.ServerStats; -import feign.Client; -import feign.Request; -import feign.Request.Options; -import feign.RequestTemplate; -import org.hamcrest.CustomMatcher; -import org.junit.Before; -import org.junit.Test; -import org.springframework.cloud.netflix.ribbon.DefaultServerIntrospector; -import org.springframework.cloud.netflix.ribbon.RibbonLoadBalancedRetryPolicyFactory; -import org.springframework.cloud.netflix.ribbon.ServerIntrospector; -import org.springframework.cloud.netflix.ribbon.SpringClientFactory; - -import static org.mockito.Matchers.any; -import static org.mockito.Matchers.argThat; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -/** - * @author Dave Syer - * @author Spencer Gibb - */ -public class FeignRibbonClientTests { - - private AbstractLoadBalancer loadBalancer = mock(AbstractLoadBalancer.class); - private Client delegate = mock(Client.class); - private RibbonLoadBalancedRetryPolicyFactory retryPolicyFactory = mock(RibbonLoadBalancedRetryPolicyFactory.class); - - private SpringClientFactory factory = new SpringClientFactory() { - @Override - public IClientConfig getClientConfig(String name) { - DefaultClientConfigImpl config = new DefaultClientConfigImpl(); - config.set(CommonClientConfigKey.ConnectTimeout, 1000); - config.set(CommonClientConfigKey.ReadTimeout, 500); - return config; - } - - @Override - public C getInstance(String name, Class type) { - if (type.isAssignableFrom(ServerIntrospector.class)) { - @SuppressWarnings("unchecked") - C instance = (C) new DefaultServerIntrospector(); - return instance; - } - return null; - } - - @Override - public ILoadBalancer getLoadBalancer(String name) { - return FeignRibbonClientTests.this.loadBalancer; - } - }; - - // Even though we don't maintain FeignRibbonClient, keep these tests - // around to make sure the expected behaviour doesn't break - private Client client = new LoadBalancerFeignClient(this.delegate, new CachingSpringLoadBalancerFactory(this.factory, - retryPolicyFactory), this.factory); - - @Before - public void init() { - when(this.loadBalancer.chooseServer(any())).thenReturn( - new Server("foo.com", 8000)); - //to fix NPE - LoadBalancerStats stats = mock(LoadBalancerStats.class); - when(this.loadBalancer.getLoadBalancerStats()).thenReturn(stats); - when(stats.getSingleServerStat(any(Server.class))).thenReturn(mock(ServerStats.class)); - } - - @Test - public void remoteRequestIsSent() throws Exception { - Request request = new RequestTemplate().method("GET").append("http://foo/") - .request(); - this.client.execute(request, new Options()); - RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/"); - verify(this.delegate).execute(argThat(matcher), - any(Options.class)); - } - - @Test - public void remoteRequestIsSentAtRoot() throws Exception { - Request request = new RequestTemplate().method("GET").append("http://foo") - .request(); - this.client.execute(request, new Options()); - RequestMatcher matcher = new RequestMatcher("http://foo.com:8000/"); - verify(this.delegate).execute(argThat(matcher), - any(Options.class)); - } - - @Test - public void remoteRequestIsSecure() throws Exception { - Request request = new RequestTemplate().method("GET").append("https://foo/") - .request(); - this.client.execute(request, new Options()); - RequestMatcher matcher = new RequestMatcher("https://foo.com:8000/"); - verify(this.delegate).execute(argThat(matcher), - any(Options.class)); - } - - private final static class RequestMatcher extends CustomMatcher { - private String url; - - private RequestMatcher(String url) { - super("request has URI: " + url); - this.url = url; - } - - @Override - public boolean matches(Object item) { - Request request = (Request) item; - return request.url().equals(this.url); - } - } - -} From e814cdabb60a0890002d90576e8ce327f368f19a Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 5 Oct 2018 17:17:56 -0400 Subject: [PATCH 11/11] Adds HystrixCommands.fallback(Function>) fixes gh-3210 --- .../cloud/netflix/hystrix/HystrixCommands.java | 14 ++++++++++---- .../netflix/hystrix/HystrixCommandsTests.java | 15 +++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixCommands.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixCommands.java index 763442dd17..8d94a7722d 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixCommands.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/hystrix/HystrixCommands.java @@ -48,7 +48,7 @@ public static class PublisherBuilder { private final Publisher publisher; private String commandName; private String groupName; - private Publisher fallback; + private Function> fallback; private Setter setter; private HystrixCommandProperties.Setter commandProperties; private boolean eager = false; @@ -69,6 +69,11 @@ public PublisherBuilder groupName(String groupName) { } public PublisherBuilder fallback(Publisher fallback) { + this.fallback = throwable -> fallback; + return this; + } + + public PublisherBuilder fallback(Function> fallback) { this.fallback = fallback; return this; } @@ -166,10 +171,10 @@ public Mono toMono() { private static class PublisherHystrixCommand extends HystrixObservableCommand { private Publisher publisher; - private Publisher fallback; + private Function> fallback; protected PublisherHystrixCommand(Setter setter, Publisher publisher, - Publisher fallback) { + Function> fallback) { super(setter); this.publisher = publisher; this.fallback = fallback; @@ -183,7 +188,8 @@ protected Observable construct() { @Override protected Observable resumeWithFallback() { if (this.fallback != null) { - return RxReactiveStreams.toObservable(this.fallback); + Publisher fallbackPublisher = fallback.apply(getExecutionException()); + return RxReactiveStreams.toObservable(fallbackPublisher); } return super.resumeWithFallback(); } diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixCommandsTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixCommandsTests.java index 698ee8a24c..616a4fc4df 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixCommandsTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/hystrix/HystrixCommandsTests.java @@ -66,6 +66,21 @@ public void monoFallbackWorks() { .verifyComplete(); } + @Test + public void monoFallbackWithExceptionWorks() { + StepVerifier.create(HystrixCommands.from(Mono.error(new IllegalStateException())) + .commandName("failcmd") + .fallback(throwable -> { + if (throwable instanceof IllegalStateException) { + return Mono.just("specificfallback"); + } + return Mono.just("genericfallback"); + }) + .toMono()) + .expectNext("specificfallback") + .verifyComplete(); + } + @Test public void fluxWorks() { StepVerifier.create(HystrixCommands.from( Flux.just("1", "2"))