diff --git a/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java b/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java index 3f299b87dec..e8008d3fa4c 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/UrlHandlerFilter.java @@ -74,41 +74,23 @@ 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 = previousPath; - try { - if (path == null) { - path = ServletRequestPathUtils.parseAndCache(request); + RequestPath path = (ServletRequestPathUtils.hasParsedRequestPath(request) ? + ServletRequestPathUtils.getParsedRequestPath(request) : + ServletRequestPathUtils.parse(request)); + + for (Map.Entry> entry : this.handlers.entrySet()) { + if (!entry.getKey().supports(request, path)) { + continue; } - for (Map.Entry> 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); } } @@ -349,7 +331,18 @@ public final class UrlHandlerFilter extends OncePerRequestFilter { 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); + } + } } } diff --git a/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java b/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java index e191ede839b..05cd8b3661e 100644 --- a/spring-web/src/main/java/org/springframework/web/util/ServletRequestPathUtils.java +++ b/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}. *

The returned {@code RequestPath} will have both the contextPath and any * servletPath prefix omitted from the {@link RequestPath#pathWithinApplication() * pathWithinApplication} it exposes. - *

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); diff --git a/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java index 5bcf0b76e09..0f3701dd63b 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/UrlHandlerFilterTests.java @@ -19,7 +19,9 @@ package org.springframework.web.filter; 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.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 +127,65 @@ public class UrlHandlerFilterTests { 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(); + } + } + }