diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java index a87a8cc6bcfa..b5cebbec60f9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/DispatcherServlet.java @@ -969,7 +969,9 @@ protected void doService(HttpServletRequest request, HttpServletResponse respons restoreAttributesAfterInclude(request, attributesSnapshot); } } - ServletRequestPathUtils.setParsedRequestPath(previousRequestPath, request); + if (this.parseRequestPath) { + ServletRequestPathUtils.setParsedRequestPath(previousRequestPath, request); + } } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java index 98c9f848ec2a..c696d43f6698 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Properties; import java.util.concurrent.ConcurrentHashMap; +import java.util.function.BiFunction; import java.util.stream.Collectors; import javax.servlet.http.HttpServletRequest; @@ -36,6 +37,7 @@ import org.springframework.core.io.ClassPathResource; import org.springframework.core.io.Resource; import org.springframework.core.io.support.PropertiesLoaderUtils; +import org.springframework.http.server.RequestPath; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.ClassUtils; @@ -46,6 +48,7 @@ import org.springframework.web.servlet.HandlerExecutionChain; import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.HandlerMapping; +import org.springframework.web.util.ServletRequestPathUtils; import org.springframework.web.util.UrlPathHelper; /** @@ -79,8 +82,7 @@ public class HandlerMappingIntrospector private List handlerMappings; @Nullable - private Map pathPatternMatchableHandlerMappings = - new ConcurrentHashMap<>(); + private Map pathPatternHandlerMappings = new ConcurrentHashMap<>(); /** @@ -119,7 +121,7 @@ public void afterPropertiesSet() { if (this.handlerMappings == null) { Assert.notNull(this.applicationContext, "No ApplicationContext"); this.handlerMappings = initHandlerMappings(this.applicationContext); - this.pathPatternMatchableHandlerMappings = initPathPatternMatchableHandlerMappings(this.handlerMappings); + this.pathPatternHandlerMappings = initPathPatternMatchableHandlerMappings(this.handlerMappings); } } @@ -136,51 +138,90 @@ public void afterPropertiesSet() { */ @Nullable public MatchableHandlerMapping getMatchableHandlerMapping(HttpServletRequest request) throws Exception { - Assert.notNull(this.handlerMappings, "Handler mappings not initialized"); - Assert.notNull(this.pathPatternMatchableHandlerMappings, "Handler mappings with PathPatterns not initialized"); - HttpServletRequest wrapper = new RequestAttributeChangeIgnoringWrapper(request); - for (HandlerMapping handlerMapping : this.handlerMappings) { - Object handler = handlerMapping.getHandler(wrapper); - if (handler == null) { - continue; - } - if (handlerMapping instanceof MatchableHandlerMapping) { - return this.pathPatternMatchableHandlerMappings.getOrDefault( - handlerMapping, (MatchableHandlerMapping) handlerMapping); + HttpServletRequest wrappedRequest = new RequestAttributeChangeIgnoringWrapper(request); + return doWithMatchingMapping(wrappedRequest, false, (matchedMapping, executionChain) -> { + if (matchedMapping instanceof MatchableHandlerMapping) { + PathPatternMatchableHandlerMapping mapping = this.pathPatternHandlerMappings.get(matchedMapping); + if (mapping != null) { + RequestPath requestPath = ServletRequestPathUtils.getParsedRequestPath(request); + return mapping.decorate(requestPath); + } + else { + return (MatchableHandlerMapping) matchedMapping; + } } throw new IllegalStateException("HandlerMapping is not a MatchableHandlerMapping"); - } - return null; + }); } @Override @Nullable public CorsConfiguration getCorsConfiguration(HttpServletRequest request) { - Assert.notNull(this.handlerMappings, "Handler mappings not initialized"); - RequestAttributeChangeIgnoringWrapper wrapper = new RequestAttributeChangeIgnoringWrapper(request); - for (HandlerMapping handlerMapping : this.handlerMappings) { - HandlerExecutionChain handler = null; - try { - handler = handlerMapping.getHandler(wrapper); - } - catch (Exception ex) { - // Ignore + RequestAttributeChangeIgnoringWrapper wrappedRequest = new RequestAttributeChangeIgnoringWrapper(request); + return doWithMatchingMappingIgnoringException(wrappedRequest, (handlerMapping, executionChain) -> { + for (HandlerInterceptor interceptor : executionChain.getInterceptorList()) { + if (interceptor instanceof CorsConfigurationSource) { + return ((CorsConfigurationSource) interceptor).getCorsConfiguration(wrappedRequest); + } } - if (handler == null) { - continue; + if (executionChain.getHandler() instanceof CorsConfigurationSource) { + return ((CorsConfigurationSource) executionChain.getHandler()).getCorsConfiguration(wrappedRequest); } - for (HandlerInterceptor interceptor : handler.getInterceptorList()) { - if (interceptor instanceof CorsConfigurationSource) { - return ((CorsConfigurationSource) interceptor).getCorsConfiguration(wrapper); + return null; + }); + } + + @Nullable + private T doWithMatchingMapping( + HttpServletRequest request, boolean ignoreException, + BiFunction matchHandler) throws Exception { + + Assert.notNull(this.handlerMappings, "Handler mappings not initialized"); + Assert.notNull(this.pathPatternHandlerMappings, "Handler mappings with PathPatterns not initialized"); + + boolean parseRequestPath = !this.pathPatternHandlerMappings.isEmpty(); + RequestPath previousPath = null; + if (parseRequestPath) { + previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE); + ServletRequestPathUtils.parseAndCache(request); + } + try { + for (HandlerMapping handlerMapping : this.handlerMappings) { + HandlerExecutionChain chain = null; + try { + chain = handlerMapping.getHandler(request); + } + catch (Exception ex) { + if (!ignoreException) { + throw ex; + } } + if (chain == null) { + continue; + } + return matchHandler.apply(handlerMapping, chain); } - if (handler.getHandler() instanceof CorsConfigurationSource) { - return ((CorsConfigurationSource) handler.getHandler()).getCorsConfiguration(wrapper); + } + finally { + if (parseRequestPath) { + ServletRequestPathUtils.setParsedRequestPath(previousPath, request); } } return null; } + @Nullable + private T doWithMatchingMappingIgnoringException( + HttpServletRequest request, BiFunction matchHandler) { + + try { + return doWithMatchingMapping(request, true, matchHandler); + } + catch (Exception ex) { + throw new IllegalStateException("HandlerMapping exception not suppressed", ex); + } + } + private static List initHandlerMappings(ApplicationContext applicationContext) { Map beans = BeanFactoryUtils.beansOfTypeIncludingAncestors( @@ -219,7 +260,7 @@ private static List initFallback(ApplicationContext applicationC return result; } - private static Map initPathPatternMatchableHandlerMappings( + private static Map initPathPatternMatchableHandlerMappings( List mappings) { return mappings.stream() @@ -241,8 +282,7 @@ private static class RequestAttributeChangeIgnoringWrapper extends HttpServletRe @Override public void setAttribute(String name, Object value) { - // Allow UrlPathHelper-resolved lookupPath to be saved for efficiency - if (name.equals(UrlPathHelper.PATH_ATTRIBUTE)) { + if (name.equals(ServletRequestPathUtils.PATH_ATTRIBUTE) || name.equals(UrlPathHelper.PATH_ATTRIBUTE)) { super.setAttribute(name, value); } } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java index 3a832b001d1b..9d53ce0e09e9 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -21,6 +21,7 @@ import javax.servlet.http.HttpServletRequest; import org.springframework.http.server.PathContainer; +import org.springframework.http.server.RequestPath; import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.web.servlet.HandlerExecutionChain; @@ -70,4 +71,39 @@ public RequestMatchResult match(HttpServletRequest request, String pattern) { public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception { return this.delegate.getHandler(request); } + + MatchableHandlerMapping decorate(RequestPath requestPath) { + return new MatchableHandlerMapping() { + + @Nullable + @Override + public RequestMatchResult match(HttpServletRequest request, String pattern) { + RequestPath previousPath = setRequestPathAttribute(request); + try { + return PathPatternMatchableHandlerMapping.this.match(request, pattern); + } + finally { + ServletRequestPathUtils.setParsedRequestPath(previousPath, request); + } + } + + @Nullable + @Override + public HandlerExecutionChain getHandler(HttpServletRequest request) throws Exception { + RequestPath previousPath = setRequestPathAttribute(request); + try { + return PathPatternMatchableHandlerMapping.this.getHandler(request); + } + finally { + ServletRequestPathUtils.setParsedRequestPath(previousPath, request); + } + } + + private RequestPath setRequestPathAttribute(HttpServletRequest request) { + RequestPath previous = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE); + ServletRequestPathUtils.setParsedRequestPath(requestPath, request); + return previous; + } + }; + } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingIntrospectorTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingIntrospectorTests.java index c6d03c054a3a..f7437ef77d3c 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingIntrospectorTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMappingIntrospectorTests.java @@ -127,16 +127,11 @@ void getMatchable(boolean usePathPatterns) throws Exception { context.refresh(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123"); - - // Initialize the RequestPath. At runtime, ServletRequestPathFilter is expected to do that. - if (usePathPatterns) { - ServletRequestPathUtils.parseAndCache(request); - } - MatchableHandlerMapping mapping = initIntrospector(context).getMatchableHandlerMapping(request); assertThat(mapping).isNotNull(); assertThat(request.getAttribute(BEST_MATCHING_PATTERN_ATTRIBUTE)).as("Attribute changes not ignored").isNull(); + assertThat(request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE)).as("Parsed path not cleaned").isNull(); assertThat(mapping.match(request, "/p*/*")).isNotNull(); assertThat(mapping.match(request, "/b*/*")).isNull();