From a01fc5b7588947efe70e287222f3bf6a895f1504 Mon Sep 17 00:00:00 2001 From: Olivier Bourgain Date: Fri, 4 Aug 2017 21:40:10 +0200 Subject: [PATCH 1/3] Fix check then act issue (#2160) --- .../zuul/filters/pre/FormBodyWrapperFilter.java | 10 +++++++--- 1 file changed, 7 insertions(+), 3 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 175c5cfb..5f1fef74 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 @@ -140,7 +140,7 @@ public class FormBodyWrapperFilter extends ZuulFilter { private HttpServletRequest request; - private byte[] contentData; + private volatile byte[] contentData; private MediaType contentType; @@ -183,6 +183,9 @@ public class FormBodyWrapperFilter extends ZuulFilter { } private synchronized void buildContentData() { + if (this.contentData != null) { + return; + } try { MultiValueMap builder = RequestContentDataExtractor.extract(this.request); FormHttpOutputMessage data = new FormHttpOutputMessage(); @@ -192,8 +195,9 @@ public class FormBodyWrapperFilter extends ZuulFilter { FormBodyWrapperFilter.this.formHttpMessageConverter.write(builder, this.contentType, data); // copy new content type including multipart boundary this.contentType = data.getHeaders().getContentType(); - this.contentData = data.getInput(); - this.contentLength = this.contentData.length; + byte[] input = data.getInput(); + this.contentLength = input.length; + this.contentData = input; } catch (Exception e) { throw new IllegalStateException("Cannot convert form data", e); From 63f975ad436c506354e8c5a8f49c0b1a7637ed51 Mon Sep 17 00:00:00 2001 From: Biju Kunjummen Date: Fri, 4 Aug 2017 12:41:58 -0700 Subject: [PATCH 2/3] Sync up matching of ignored paths and route paths (#2163) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: João Salavessa --- .../netflix/zuul/web/ZuulHandlerMapping.java | 25 +++++++--- .../zuul/web/ZuulHandlerMappingTests.java | 50 ++++++++++++++++--- 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java index c1e6dd54..7a89a12b 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2017 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. @@ -24,7 +24,8 @@ import org.springframework.boot.autoconfigure.web.ErrorController; import org.springframework.cloud.netflix.zuul.filters.RefreshableRouteLocator; import org.springframework.cloud.netflix.zuul.filters.Route; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; -import org.springframework.util.PatternMatchUtils; +import org.springframework.util.AntPathMatcher; +import org.springframework.util.PathMatcher; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping; @@ -36,6 +37,8 @@ import com.netflix.zuul.context.RequestContext; * * @author Spencer Gibb * @author Dave Syer + * @author João Salavessa + * @author Biju Kunjummen */ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { @@ -45,6 +48,8 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { private ErrorController errorController; + private PathMatcher pathMatcher = new AntPathMatcher(); + private volatile boolean dirty = true; public ZuulHandlerMapping(RouteLocator routeLocator, ZuulController zuul) { @@ -79,10 +84,7 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { if (this.errorController != null && urlPath.equals(this.errorController.getErrorPath())) { return null; } - String[] ignored = this.routeLocator.getIgnoredPaths().toArray(new String[0]); - if (PatternMatchUtils.simpleMatch(ignored, urlPath)) { - return null; - } + if (isIgnoredPath(urlPath, this.routeLocator.getIgnoredPaths())) return null; RequestContext ctx = RequestContext.getCurrentContext(); if (ctx.containsKey("forward.to")) { return null; @@ -98,6 +100,17 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { return super.lookupHandler(urlPath, request); } + private boolean isIgnoredPath(String urlPath, Collection ignored) { + if (ignored != null) { + for (String ignoredPath : ignored) { + if (this.pathMatcher.match(ignoredPath, urlPath)) { + return true; + } + } + } + return false; + } + private void registerHandlers() { Collection routes = this.routeLocator.getRoutes(); if (routes.isEmpty()) { diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java index 4205f79a..12bef079 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2015 the original author or authors. + * Copyright 2013-2017 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. @@ -16,7 +16,11 @@ package org.springframework.cloud.netflix.zuul.web; +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Arrays; import java.util.Collections; +import java.util.List; import org.junit.Before; import org.junit.Test; @@ -28,11 +32,9 @@ import org.springframework.mock.web.MockHttpServletRequest; import com.netflix.zuul.context.RequestContext; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; - /** * @author Dave Syer + * @author Biju Kunjummen */ public class ZuulHandlerMappingTests { @@ -58,7 +60,7 @@ public class ZuulHandlerMappingTests { .singletonList(new Route("foo", "/foo/**", "foo", "", null, null))); this.request.setServletPath("/foo/"); this.mapping.setDirty(true); - assertNotNull(this.mapping.getHandler(this.request)); + assertThat(this.mapping.getHandler(this.request)).isNotNull(); } @Test @@ -68,7 +70,7 @@ public class ZuulHandlerMappingTests { ; this.request.setServletPath("/"); this.mapping.setDirty(true); - assertNotNull(this.mapping.getHandler(this.request)); + assertThat(this.mapping.getHandler(this.request)).isNotNull(); } @Test @@ -77,7 +79,41 @@ public class ZuulHandlerMappingTests { .singletonList(new Route("default", "/**", "foo", "", null, null))); this.request.setServletPath("/error"); this.mapping.setDirty(true); - assertNull(this.mapping.getHandler(this.request)); + assertThat(this.mapping.getHandler(this.request)).isNull(); + } + + @Test + public void ignoredPathsShouldNotReturnAHandler() throws Exception { + assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**"), + new Route("p1", "/p1/**", "p1", "", null, null)) + .getHandler(requestForAPath("/p1"))).isNull(); + + assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**/p3/"), + new Route("p1", "/p1/**/p3", "p1", "", null, null)) + .getHandler(requestForAPath("/p1/p2/p3"))).isNull(); + + assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**/p3/**"), + new Route("p1", "/p1/**/p3", "p1", "", null, null)) + .getHandler(requestForAPath("/p1/p2/p3"))).isNull(); + + assertThat(mappingWithIgnoredPathsAndRoutes(Arrays.asList("/p1/**/p4/"), + new Route("p1", "/p1/**/p4/", "p1", "", null, null)) + .getHandler(requestForAPath("/p1/p2/p3/p4"))).isNull(); + } + + private ZuulHandlerMapping mappingWithIgnoredPathsAndRoutes(List ignoredPaths, Route route) { + RouteLocator routeLocator = Mockito.mock(RouteLocator.class); + Mockito.when(routeLocator.getIgnoredPaths()) + .thenReturn(ignoredPaths); + Mockito.when(routeLocator.getRoutes()).thenReturn(Collections.singletonList(route)); + ZuulHandlerMapping zuulHandlerMapping = new ZuulHandlerMapping(routeLocator, new ZuulController()); + return zuulHandlerMapping; + } + + private MockHttpServletRequest requestForAPath(String path) { + MockHttpServletRequest request = new MockHttpServletRequest(); + request.setServletPath(path); + return request; } } From e4e29e131615e7aa141cc386414bb128168dfff5 Mon Sep 17 00:00:00 2001 From: Spencer Gibb Date: Fri, 4 Aug 2017 14:37:24 -0600 Subject: [PATCH 3/3] Move tests out of default and base package --- .../ApacheHttpClientConfigurationTests.java | 27 ++++++++--------- .../test}/OkHttpClientConfigurationTests.java | 30 +++++++++---------- 2 files changed, 28 insertions(+), 29 deletions(-) rename spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/{ => test}/ApacheHttpClientConfigurationTests.java (91%) rename spring-cloud-netflix-core/src/test/java/{ => org/springframework/cloud/netflix/test}/OkHttpClientConfigurationTests.java (88%) diff --git a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/test/ApacheHttpClientConfigurationTests.java similarity index 91% rename from spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java rename to spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/test/ApacheHttpClientConfigurationTests.java index dc27ce47..82ef658e 100644 --- a/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/ApacheHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/test/ApacheHttpClientConfigurationTests.java @@ -1,22 +1,21 @@ /* + * Copyright 2013-2017 the original author or authors. * - * * Copyright 2013-2016 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. + * 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; +package org.springframework.cloud.netflix.test; import feign.Client; import feign.httpclient.ApacheHttpClient; diff --git a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/test/OkHttpClientConfigurationTests.java similarity index 88% rename from spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java rename to spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/test/OkHttpClientConfigurationTests.java index e9d58e06..a75f24e3 100644 --- a/spring-cloud-netflix-core/src/test/java/OkHttpClientConfigurationTests.java +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/test/OkHttpClientConfigurationTests.java @@ -1,21 +1,22 @@ /* + * Copyright 2013-2017 the original author or authors. * - * * Copyright 2013-2016 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. + * 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.test; + import feign.Client; import okhttp3.ConnectionPool; import okhttp3.OkHttpClient; @@ -23,8 +24,7 @@ import okhttp3.OkHttpClient; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.concurrent.TimeUnit; -import javax.net.ssl.SSLSocketFactory; -import javax.net.ssl.X509TrustManager; + import org.assertj.core.api.Assertions; import org.junit.Test; import org.junit.runner.RunWith;