HandlerMappingIntrospector ensures initialized RequestPath

Closes spring-projectsgh-26814
Zoran0104 · Aug 20, 2021 · 82c0d5a · 82c0d5a
1 parent 74a4d87
commit 82c0d5a
Show file tree

Hide file tree

Showing 4 changed files with 117 additions and 44 deletions.
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
@@ -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/...mvc/src/main/java/org/springframework/web/servlet/handler/HandlerMappingIntrospector.java b/...mvc/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<HandlerMapping> handlerMappings;
 
 	@Nullable
-	private Map<HandlerMapping, MatchableHandlerMapping> pathPatternMatchableHandlerMappings =
-			new ConcurrentHashMap<>();
+	private Map<HandlerMapping, PathPatternMatchableHandlerMapping> 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> T doWithMatchingMapping(
+			HttpServletRequest request, boolean ignoreException,
+			BiFunction<HandlerMapping, HandlerExecutionChain, T> 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> T doWithMatchingMappingIgnoringException(
+			HttpServletRequest request, BiFunction<HandlerMapping, HandlerExecutionChain, T> matchHandler) {
+
+		try {
+			return doWithMatchingMapping(request, true, matchHandler);
+		}
+		catch (Exception ex) {
+			throw new IllegalStateException("HandlerMapping exception not suppressed", ex);
+		}
+	}
+
 
 	private static List<HandlerMapping> initHandlerMappings(ApplicationContext applicationContext) {
 		Map<String, HandlerMapping> beans = BeanFactoryUtils.beansOfTypeIncludingAncestors(
@@ -219,7 +260,7 @@ private static List<HandlerMapping> initFallback(ApplicationContext applicationC
 		return result;
 	}
 
-	private static Map<HandlerMapping, MatchableHandlerMapping> initPathPatternMatchableHandlerMappings(
+	private static Map<HandlerMapping, PathPatternMatchableHandlerMapping> initPathPatternMatchableHandlerMappings(
 			List<HandlerMapping> 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/...main/java/org/springframework/web/servlet/handler/PathPatternMatchableHandlerMapping.java b/...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/...rc/test/java/org/springframework/web/servlet/handler/HandlerMappingIntrospectorTests.java b/...rc/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();