From 5961931caf52b6da52f9fa491004df3f169ab800 Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Wed, 1 Apr 2015 11:50:33 +0100 Subject: [PATCH] Make /error path a special case in ZuulHandlerMapping If there is a default route (/**) then the ZuulHandlerMapping will mask the /error path in a normal Spring Boot application. This change makes it a special case so that ZuulHandlerMapping will never map the /error route (the one specified by the ErrorController). Fixes gh-284 --- .../cloud/netflix/zuul/ZuulConfiguration.java | 8 +- .../netflix/zuul/web/ZuulHandlerMapping.java | 21 +++++- .../zuul/web/ZuulHandlerMappingTests.java | 75 +++++++++++++++++++ 3 files changed, 101 insertions(+), 3 deletions(-) create mode 100644 spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java diff --git a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java index a7851d76..63317c59 100644 --- a/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java +++ b/spring-cloud-netflix-core/src/main/java/org/springframework/cloud/netflix/zuul/ZuulConfiguration.java @@ -20,6 +20,7 @@ import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; +import org.springframework.boot.autoconfigure.web.ErrorController; import org.springframework.boot.context.embedded.ServletRegistrationBean; import org.springframework.boot.context.properties.EnableConfigurationProperties; import org.springframework.cloud.context.scope.refresh.RefreshScopeRefreshedEvent; @@ -54,6 +55,9 @@ public class ZuulConfiguration { @Autowired private ZuulProperties zuulProperties; + @Autowired(required = false) + private ErrorController errorController; + @Bean public RouteLocator routeLocator() { return new SimpleRouteLocator(this.zuulProperties); @@ -66,7 +70,9 @@ public class ZuulConfiguration { @Bean public ZuulHandlerMapping zuulHandlerMapping(RouteLocator routes) { - return new ZuulHandlerMapping(routes, zuulController()); + ZuulHandlerMapping mapping = new ZuulHandlerMapping(routes, zuulController()); + mapping.setErrorController(this.errorController); + return mapping; } @Bean 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 c75f84ff..92668599 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 @@ -18,7 +18,9 @@ package org.springframework.cloud.netflix.zuul.web; import java.util.Collection; -import org.springframework.beans.factory.annotation.Autowired; +import javax.servlet.http.HttpServletRequest; + +import org.springframework.boot.autoconfigure.web.ErrorController; import org.springframework.cloud.netflix.zuul.filters.RouteLocator; import org.springframework.web.servlet.handler.AbstractUrlHandlerMapping; @@ -34,13 +36,28 @@ public class ZuulHandlerMapping extends AbstractUrlHandlerMapping { private final ZuulController zuul; - @Autowired + private ErrorController errorController; + public ZuulHandlerMapping(RouteLocator routeLocator, ZuulController zuul) { this.routeLocator = routeLocator; this.zuul = zuul; setOrder(-200); } + public void setErrorController(ErrorController errorController) { + this.errorController = errorController; + } + + @Override + protected Object lookupHandler(String urlPath, HttpServletRequest request) + throws Exception { + if (this.errorController != null + && urlPath.equals(this.errorController.getErrorPath())) { + return null; + } + return super.lookupHandler(urlPath, request); + } + public void registerHandlers() { Collection routes = this.routeLocator.getRoutePaths(); 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 new file mode 100644 index 00000000..777db7a9 --- /dev/null +++ b/spring-cloud-netflix-core/src/test/java/org/springframework/cloud/netflix/zuul/web/ZuulHandlerMappingTests.java @@ -0,0 +1,75 @@ +/* + * 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.zuul.web; + +import java.util.Arrays; + +import org.junit.Before; +import org.junit.Test; +import org.mockito.Mockito; +import org.springframework.boot.autoconfigure.web.ErrorController; +import org.springframework.cloud.netflix.zuul.filters.RouteLocator; +import org.springframework.mock.web.MockHttpServletRequest; + +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + +/** + * @author Dave Syer + */ +public class ZuulHandlerMappingTests { + + private ZuulHandlerMapping mapping; + + private RouteLocator locator = Mockito.mock(RouteLocator.class); + + private ErrorController errors = Mockito.mock(ErrorController.class); + + private MockHttpServletRequest request = new MockHttpServletRequest(); + + @Before + public void init() { + this.mapping = new ZuulHandlerMapping(this.locator, new ZuulController()); + this.mapping.setErrorController(this.errors); + Mockito.when(this.errors.getErrorPath()).thenReturn("/error"); + } + + @Test + public void mappedPath() throws Exception { + Mockito.when(this.locator.getRoutePaths()).thenReturn(Arrays.asList("/foo/**")); + this.request.setServletPath("/foo/"); + this.mapping.registerHandlers(); + assertNotNull(this.mapping.getHandler(this.request)); + } + + @Test + public void defaultPath() throws Exception { + Mockito.when(this.locator.getRoutePaths()).thenReturn(Arrays.asList("/**")); + this.request.setServletPath("/"); + this.mapping.registerHandlers(); + assertNotNull(this.mapping.getHandler(this.request)); + } + + @Test + public void errorPath() throws Exception { + Mockito.when(this.locator.getRoutePaths()).thenReturn(Arrays.asList("/**")); + this.request.setServletPath("/error"); + this.mapping.registerHandlers(); + assertNull(this.mapping.getHandler(this.request)); + } + +}