From c6366baee232686fd584c8765f00128e7b1b3444 Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Fri, 19 Aug 2016 11:12:38 -0500 Subject: [PATCH] Remove MvcRequestMatcher.afterPropertiesSet() The validation does not work due to restrictions within the servlet container. Specifically we cannot access the servlets that are registered. This commit reverts the validation logic for MvcRequestMatcher to determine if servletPath is required. Fixes gh-4027 --- .../http/InterceptUrlConfigTests.groovy | 34 ++----- .../AbstractRequestMatcherRegistryTests.java | 89 ------------------- .../configurers/AuthorizeRequestsTests.java | 23 ----- .../util/matcher/MvcRequestMatcher.java | 64 +++---------- 4 files changed, 15 insertions(+), 195 deletions(-) delete mode 100644 config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java diff --git a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy index 7df41dcbfe..457f09da67 100644 --- a/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy +++ b/config/src/test/groovy/org/springframework/security/config/http/InterceptUrlConfigTests.groovy @@ -15,9 +15,13 @@ */ package org.springframework.security.config.http +import javax.servlet.ServletContext +import javax.servlet.ServletRegistration +import javax.servlet.http.HttpServletResponse + import org.mockito.invocation.InvocationOnMock import org.mockito.stubbing.Answer -import org.springframework.beans.factory.BeanCreationException + import org.springframework.beans.factory.parsing.BeanDefinitionParsingException import org.springframework.mock.web.MockFilterChain import org.springframework.mock.web.MockHttpServletRequest @@ -29,10 +33,6 @@ import org.springframework.security.web.access.intercept.FilterSecurityIntercept import org.springframework.web.bind.annotation.RequestMapping import org.springframework.web.bind.annotation.RestController -import javax.servlet.ServletContext -import javax.servlet.ServletRegistration -import javax.servlet.http.HttpServletResponse - import static org.mockito.Mockito.* /** * @@ -313,30 +313,6 @@ class InterceptUrlConfigTests extends AbstractHttpConfigTests { response.status == HttpServletResponse.SC_UNAUTHORIZED } - def "intercept-url mvc matchers servlet path required"() { - when: - MockServletContext servletContext = mockServletContext("/spring"); - xml.http('request-matcher':'mvc') { - 'http-basic'() - 'intercept-url'(pattern: '/path', access: "denyAll") - } - createWebAppContext(servletContext) - then: - thrown(BeanCreationException) - } - - def "intercept-url mvc matchers servlet path NOT required"() { - when: - MockServletContext servletContext = mockServletContext(); - xml.http('request-matcher':'mvc') { - 'http-basic'() - 'intercept-url'(pattern: '/path', access: "denyAll") - } - createWebAppContext(servletContext) - then: - noExceptionThrown() - } - def "intercept-url ant matcher with servlet path fails"() { when: xml.http('request-matcher':'ant') { diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java deleted file mode 100644 index e20bee96d3..0000000000 --- a/config/src/test/java/org/springframework/security/config/annotation/web/AbstractRequestMatcherRegistryTests.java +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright 2012-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. - */ - -package org.springframework.security.config.annotation.web; - -import org.junit.Before; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.mockito.Mock; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.runners.MockitoJUnitRunner; -import org.mockito.stubbing.Answer; -import org.springframework.mock.web.MockServletContext; -import org.springframework.security.web.servlet.util.matcher.MvcRequestMatcher; -import org.springframework.web.servlet.handler.HandlerMappingIntrospector; - -import javax.servlet.ServletContext; -import javax.servlet.ServletRegistration; -import java.util.Arrays; -import java.util.Collections; -import java.util.Map; - -import static org.mockito.Mockito.*; - -/** - * @author Rob Winch - */ -@RunWith(MockitoJUnitRunner.class) -public class AbstractRequestMatcherRegistryTests { - - @Mock - HandlerMappingIntrospector introspector; - - MvcRequestMatcher matcher; - - ServletContext servletContext; - - @Before - public void setup() { - servletContext = spy(new MockServletContext()); - matcher = new MvcRequestMatcher(introspector, "/foo"); - matcher.setServletContext(servletContext); - } - - @Test(expected = IllegalStateException.class) - public void servletPathValidatingMvcRequestMatcherAfterPropertiesSetFailsWithSpringServlet() throws Exception { - setMappings("/spring"); - matcher.afterPropertiesSet(); - } - - @Test - public void servletPathValidatingMvcRequestMatcherAfterPropertiesSetWithSpringServlet() throws Exception { - matcher.setServletPath("/spring"); - setMappings("/spring"); - matcher.afterPropertiesSet(); - } - - @Test - public void servletPathValidatingMvcRequestMatcherAfterPropertiesSetDefaultServlet() throws Exception { - setMappings("/"); - matcher.afterPropertiesSet(); - } - - private void setMappings(String... mappings) { - final ServletRegistration registration = mock(ServletRegistration.class); - when(registration.getMappings()).thenReturn(Arrays.asList(mappings)); - Answer> answer = new Answer>() { - @Override - public Map answer(InvocationOnMock invocation) throws Throwable { - return Collections.singletonMap("spring", registration); - } - }; - when(servletContext.getServletRegistrations()).thenAnswer(answer); - } - -} diff --git a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java index f6363423ea..e05f6e7a01 100644 --- a/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java +++ b/config/src/test/java/org/springframework/security/config/annotation/web/configurers/AuthorizeRequestsTests.java @@ -15,19 +15,12 @@ */ package org.springframework.security.config.annotation.web.configurers; -import java.util.Collections; -import java.util.Map; - -import javax.servlet.ServletRegistration; import javax.servlet.http.HttpServletResponse; import org.junit.After; import org.junit.Before; import org.junit.Test; -import org.mockito.invocation.InvocationOnMock; -import org.mockito.stubbing.Answer; -import org.springframework.beans.factory.BeanCreationException; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; @@ -55,9 +48,7 @@ import org.springframework.web.context.support.AnnotationConfigWebApplicationCon import org.springframework.web.servlet.config.annotation.EnableWebMvc; import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.when; /** * @author Rob Winch @@ -450,20 +441,6 @@ public class AuthorizeRequestsTests { } } - @Test(expected = BeanCreationException.class) - public void mvcMatcherServletPathRequired() throws Exception { - final ServletRegistration registration = mock(ServletRegistration.class); - when(registration.getMappings()).thenReturn(Collections.singleton("/spring")); - Answer> answer = new Answer>() { - @Override - public Map answer(InvocationOnMock invocation) throws Throwable { - return Collections.singletonMap("spring", registration); - } - }; - when(servletContext.getServletRegistrations()).thenAnswer(answer); - loadConfig(MvcMatcherPathServletPathRequiredConfig.class); - } - @EnableWebSecurity @Configuration @EnableWebMvc diff --git a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java index 75a5c7cf3a..5e7b63ec6e 100644 --- a/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java +++ b/web/src/main/java/org/springframework/security/web/servlet/util/matcher/MvcRequestMatcher.java @@ -16,27 +16,21 @@ package org.springframework.security.web.servlet.util.matcher; -import org.springframework.beans.factory.InitializingBean; +import java.util.Collections; +import java.util.Map; + +import javax.servlet.http.HttpServletRequest; + import org.springframework.http.HttpMethod; -import org.springframework.security.web.access.intercept.FilterInvocationSecurityMetadataSource; import org.springframework.security.web.util.matcher.RequestMatcher; import org.springframework.security.web.util.matcher.RequestVariablesExtractor; import org.springframework.util.AntPathMatcher; -import org.springframework.util.ClassUtils; import org.springframework.util.PathMatcher; -import org.springframework.web.context.ServletContextAware; import org.springframework.web.servlet.handler.HandlerMappingIntrospector; import org.springframework.web.servlet.handler.MatchableHandlerMapping; import org.springframework.web.servlet.handler.RequestMatchResult; import org.springframework.web.util.UrlPathHelper; -import javax.servlet.ServletContext; -import javax.servlet.ServletRegistration; -import javax.servlet.http.HttpServletRequest; -import java.util.Collection; -import java.util.Collections; -import java.util.Map; - /** * A {@link RequestMatcher} that uses Spring MVC's {@link HandlerMappingIntrospector} to * match the path and extract variables. @@ -44,18 +38,14 @@ import java.util.Map; *

* It is important to understand that Spring MVC's matching is relative to the servlet * path. This means if you have mapped any servlet to a path that starts with "/" and is - * greater than one, you should also specify the {@link #setServletPath(String)} - * attribute to differentiate mappings. + * greater than one, you should also specify the {@link #setServletPath(String)} attribute + * to differentiate mappings. *

* * @author Rob Winch * @since 4.1.1 */ -public class MvcRequestMatcher - implements RequestMatcher, RequestVariablesExtractor, InitializingBean, ServletContextAware { - - private static final boolean isServlet30 = ClassUtils.isPresent( - "javax.servlet.ServletRegistration", MvcRequestMatcher.class.getClassLoader()); +public class MvcRequestMatcher implements RequestMatcher, RequestVariablesExtractor { private final DefaultMatcher defaultMatcher = new DefaultMatcher(); @@ -63,7 +53,6 @@ public class MvcRequestMatcher private final String pattern; private HttpMethod method; private String servletPath; - private ServletContext servletContext; public MvcRequestMatcher(HandlerMappingIntrospector introspector, String pattern) { this.introspector = introspector; @@ -75,7 +64,8 @@ public class MvcRequestMatcher if (this.method != null && !this.method.name().equals(request.getMethod())) { return false; } - if (this.servletPath != null && !this.servletPath.equals(request.getServletPath())) { + if (this.servletPath != null + && !this.servletPath.equals(request.getServletPath())) { return false; } MatchableHandlerMapping mapping = getMapping(request); @@ -112,40 +102,6 @@ public class MvcRequestMatcher : result.extractUriTemplateVariables(); } - /* (non-Javadoc) - * @see org.springframework.beans.factory.InitializingBean#afterPropertiesSet() - */ - @Override - public void afterPropertiesSet() throws Exception { - // servletPath is required when at least one registered Servlet - // is mapped to a path other than the default (root) path '/' - if (this.servletContext == null || !isServlet30) { - return; - } - if (this.getServletPath() != null) { - return; - } - for (ServletRegistration registration : this.servletContext.getServletRegistrations().values()) { - for (String mapping : registration.getMappings()) { - if (mapping.startsWith("/") && mapping.length() > 1) { - throw new IllegalStateException( - "servletPath must not be null for mvcPattern \"" + this.getMvcPattern() - + "\" when providing a servlet mapping of " - + mapping + " for servlet " - + registration.getClassName()); - } - } - } - } - - /* (non-Javadoc) - * @see org.springframework.web.context.ServletContextAware#setServletContext(javax.servlet.ServletContext) - */ - @Override - public void setServletContext(ServletContext servletContext) { - this.servletContext = servletContext; - } - /** * @param method the method to set */