Respect forwarded path in UrlHandlerFilter

Closes gh-35509
This commit is contained in:
rstoyanchev 2025-09-25 12:19:56 +01:00
parent 5a858915ea
commit e9fb5eb38a
2 changed files with 54 additions and 23 deletions

View File

@ -22,6 +22,7 @@ import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.function.Consumer; import java.util.function.Consumer;
import jakarta.servlet.DispatcherType;
import jakarta.servlet.FilterChain; import jakarta.servlet.FilterChain;
import jakarta.servlet.ServletException; import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequest;
@ -43,8 +44,8 @@ import org.springframework.web.util.pattern.PathPattern;
import org.springframework.web.util.pattern.PathPatternParser; 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: * <p>To create an instance, you can use the following:
* *
@ -55,8 +56,8 @@ import org.springframework.web.util.pattern.PathPatternParser;
* .build(); * .build();
* </pre> * </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 * @author Rossen Stoyanchev
* @since 6.2 * @since 6.2
@ -332,9 +333,7 @@ public final class UrlHandlerFilter extends OncePerRequestFilter {
hasPathInfo ? trimTrailingSlash(pathInfo) : pathInfo); hasPathInfo ? trimTrailingSlash(pathInfo) : pathInfo);
RequestPath previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE); RequestPath previousPath = (RequestPath) request.getAttribute(ServletRequestPathUtils.PATH_ATTRIBUTE);
if (previousPath != null) { ServletRequestPathUtils.clearParsedRequestPath(request);
ServletRequestPathUtils.parseAndCache(request);
}
try { try {
chain.doFilter(request, response); chain.doFilter(request, response);
} }
@ -372,22 +371,30 @@ public final class UrlHandlerFilter extends OncePerRequestFilter {
@Override @Override
public String getRequestURI() { public String getRequestURI() {
return this.requestURI; return (isForward() ? getDelegate().getRequestURI() : this.requestURI);
} }
@Override @Override
public StringBuffer getRequestURL() { public StringBuffer getRequestURL() {
return this.requestURL; return (isForward() ? getDelegate().getRequestURL() : this.requestURL);
} }
@Override @Override
public String getServletPath() { public String getServletPath() {
return this.servletPath; return (isForward() ? getDelegate().getServletPath() : this.servletPath);
} }
@Override @Override
public String getPathInfo() { 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();
} }
} }

View File

@ -18,6 +18,7 @@ package org.springframework.web.filter;
import java.io.IOException; import java.io.IOException;
import jakarta.servlet.DispatcherType;
import jakarta.servlet.ServletException; import jakarta.servlet.ServletException;
import jakarta.servlet.http.HttpServlet; import jakarta.servlet.http.HttpServlet;
import jakarta.servlet.http.HttpServletRequest; import jakarta.servlet.http.HttpServletRequest;
@ -127,20 +128,20 @@ public class UrlHandlerFilterTests {
assertThat(response.isCommitted()).isFalse(); assertThat(response.isCommitted()).isFalse();
} }
@Test @Test // gh-35538
void shouldNotFilterErrorAndAsyncDispatches() { void shouldNotFilterErrorAndAsyncDispatches() {
UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/**").wrapRequest().build(); UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/**").wrapRequest().build();
assertThat(filter.shouldNotFilterAsyncDispatch()) 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(); .isTrue();
assertThat(filter.shouldNotFilterErrorDispatch()) 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(); .isTrue();
} }
@Test @Test // gh-35538
void shouldNotCacheParsedPath() throws Exception { void shouldNotCacheParsedPath() throws Exception {
UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build(); UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build();
@ -155,8 +156,8 @@ public class UrlHandlerFilterTests {
.isFalse(); .isFalse();
} }
@Test @Test // gh-35538
void shouldReplaceCachedPath() throws Exception { void shouldClearPreviouslyCachedPath() throws Exception {
UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build(); UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler("/path/*").wrapRequest().build();
MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123/"); MockHttpServletRequest request = new MockHttpServletRequest("GET", "/path/123/");
@ -165,16 +166,38 @@ public class UrlHandlerFilterTests {
ServletRequestPathUtils.parseAndCache(request); ServletRequestPathUtils.parseAndCache(request);
assertThat(ServletRequestPathUtils.getParsedRequestPath(request).value()).isEqualTo("/path/123/"); assertThat(ServletRequestPathUtils.getParsedRequestPath(request).value()).isEqualTo("/path/123/");
PathSavingServlet servlet = new PathSavingServlet(); PathServlet servlet = new PathServlet();
MockFilterChain chain = new MockFilterChain(servlet); MockFilterChain chain = new MockFilterChain(servlet);
filter.doFilterInternal(request, new MockHttpServletResponse(), chain); filter.doFilterInternal(request, new MockHttpServletResponse(), chain);
assertThat(servlet.getParsedPath()).isEqualTo("/path/123"); assertThat(servlet.getParsedPath()).isNull();
assertThat(ServletRequestPathUtils.getParsedRequestPath(request).value()).isEqualTo("/path/123/"); }
@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; private String parsedPath;
@ -183,8 +206,9 @@ public class UrlHandlerFilterTests {
} }
@Override @Override
protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { protected void doGet(HttpServletRequest request, HttpServletResponse response) {
this.parsedPath = ServletRequestPathUtils.getParsedRequestPath(request).value(); this.parsedPath = (ServletRequestPathUtils.hasParsedRequestPath(request) ?
ServletRequestPathUtils.getParsedRequestPath(request).value() : null);
} }
} }