From e9fb5eb38aeb7f422b19ed405ff6750574625f8c Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Thu, 25 Sep 2025 12:19:56 +0100 Subject: [PATCH] Respect forwarded path in UrlHandlerFilter Closes gh-35509 --- .../web/filter/UrlHandlerFilter.java | 29 ++++++----- .../web/filter/UrlHandlerFilterTests.java | 48 ++++++++++++++----- 2 files changed, 54 insertions(+), 23 deletions(-) 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 e8008d3fa4..cc7d206d6d 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 @@ -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 - * wraps the request to apply the change. + * {@link jakarta.servlet.Filter} that modifies the URL, and then either + * redirects or wraps the request to effect the change. * *

To create an instance, you can use the following: * @@ -55,8 +56,8 @@ import org.springframework.web.util.pattern.PathPatternParser; * .build(); * * - *

This {@code Filter} should be ordered after {@link ForwardedHeaderFilter} - * and before any security filters. + *

This {@code Filter} should be ordered after {@link ForwardedHeaderFilter}, + * before {@link ServletRequestPathFilter}, and before security filters. * * @author Rossen Stoyanchev * @since 6.2 @@ -332,9 +333,7 @@ public final class UrlHandlerFilter extends OncePerRequestFilter { hasPathInfo ? trimTrailingSlash(pathInfo) : pathInfo); RequestPath previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE); - if (previousPath != null) { - ServletRequestPathUtils.parseAndCache(request); - } + ServletRequestPathUtils.clearParsedRequestPath(request); try { chain.doFilter(request, response); } @@ -372,22 +371,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(); } } 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 0f3701dd63..efdc1ce0b6 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 @@ -18,6 +18,7 @@ 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; @@ -127,20 +128,20 @@ public class UrlHandlerFilterTests { assertThat(response.isCommitted()).isFalse(); } - @Test + @Test // gh-35538 void shouldNotFilterErrorAndAsyncDispatches() { UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/**").wrapRequest().build(); assertThat(filter.shouldNotFilterAsyncDispatch()) - .as("Should not filter async dispatch: wrapped request is reused") + .as("Should not filter ASYNC dispatch as wrapped request is reused") .isTrue(); assertThat(filter.shouldNotFilterErrorDispatch()) - .as("Should not filter error dispatch: it's a different path") + .as("Should not filter ERROR dispatch as it's an internal, fixed path") .isTrue(); } - @Test + @Test // gh-35538 void shouldNotCacheParsedPath() throws Exception { UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build(); @@ -155,8 +156,8 @@ public class UrlHandlerFilterTests { .isFalse(); } - @Test - void shouldReplaceCachedPath() throws Exception { + @Test // gh-35538 + void shouldClearPreviouslyCachedPath() throws Exception { UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build(); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123/"); @@ -165,16 +166,38 @@ public class UrlHandlerFilterTests { ServletRequestPathUtils.parseAndCache(request); assertThat(ServletRequestPathUtils.getParsedRequestPath(request).value()).isEqualTo("/path/123/"); - PathSavingServlet servlet = new PathSavingServlet(); + PathServlet servlet = new PathServlet(); 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/"); + 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); } - private static class PathSavingServlet extends HttpServlet { + @SuppressWarnings("serial") + private static class PathServlet extends HttpServlet { private String parsedPath; @@ -183,8 +206,9 @@ public class UrlHandlerFilterTests { } @Override - protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { - this.parsedPath = ServletRequestPathUtils.getParsedRequestPath(request).value(); + protected void doGet(HttpServletRequest request, HttpServletResponse response) { + this.parsedPath = (ServletRequestPathUtils.hasParsedRequestPath(request) ? + ServletRequestPathUtils.getParsedRequestPath(request).value() : null); } }