Merge branch '6.2.x'

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

@@ -22,6 +22,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.function.Consumer;
 
+import jakarta.servlet.DispatcherType;
 import jakarta.servlet.FilterChain;
 import jakarta.servlet.ServletException;
 import jakarta.servlet.http.HttpServletRequest;
@@ -43,8 +44,8 @@ import org.springframework.web.util.pattern.PathPattern;
 import org.springframework.web.util.pattern.PathPatternParser;
 
 /**
- * {@link jakarta.servlet.Filter} that modifies the URL, and then redirects or
+ * {@link jakarta.servlet.Filter} that modifies the URL, and then either
- * wraps the request to apply the change.
+ * redirects or wraps the request to effect the change.
  *
  * <p>To create an instance, you can use the following:
  *
@@ -55,8 +56,8 @@ import org.springframework.web.util.pattern.PathPatternParser;
  *    .build();
  * </pre>
  *
- * <p>This {@code Filter} should be ordered after {@link ForwardedHeaderFilter}
+ * <p>This {@code Filter} should be ordered after {@link ForwardedHeaderFilter},
- * and before any security filters.
+ * before {@link ServletRequestPathFilter}, and before security filters.
  *
  * @author Rossen Stoyanchev
  * @since 6.2
@@ -74,26 +75,14 @@ public final class UrlHandlerFilter extends OncePerRequestFilter {
 	}
 
 
-	@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 = (ServletRequestPathUtils.hasParsedRequestPath(request) ?
-		RequestPath path = previousPath;
+				ServletRequestPathUtils.getParsedRequestPath(request) :
-		try {
+				ServletRequestPathUtils.parse(request));
-			if (path == null) {
+
-				path = ServletRequestPathUtils.parseAndCache(request);
-			}
 		for (Map.Entry<Handler, List<PathPattern>> entry : this.handlers.entrySet()) {
 			if (!entry.getKey().supports(request, path)) {
 				continue;
@@ -105,12 +94,6 @@ public final class UrlHandlerFilter extends OncePerRequestFilter {
 				}
 			}
 		}
-		}
-		finally {
-			if (previousPath != null) {
-				ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
-			}
-		}
 
 		chain.doFilter(request, response);
 	}
@@ -348,8 +331,17 @@ public final class UrlHandlerFilter extends OncePerRequestFilter {
 					hasPathInfo ? servletPath : trimTrailingSlash(servletPath),
 					hasPathInfo ? trimTrailingSlash(pathInfo) : pathInfo);
 
+			RequestPath previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
+			ServletRequestPathUtils.clearParsedRequestPath(request);
+			try {
 				chain.doFilter(request, response);
 			}
+			finally {
+				if (previousPath != null) {
+					ServletRequestPathUtils.setParsedRequestPath(previousPath, request);
+				}
+			}
+		}
 	}
 
 
@@ -378,22 +370,30 @@ public final class UrlHandlerFilter extends OncePerRequestFilter {
 
 		@Override
 		public String getRequestURI() {
-			return this.requestURI;
+			return (isForward() ? getDelegate().getRequestURI() : this.requestURI);
 		}
 
 		@Override
 		public StringBuffer getRequestURL() {
-			return this.requestURL;
+			return (isForward() ? getDelegate().getRequestURL() : this.requestURL);
 		}
 
 		@Override
 		public String getServletPath() {
-			return this.servletPath;
+			return (isForward() ? getDelegate().getServletPath() : this.servletPath);
 		}
 
 		@Override
 		public String getPathInfo() {
-			return this.pathInfo;
+			return (isForward() ? getDelegate().getPathInfo() : this.pathInfo);
+		}
+
+		private boolean isForward() {
+			return (getDispatcherType() == DispatcherType.FORWARD);
+		}
+
+		private HttpServletRequest getDelegate() {
+			return (HttpServletRequest) getRequest();
 		}
 	}
 

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 RequestPath}.
-	 * {@link #PATH_ATTRIBUTE} for subsequent use with
-	 * {@link org.springframework.web.util.pattern.PathPattern parsed patterns}.
 	 * <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
+	 * @since 6.2.12
-	 * if any {@code HandlerMapping} indicates that it uses parsed patterns.
+	 */
-	 * After that the pre-parsed and cached {@code RequestPath} can be accessed
+	public static RequestPath parse(HttpServletRequest request) {
-	 * through {@link #getParsedRequestPath(ServletRequest)}.
+		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

@@ -18,8 +18,11 @@ package org.springframework.web.filter;
 
 import java.io.IOException;
 
+import jakarta.servlet.DispatcherType;
 import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServlet;
 import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
 import org.jspecify.annotations.Nullable;
 import org.junit.jupiter.api.Test;
 
@@ -29,6 +32,7 @@ import org.springframework.util.StringUtils;
 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 +128,88 @@ public class UrlHandlerFilterTests {
 		assertThat(response.isCommitted()).isFalse();
 	}
 
+	@Test // gh-35538
+	void shouldNotFilterErrorAndAsyncDispatches() {
+		UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/**").wrapRequest().build();
+
+		assertThat(filter.shouldNotFilterAsyncDispatch())
+				.as("Should not filter ASYNC dispatch as wrapped request is reused")
+				.isTrue();
+
+		assertThat(filter.shouldNotFilterErrorDispatch())
+				.as("Should not filter ERROR dispatch as it's an internal, fixed path")
+				.isTrue();
+	}
+
+	@Test // gh-35538
+	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 // gh-35538
+	void shouldClearPreviouslyCachedPath() 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/");
+
+		PathServlet servlet = new PathServlet();
+		MockFilterChain chain = new MockFilterChain(servlet);
+		filter.doFilterInternal(request, new MockHttpServletResponse(), chain);
+
+		assertThat(servlet.getParsedPath()).isNull();
+	}
+
+	@Test // gh-35509
+	void shouldRespectForwardedPath() throws Exception {
+		UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/requestURI/*").wrapRequest().build();
+
+		String requestURI = "/requestURI/123/";
+		MockHttpServletRequest originalRequest = new MockHttpServletRequest("GET", requestURI);
+		originalRequest.setServletPath(requestURI);
+
+		MockFilterChain chain = new MockFilterChain();
+		filter.doFilterInternal(originalRequest, new MockHttpServletResponse(), chain);
+
+		HttpServletRequest wrapped = (HttpServletRequest) chain.getRequest();
+		assertThat(wrapped).isNotNull().isNotSameAs(originalRequest);
+		assertThat(wrapped.getRequestURI()).isEqualTo("/requestURI/123");
+
+		// Change dispatcher type of underlying requests
+		originalRequest.setDispatcherType(DispatcherType.FORWARD);
+		assertThat(wrapped.getRequestURI())
+				.as("Should delegate to underlying request for the requestURI on FORWARD")
+				.isEqualTo(requestURI);
+	}
+
+
+	@SuppressWarnings("serial")
+	private static class PathServlet extends HttpServlet {
+
+		private String parsedPath;
+
+		public String getParsedPath() {
+			return parsedPath;
+		}
+
+		@Override
+		protected void doGet(HttpServletRequest request, HttpServletResponse response) {
+			this.parsedPath = (ServletRequestPathUtils.hasParsedRequestPath(request) ?
+					ServletRequestPathUtils.getParsedRequestPath(request).value() : null);
+		}
+	}
+
 }