Revise parsed path handling in UrlHandlerFilter

Closes gh-35538

Author: rstoyanchev

spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java

@@ -74,43 +74,25 @@ private UrlHandlerFilter(MultiValueMap<Handler, PathPattern> handlers) {
 	}
 
 
-	@Override
-	protected boolean shouldNotFilterAsyncDispatch() {
-		return false;
-	}
-
-	@Override
-	protected boolean shouldNotFilterErrorDispatch() {
-		return false;
-	}
-
 	@Override
 	protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain chain)
 			throws ServletException, IOException {
 
-		RequestPath previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
-		RequestPath path = previousPath;
-		try {
-			if (path == null) {
-				path = ServletRequestPathUtils.parseAndCache(request);
+		RequestPath path = (ServletRequestPathUtils.hasParsedRequestPath(request) ?
+				ServletRequestPathUtils.getParsedRequestPath(request) :
+				ServletRequestPathUtils.parse(request));
+
+		for (Map.Entry<Handler, List<PathPattern>> entry : this.handlers.entrySet()) {
+			if (!entry.getKey().supports(request, path)) {
+				continue;
 			}
-			for (Map.Entry<Handler, List<PathPattern>> entry : this.handlers.entrySet()) {
-				if (!entry.getKey().supports(request, path)) {
-					continue;
-				}
-				for (PathPattern pattern : entry.getValue()) {
-					if (pattern.matches(path)) {
-						entry.getKey().handle(request, response, chain);
-						return;
-					}
+			for (PathPattern pattern : entry.getValue()) {
+				if (pattern.matches(path)) {
+					entry.getKey().handle(request, response, chain);
+					return;
 				}
 			}
 		}
-		finally {
-			if (previousPath != null) {
-				ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
-			}
-		}
 
 		chain.doFilter(request, response);
 	}
@@ -349,7 +331,18 @@ public void handleInternal(HttpServletRequest request, HttpServletResponse respo
 					hasPathInfo ? servletPath : trimTrailingSlash(servletPath),
 					hasPathInfo ? trimTrailingSlash(pathInfo) : pathInfo);
 
-			chain.doFilter(request, response);
+			RequestPath previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
+			if (previousPath != null) {
+				ServletRequestPathUtils.parseAndCache(request);
+			}
+			try {
+				chain.doFilter(request, response);
+			}
+			finally {
+				if (previousPath != null) {
+					ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
+				}
+			}
 		}
 	}
 

spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java

@@ -52,16 +52,19 @@ public abstract class ServletRequestPathUtils {
 
 	/**
 	 * Parse the {@link HttpServletRequest#getRequestURI() requestURI} to a
-	 * {@link RequestPath} and save it in the request attribute
-	 * {@link #PATH_ATTRIBUTE} for subsequent use with
-	 * {@link org.springframework.web.util.pattern.PathPattern parsed patterns}.
+	 * {@link RequestPath}.
 	 * <p>The returned {@code RequestPath} will have both the contextPath and any
 	 * servletPath prefix omitted from the {@link RequestPath#pathWithinApplication()
 	 * pathWithinApplication} it exposes.
-	 * <p>This method is typically called by the {@code DispatcherServlet} to determine
-	 * if any {@code HandlerMapping} indicates that it uses parsed patterns.
-	 * After that the pre-parsed and cached {@code RequestPath} can be accessed
-	 * through {@link #getParsedRequestPath(ServletRequest)}.
+	 * @since 6.2.12
+	 */
+	public static RequestPath parse(HttpServletRequest request) {
+		return ServletRequestPath.parse(request);
+	}
+
+	/**
+	 * Variant of {@link #parse(HttpServletRequest)} that also saves the parsed
+	 * path in the request attribute {@link #PATH_ATTRIBUTE}.
 	 */
 	public static RequestPath parseAndCache(HttpServletRequest request) {
 		RequestPath requestPath = ServletRequestPath.parse(request);

spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java

@@ -19,7 +19,9 @@
 import java.io.IOException;
 
 import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServlet;
 import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import org.junit.jupiter.api.Test;
 
 import org.springframework.http.HttpHeaders;
@@ -29,6 +31,7 @@
 import org.springframework.web.testfixture.servlet.MockFilterChain;
 import org.springframework.web.testfixture.servlet.MockHttpServletRequest;
 import org.springframework.web.testfixture.servlet.MockHttpServletResponse;
+import org.springframework.web.util.ServletRequestPathUtils;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -124,4 +127,65 @@ private static void testNoUrlHandling(String pattern, String contextPath, String
 		assertThat(response.isCommitted()).isFalse();
 	}
 
+	@Test
+	void shouldNotFilterErrorAndAsyncDispatches() {
+		UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/**").wrapRequest().build();
+
+		assertThat(filter.shouldNotFilterAsyncDispatch())
+				.as("Should not filter async dispatch: wrapped request is reused")
+				.isTrue();
+
+		assertThat(filter.shouldNotFilterErrorDispatch())
+				.as("Should not filter error dispatch: it's a different path")
+				.isTrue();
+	}
+
+	@Test
+	void shouldNotCacheParsedPath() throws Exception {
+		UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build();
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123/");
+		request.setServletPath("/path/123/");
+
+		MockFilterChain chain = new MockFilterChain();
+		filter.doFilterInternal(request, new MockHttpServletResponse(), chain);
+
+		assertThat(ServletRequestPathUtils.hasParsedRequestPath(request))
+				.as("Path with trailing slash should not be cached")
+				.isFalse();
+	}
+
+	@Test
+	void shouldReplaceCachedPath() throws Exception {
+		UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build();
+
+		MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123/");
+		request.setServletPath("/path/123/");
+
+		ServletRequestPathUtils.parseAndCache(request);
+		assertThat(ServletRequestPathUtils.getParsedRequestPath(request).value()).isEqualTo("/path/123/");
+
+		PathSavingServlet servlet = new PathSavingServlet();
+		MockFilterChain chain = new MockFilterChain(servlet);
+		filter.doFilterInternal(request, new MockHttpServletResponse(), chain);
+
+		assertThat(servlet.getParsedPath()).isEqualTo("/path/123");
+		assertThat(ServletRequestPathUtils.getParsedRequestPath(request).value()).isEqualTo("/path/123/");
+	}
+
+
+	private static class PathSavingServlet extends HttpServlet {
+
+		private String parsedPath;
+
+		public String getParsedPath() {
+			return parsedPath;
+		}
+
+		@Override
+		protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException {
+			this.parsedPath = ServletRequestPathUtils.getParsedRequestPath(request).value();
+		}
+	}
+
 }