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 c64a12d116a..0e0ab355f58 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 @@ -18,8 +18,10 @@ import java.io.IOException; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.TreeMap; import java.util.function.Consumer; import jakarta.servlet.DispatcherType; @@ -33,11 +35,13 @@ import org.jspecify.annotations.Nullable; import org.springframework.http.HttpHeaders; -import org.springframework.http.HttpStatus; +import org.springframework.http.HttpStatusCode; import org.springframework.http.server.PathContainer; import org.springframework.http.server.RequestPath; +import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.util.ServletRequestPathUtils; import org.springframework.web.util.pattern.PathPattern; @@ -53,13 +57,14 @@ * UrlHandlerFilter filter = UrlHandlerFilter * .trailingSlashHandler("/path1/**").redirect(HttpStatus.PERMANENT_REDIRECT) * .trailingSlashHandler("/path2/**").wrapRequest() + * .exclude("/path1/foo/bar/**", "/path2/baz/") * .build(); * * *

This {@code Filter} should be ordered after {@link ForwardedHeaderFilter}, * before {@link ServletRequestPathFilter}, and before security filters. * - * @author Rossen Stoyanchev + * @author Rossen Stoyanchev, James Missen * @since 6.2 */ public final class UrlHandlerFilter extends OncePerRequestFilter { @@ -67,11 +72,11 @@ public final class UrlHandlerFilter extends OncePerRequestFilter { private static final Log logger = LogFactory.getLog(UrlHandlerFilter.class); - private final MultiValueMap handlers; + private final HandlerRegistry handlerRegistry; - private UrlHandlerFilter(MultiValueMap handlers) { - this.handlers = new LinkedMultiValueMap<>(handlers); + private UrlHandlerFilter(HandlerRegistry handlerRegistry) { + this.handlerRegistry = handlerRegistry; } @@ -82,33 +87,26 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse 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 (PathPattern pattern : entry.getValue()) { - if (pattern.matches(path)) { - entry.getKey().handle(request, response, chain); - return; - } - } + PathContainer lookupPath = path.subPath(path.contextPath().elements().size()); + Handler handler = handlerRegistry.lookupHandler(path, lookupPath, request); + if (handler != null) { + handler.handle(request, response, chain); + return; } - chain.doFilter(request, response); } /** - * Create a builder by adding a handler for URL's with a trailing slash. - * @param pathPatterns path patterns to map the handler to, for example, + * Create a builder by adding a handler for URLs with a trailing slash. + * @param patterns path patterns to map the handler to, for example, * "/path/*", "/path/**", * "/path/foo/". * @return a spec to configure the trailing slash handler with * @see Builder#trailingSlashHandler(String...) */ - public static Builder.TrailingSlashSpec trailingSlashHandler(String... pathPatterns) { - return new DefaultBuilder().trailingSlashHandler(pathPatterns); + public static Builder.TrailingSlashSpec trailingSlashHandler(String... patterns) { + return new DefaultBuilder().trailingSlashHandler(patterns); } @@ -118,13 +116,32 @@ public static Builder.TrailingSlashSpec trailingSlashHandler(String... pathPatte public interface Builder { /** - * Add a handler for URL's with a trailing slash. - * @param pathPatterns path patterns to map the handler to, for example, + * Add a handler for URLs with a trailing slash. + * @param patterns path patterns to map the handler to, for example, * "/path/*", "/path/**", * "/path/foo/". * @return a spec to configure the handler with */ - TrailingSlashSpec trailingSlashHandler(String... pathPatterns); + TrailingSlashSpec trailingSlashHandler(String... patterns); + + /** + * Exclude patterns from matching any other handler. + * @param patterns path patterns to not map any handler to, e.g. + * "/path/foo/*", "/path/foo/**", + * "/path/foo/bar/". + * @return the {@link Builder}, which allows adding more + * handlers and then building the Filter instance. + */ + Builder exclude(String... patterns); + + /** + * Specify whether to use path pattern specificity for matching handlers, + * with more specific patterns taking precedence. + *

The default value is {@code false}. + * @return the {@link Builder}, which allows adding more + * handlers and then building the Filter instance. + */ + Builder useSpecificityOrder(boolean useSpecificityOrder); /** * Build the {@link UrlHandlerFilter} instance. @@ -146,11 +163,11 @@ interface TrailingSlashSpec { /** * Handle requests by sending a redirect to the same URL but the * trailing slash trimmed. - * @param status the redirect status to use + * @param statusCode the redirect status to use * @return the top level {@link Builder}, which allows adding more * handlers and then building the Filter instance. */ - Builder redirect(HttpStatus status); + Builder redirect(HttpStatusCode statusCode); /** * Handle the request by wrapping it in order to trim the trailing @@ -168,36 +185,89 @@ interface TrailingSlashSpec { */ private static final class DefaultBuilder implements Builder { - private final PathPatternParser patternParser = new PathPatternParser(); + /** + * Empty handler that does not handle the request URL, and proceeds directly to the next filter. + */ + private static final Handler NO_OP_HANDLER = new Handler() { + + @Override + public boolean supports(HttpServletRequest request, RequestPath path) { + return true; + } + + @Override + public void handle(HttpServletRequest request, HttpServletResponse response, FilterChain chain) + throws ServletException, IOException { + + chain.doFilter(request, response); + } + + @Override + public String toString() { + return "NoOpHandler"; + } + }; + + private final MultiValueMap handlers = new LinkedMultiValueMap<>(); - private final MultiValueMap handlers = new LinkedMultiValueMap<>(); + private boolean useSpecificityOrder = false; + + private DefaultBuilder() { + // Ensure any no-op handlers are registered first when building + this.handlers.addAll(NO_OP_HANDLER, Collections.emptyList()); + } @Override public TrailingSlashSpec trailingSlashHandler(String... patterns) { return new DefaultTrailingSlashSpec(patterns); } - private DefaultBuilder addHandler(List pathPatterns, Handler handler) { - pathPatterns.forEach(pattern -> this.handlers.add(handler, pattern)); + @Override + public Builder exclude(String... patterns) { + return addHandler(NO_OP_HANDLER, patterns); + } + + @Override + public Builder useSpecificityOrder(boolean useSpecificityOrder) { + this.useSpecificityOrder = useSpecificityOrder; + return this; + } + + private Builder addHandler(Handler handler, String... patterns) { + if (!ObjectUtils.isEmpty(patterns)) { + this.handlers.addAll(handler, Arrays.stream(patterns).toList()); + } return this; } @Override public UrlHandlerFilter build() { - return new UrlHandlerFilter(this.handlers); + HandlerRegistry handlerRegistry; + if (this.useSpecificityOrder) { + handlerRegistry = new OrderedHandlerRegistry(); + } + else { + handlerRegistry = new DefaultHandlerRegistry(); + } + for (Map.Entry> entry : this.handlers.entrySet()) { + for (String pattern : entry.getValue()) { + handlerRegistry.registerHandler(pattern, entry.getKey()); + } + } + + return new UrlHandlerFilter(handlerRegistry); } private final class DefaultTrailingSlashSpec implements TrailingSlashSpec { - private final List pathPatterns; + private final String[] pathPatterns; private @Nullable Consumer interceptor; private DefaultTrailingSlashSpec(String[] patterns) { this.pathPatterns = Arrays.stream(patterns) .map(pattern -> pattern.endsWith("**") || pattern.endsWith("/") ? pattern : pattern + "/") - .map(patternParser::parse) - .toList(); + .toArray(String[]::new); } @Override @@ -207,21 +277,20 @@ public TrailingSlashSpec intercept(Consumer consumer) { } @Override - public Builder redirect(HttpStatus status) { - Handler handler = new RedirectTrailingSlashHandler(status, this.interceptor); - return DefaultBuilder.this.addHandler(this.pathPatterns, handler); + public Builder redirect(HttpStatusCode statusCode) { + Handler handler = new RedirectTrailingSlashHandler(statusCode, this.interceptor); + return DefaultBuilder.this.addHandler(handler, this.pathPatterns); } @Override public Builder wrapRequest() { Handler handler = new RequestWrappingTrailingSlashHandler(this.interceptor); - return DefaultBuilder.this.addHandler(this.pathPatterns, handler); + return DefaultBuilder.this.addHandler(handler, this.pathPatterns); } } } - /** * Internal handler to encapsulate different ways to handle a request. */ @@ -280,6 +349,11 @@ protected String trimTrailingSlash(String path) { int index = (StringUtils.hasLength(path) ? path.lastIndexOf('/') : -1); return (index != -1 ? path.substring(0, index) : path); } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } @@ -288,11 +362,13 @@ protected String trimTrailingSlash(String path) { */ private static final class RedirectTrailingSlashHandler extends AbstractTrailingSlashHandler { - private final HttpStatus httpStatus; + private final HttpStatusCode statusCode; - RedirectTrailingSlashHandler(HttpStatus httpStatus, @Nullable Consumer interceptor) { + RedirectTrailingSlashHandler(HttpStatusCode statusCode, @Nullable Consumer interceptor) { super(interceptor); - this.httpStatus = httpStatus; + Assert.isTrue(statusCode.is3xxRedirection(), "HTTP status code for redirect handlers " + + "must be in the Redirection class (3xx)"); + this.statusCode = statusCode; } @Override @@ -305,10 +381,15 @@ public void handleInternal(HttpServletRequest request, HttpServletResponse respo } response.resetBuffer(); - response.setStatus(this.httpStatus.value()); + response.setStatus(this.statusCode.value()); response.setHeader(HttpHeaders.LOCATION, location); response.flushBuffer(); } + + @Override + public String toString() { + return getClass().getSimpleName() + " {statusCode=" + this.statusCode.value() + "}"; + } } @@ -402,4 +483,122 @@ private HttpServletRequest getDelegate() { } } + + /** + * Internal registry to encapsulate different ways to select a handler for a request. + */ + private interface HandlerRegistry { + + /** + * Register the specified handler for the given path pattern. + * @param pattern the path pattern the handler should be mapped to + * @param handler the handler instance to register + * @throws IllegalStateException if there is a conflicting handler registered + */ + void registerHandler(String pattern, Handler handler); + + /** + * Look up a handler instance for the given URL lookup path. + * @param path the parsed RequestPath + * @param lookupPath the URL path the handler is mapped to + * @param request the current request + * @return the associated handler instance, or {@code null} if not found + * @see org.springframework.web.util.pattern.PathPattern + */ + @Nullable Handler lookupHandler(RequestPath path, PathContainer lookupPath, HttpServletRequest request); + } + + + /** + * Base class for {@link HandlerRegistry} implementations. + */ + private static abstract class AbstractHandlerRegistry implements HandlerRegistry { + + private final PathPatternParser patternParser = new PathPatternParser(); + + @Override + public final void registerHandler(String pattern, Handler handler) { + Assert.notNull(pattern, "Pattern must not be null"); + Assert.notNull(handler, "Handler must not be null"); + + // Parse path pattern + pattern = patternParser.initFullPathPattern(pattern); + PathPattern pathPattern = patternParser.parse(pattern); + + // Register handler + registerHandlerInternal(pathPattern, handler); + if (logger.isTraceEnabled()) { + logger.trace("Mapped [" + pattern + "] onto " + handler); + } + } + + protected abstract void registerHandlerInternal(PathPattern pathPattern, Handler handler); + } + + + /** + * Default {@link HandlerRegistry} implementation. + */ + private static final class DefaultHandlerRegistry extends AbstractHandlerRegistry { + + private final MultiValueMap handlerMap = new LinkedMultiValueMap<>(); + + @Override + protected void registerHandlerInternal(PathPattern pathPattern, Handler handler) { + this.handlerMap.add(handler, pathPattern); + } + + @Override + public @Nullable Handler lookupHandler(RequestPath path, PathContainer lookupPath, HttpServletRequest request) { + for (Map.Entry> entry : this.handlerMap.entrySet()) { + if (!entry.getKey().supports(request, path)) { + continue; + } + for (PathPattern pattern : entry.getValue()) { + if (pattern.matches(lookupPath)) { + return entry.getKey(); + } + } + } + return null; + } + } + + + /** + * Handler registry that selects the handler mapped to the best-matching + * (i.e. most specific) path pattern. + */ + private static final class OrderedHandlerRegistry extends AbstractHandlerRegistry { + + private final Map handlerMap = new TreeMap<>(); + + @Override + protected void registerHandlerInternal(PathPattern pathPattern, Handler handler) { + if (this.handlerMap.containsKey(pathPattern)) { + Handler existingHandler = this.handlerMap.get(pathPattern); + if (existingHandler != null && existingHandler != handler) { + throw new IllegalStateException( + "Cannot map " + handler + " to [" + pathPattern + "]: there is already " + + existingHandler + " mapped."); + } + } + this.handlerMap.put(pathPattern, handler); + } + + @Override + public @Nullable Handler lookupHandler(RequestPath path, PathContainer lookupPath, HttpServletRequest request) { + for (Map.Entry entry : this.handlerMap.entrySet()) { + if (!entry.getKey().matches(lookupPath)) { + continue; + } + if (entry.getValue().supports(request, path)) { + return entry.getValue(); + } + return null; // only match one path pattern + } + return null; + } + } + } diff --git a/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java b/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java index 33d612380df..fbb48d0b80c 100644 --- a/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java +++ b/spring-web/src/main/java/org/springframework/web/filter/reactive/UrlHandlerFilter.java @@ -18,8 +18,10 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.TreeMap; import java.util.function.Function; import org.apache.commons.logging.Log; @@ -34,8 +36,10 @@ import org.springframework.http.server.RequestPath; import org.springframework.http.server.reactive.ServerHttpRequest; import org.springframework.http.server.reactive.ServerHttpResponse; +import org.springframework.util.Assert; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import org.springframework.util.ObjectUtils; import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilter; @@ -44,7 +48,7 @@ import org.springframework.web.util.pattern.PathPatternParser; /** - * {@link org.springframework.web.server.WebFilter} that modifies the URL, and + * {@link WebFilter} that modifies the URL, and * then redirects or wraps the request to apply the change. * *

To create an instance, you can use the following: @@ -53,12 +57,13 @@ * UrlHandlerFilter filter = UrlHandlerFilter * .trailingSlashHandler("/path1/**").redirect(HttpStatus.PERMANENT_REDIRECT) * .trailingSlashHandler("/path2/**").mutateRequest() + * .exclude("/path1/foo/bar/**", "/path2/baz/") * .build(); * * *

This {@code WebFilter} should be ordered ahead of security filters. * - * @author Rossen Stoyanchev + * @author Rossen Stoyanchev, James Missen * @since 6.2 */ public final class UrlHandlerFilter implements WebFilter { @@ -66,40 +71,36 @@ public final class UrlHandlerFilter implements WebFilter { private static final Log logger = LogFactory.getLog(UrlHandlerFilter.class); - private final MultiValueMap handlers; + private final HandlerRegistry handlerRegistry; - private UrlHandlerFilter(MultiValueMap handlers) { - this.handlers = new LinkedMultiValueMap<>(handlers); + private UrlHandlerFilter(HandlerRegistry handlerRegistry) { + this.handlerRegistry = handlerRegistry; } @Override public Mono filter(ServerWebExchange exchange, WebFilterChain chain) { RequestPath path = exchange.getRequest().getPath(); - for (Map.Entry> entry : this.handlers.entrySet()) { - if (!entry.getKey().supports(exchange)) { - continue; - } - for (PathPattern pattern : entry.getValue()) { - if (pattern.matches(path)) { - return entry.getKey().handle(exchange, chain); - } - } + PathContainer lookupPath = path.pathWithinApplication(); + Handler handler = handlerRegistry.lookupHandler(lookupPath, exchange); + if (handler != null) { + return handler.handle(exchange, chain); } return chain.filter(exchange); } + /** - * Create a builder by adding a handler for URL's with a trailing slash. - * @param pathPatterns path patterns to map the handler to, e.g. + * Create a builder by adding a handler for URLs with a trailing slash. + * @param patterns path patterns to map the handler to, e.g. * "/path/*", "/path/**", * "/path/foo/". * @return a spec to configure the trailing slash handler with * @see Builder#trailingSlashHandler(String...) */ - public static Builder.TrailingSlashSpec trailingSlashHandler(String... pathPatterns) { - return new DefaultBuilder().trailingSlashHandler(pathPatterns); + public static Builder.TrailingSlashSpec trailingSlashHandler(String... patterns) { + return new DefaultBuilder().trailingSlashHandler(patterns); } @@ -109,13 +110,32 @@ public static Builder.TrailingSlashSpec trailingSlashHandler(String... pathPatte public interface Builder { /** - * Add a handler for URL's with a trailing slash. - * @param pathPatterns path patterns to map the handler to, e.g. + * Add a handler for URLs with a trailing slash. + * @param patterns path patterns to map the handler to, e.g. * "/path/*", "/path/**", * "/path/foo/". * @return a spec to configure the handler with */ - TrailingSlashSpec trailingSlashHandler(String... pathPatterns); + TrailingSlashSpec trailingSlashHandler(String... patterns); + + /** + * Exclude patterns from matching other handlers. + * @param patterns path patterns to exclude to, e.g. + * "/path/foo/*", "/path/foo/**", + * "/path/foo/bar/". + * @return the {@link Builder}, which allows adding more + * handlers and then building the Filter instance. + */ + Builder exclude(String... patterns); + + /** + * Specify whether to use path pattern specificity for matching handlers, + * with more specific patterns taking precedence. + *

The default value is {@code false}. + * @return the {@link Builder}, which allows adding more + * handlers and then building the Filter instance. + */ + Builder useSpecificityOrder(boolean useSpecificityOrder); /** * Build the {@link UrlHandlerFilter} instance. @@ -159,37 +179,88 @@ interface TrailingSlashSpec { */ private static final class DefaultBuilder implements Builder { - private final PathPatternParser patternParser = new PathPatternParser(); + /** + * Empty handler that does not handle the request URL, and proceeds directly to the next filter. + */ + private static final Handler NO_OP_HANDLER = new Handler() { - private final MultiValueMap handlers = new LinkedMultiValueMap<>(); + @Override + public boolean supports(ServerWebExchange exchange) { + return true; + } + + @Override + public Mono handle(ServerWebExchange exchange, WebFilterChain chain) { + return chain.filter(exchange); + } + + @Override + public String toString() { + return "NoOpHandler"; + } + }; + + private final MultiValueMap handlers = new LinkedMultiValueMap<>(); + + private boolean useSpecificityOrder = false; + + private DefaultBuilder() { + // Ensure any no-op handlers are registered first when building + this.handlers.addAll(NO_OP_HANDLER, Collections.emptyList()); + } @Override public TrailingSlashSpec trailingSlashHandler(String... patterns) { return new DefaultTrailingSlashSpec(patterns); } - private DefaultBuilder addHandler(List pathPatterns, Handler handler) { - pathPatterns.forEach(pattern -> this.handlers.add(handler, pattern)); + @Override + public Builder exclude(String... patterns) { + return addHandler(NO_OP_HANDLER, patterns); + } + + @Override + public Builder useSpecificityOrder(boolean useSpecificityOrder) { + this.useSpecificityOrder = useSpecificityOrder; + return this; + } + + private Builder addHandler(Handler handler, String... patterns) { + if (!ObjectUtils.isEmpty(patterns)) { + this.handlers.addAll(handler, Arrays.stream(patterns).toList()); + } return this; } @Override public UrlHandlerFilter build() { - return new UrlHandlerFilter(this.handlers); + HandlerRegistry handlerRegistry; + if (this.useSpecificityOrder) { + handlerRegistry = new OrderedHandlerRegistry(); + } + else { + handlerRegistry = new DefaultHandlerRegistry(); + } + for (Map.Entry> entry : this.handlers.entrySet()) { + for (String pattern : entry.getValue()) { + handlerRegistry.registerHandler(pattern, entry.getKey()); + } + } + + return new UrlHandlerFilter(handlerRegistry); } private final class DefaultTrailingSlashSpec implements TrailingSlashSpec { - private final List pathPatterns; + private final String[] patterns; private @Nullable List>> interceptors; private DefaultTrailingSlashSpec(String[] patterns) { - this.pathPatterns = Arrays.stream(patterns) + this.patterns = Arrays.stream(patterns) .map(pattern -> pattern.endsWith("**") || pattern.endsWith("/") ? pattern : pattern + "/") - .map(patternParser::parse) - .toList(); + .toArray(String[]::new); } @Override @@ -202,13 +273,13 @@ public TrailingSlashSpec intercept(Function> inter @Override public Builder redirect(HttpStatusCode statusCode) { Handler handler = new RedirectTrailingSlashHandler(statusCode, this.interceptors); - return DefaultBuilder.this.addHandler(this.pathPatterns, handler); + return DefaultBuilder.this.addHandler(handler, this.patterns); } @Override public Builder mutateRequest() { Handler handler = new RequestWrappingTrailingSlashHandler(this.interceptors); - return DefaultBuilder.this.addHandler(this.pathPatterns, handler); + return DefaultBuilder.this.addHandler(handler, this.patterns); } } } @@ -271,6 +342,11 @@ protected String trimTrailingSlash(ServerHttpRequest request) { int index = (StringUtils.hasLength(path) ? path.lastIndexOf('/') : -1); return (index != -1 ? path.substring(0, index) : path); } + + @Override + public String toString() { + return getClass().getSimpleName(); + } } @@ -285,6 +361,8 @@ private static final class RedirectTrailingSlashHandler extends AbstractTrailing HttpStatusCode statusCode, @Nullable List>> interceptors) { super(interceptors); + Assert.isTrue(statusCode.is3xxRedirection(), "HTTP status code for redirect handlers " + + "must be in the Redirection class (3xx)"); this.statusCode = statusCode; } @@ -302,6 +380,11 @@ public Mono handleInternal(ServerWebExchange exchange, WebFilterChain chai response.getHeaders().set(HttpHeaders.LOCATION, location); return Mono.empty(); } + + @Override + public String toString() { + return getClass().getSimpleName() + " {statusCode=" + this.statusCode.value() + "}"; + } } @@ -322,4 +405,118 @@ public Mono handleInternal(ServerWebExchange exchange, WebFilterChain chai } } + + /** + * Internal registry to encapsulate different ways to select a handler for a request. + */ + private interface HandlerRegistry { + + /** + * Register the specified handler for the given path pattern. + * @param pattern the path pattern the handler should be mapped to + * @param handler the handler instance to register + * @throws IllegalStateException if there is a conflicting handler registered + */ + void registerHandler(String pattern, Handler handler); + + /** + * Look up a handler instance for the given URL lookup path. + * @param lookupPath the URL path the handler is mapped to + * @param exchange the current exchange + * @return the associated handler instance, or {@code null} if not found + * @see org.springframework.web.util.pattern.PathPattern + */ + @Nullable Handler lookupHandler(PathContainer lookupPath, ServerWebExchange exchange); + } + + + /** + * Base class for {@link HandlerRegistry} implementations. + */ + private static abstract class AbstractHandlerRegistry implements HandlerRegistry { + + private final PathPatternParser patternParser = new PathPatternParser(); + + @Override + public final void registerHandler(String pattern, Handler handler) { + Assert.notNull(pattern, "Pattern must not be null"); + Assert.notNull(handler, "Handler must not be null"); + + // Parse path pattern + pattern = patternParser.initFullPathPattern(pattern); + PathPattern pathPattern = patternParser.parse(pattern); + + // Register handler + registerHandlerInternal(pathPattern, handler); + if (logger.isTraceEnabled()) { + logger.trace("Mapped [" + pattern + "] onto " + handler); + } + } + + protected abstract void registerHandlerInternal(PathPattern pathPattern, Handler handler); + } + + + /** + * Default {@link HandlerRegistry} implementation. + */ + private static final class DefaultHandlerRegistry extends AbstractHandlerRegistry { + + private final MultiValueMap handlerMap = new LinkedMultiValueMap<>(); + + @Override + protected void registerHandlerInternal(PathPattern pathPattern, Handler handler) { + this.handlerMap.add(handler, pathPattern); + } + + @Override + public @Nullable Handler lookupHandler(PathContainer lookupPath, ServerWebExchange exchange) { + for (Map.Entry> entry : this.handlerMap.entrySet()) { + if (!entry.getKey().supports(exchange)) { + continue; + } + for (PathPattern pattern : entry.getValue()) { + if (pattern.matches(lookupPath)) { + return entry.getKey(); + } + } + } + return null; + } + } + + + /** + * Handler registry that selects the handler mapped to the best-matching + * (i.e. most specific) path pattern. + */ + private static final class OrderedHandlerRegistry extends AbstractHandlerRegistry { + + private final Map handlerMap = new TreeMap<>(); + + @Override + protected void registerHandlerInternal(PathPattern pathPattern, Handler handler) { + Handler existingHandler = this.handlerMap.put(pathPattern, handler); + if (existingHandler != null && existingHandler != handler) { + throw new IllegalStateException( + "Cannot map " + handler + " to [" + pathPattern + "]: there is already " + + existingHandler + " mapped."); + } + } + + @Override + public @Nullable Handler lookupHandler(PathContainer lookupPath, ServerWebExchange exchange) { + for (Map.Entry entry : this.handlerMap.entrySet()) { + if (!entry.getKey().matches(lookupPath)) { + continue; + } + if (entry.getValue().supports(exchange)) { + return entry.getValue(); + } + return null; // only match one path pattern + } + return null; + } + } + } 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 e020527fb1f..c548ba7e2f0 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 @@ -33,8 +33,7 @@ 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; +import static org.assertj.core.api.Assertions.*; /** * Unit tests for {@link UrlHandlerFilter}. @@ -93,19 +92,66 @@ void redirect() throws Exception { assertThat(response.isCommitted()).isTrue(); } + @Test // gh-35882 + void orderedUrlHandling() throws Exception { + String path = "/path/123"; + MockHttpServletRequest request = new MockHttpServletRequest("GET", path + "/"); + + HttpStatus status = HttpStatus.PERMANENT_REDIRECT; + + // Request wrapping + UrlHandlerFilter filter = UrlHandlerFilter + .trailingSlashHandler("/path/**").redirect(status) + .trailingSlashHandler("/path/123/").wrapRequest() // most specific pattern + .trailingSlashHandler("/path/123/**").redirect(status) + .useSpecificityOrder(true) + .build(); + + MockFilterChain chain = new MockFilterChain(); + filter.doFilterInternal(request, new MockHttpServletResponse(), chain); + + HttpServletRequest actual = (HttpServletRequest) chain.getRequest(); + assertThat(actual).isNotNull().isNotSameAs(request); + assertThat(actual.getRequestURL().toString()).isEqualTo("http://localhost" + path); + + // Redirect + filter = UrlHandlerFilter + .trailingSlashHandler("/path/**").wrapRequest() + .trailingSlashHandler("/path/123/").redirect(status) // most specific pattern + .trailingSlashHandler("/path/123/**").wrapRequest() + .useSpecificityOrder(true) + .build(); + + MockHttpServletResponse response = new MockHttpServletResponse(); + + chain = new MockFilterChain(); + filter.doFilterInternal(request, response, chain); + + assertThat(chain.getRequest()).isNull(); + assertThat(response.getStatus()).isEqualTo(status.value()); + assertThat(response.getHeader(HttpHeaders.LOCATION)).isEqualTo(path); + assertThat(response.isCommitted()).isTrue(); + } + @Test void noUrlHandling() throws Exception { - testNoUrlHandling("/path/**", "", "/path/123"); - testNoUrlHandling("/path/*", "", "/path/123"); - testNoUrlHandling("/**", "", "/"); // gh-33444 - testNoUrlHandling("/**", "/myApp", "/myApp/"); // gh-33565 + testNoUrlHandling("/path/**", null, "", "/path/123"); + testNoUrlHandling("/path/*", null, "", "/path/123"); + testNoUrlHandling("/**", null, "", "/"); // gh-33444 + testNoUrlHandling("/**", null, "/myApp", "/myApp/"); // gh-33565 + testNoUrlHandling("/path/**", "/path/123/**", "", "/path/123/"); // gh-35882 + testNoUrlHandling("/path/*/*", "/path/123/**", "", "/path/123/abc/"); // gh-35882 + testNoUrlHandling("/path/**", "/path/123/abc/", "", "/path/123/abc/"); // gh-35882 } - private static void testNoUrlHandling(String pattern, String contextPath, String requestURI) - throws ServletException, IOException { + private static void testNoUrlHandling(String pattern, @Nullable String excludePattern, String contextPath, + String requestURI) throws ServletException, IOException { + + boolean hasExcludePattern = StringUtils.hasLength(excludePattern); // No request wrapping - UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler(pattern).wrapRequest().build(); + UrlHandlerFilter.Builder builder = UrlHandlerFilter.trailingSlashHandler(pattern).wrapRequest(); + UrlHandlerFilter filter = hasExcludePattern ? builder.exclude(excludePattern).build() : builder.build(); MockHttpServletRequest request = new MockHttpServletRequest("GET", requestURI); request.setContextPath(contextPath); @@ -117,7 +163,8 @@ private static void testNoUrlHandling(String pattern, String contextPath, String // No redirect HttpStatus status = HttpStatus.PERMANENT_REDIRECT; - filter = UrlHandlerFilter.trailingSlashHandler(pattern).redirect(status).build(); + builder = UrlHandlerFilter.trailingSlashHandler(pattern).redirect(status); + filter = hasExcludePattern ? builder.exclude(excludePattern).build() : builder.build(); request = new MockHttpServletRequest("GET", requestURI); request.setContextPath(contextPath); diff --git a/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java b/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java index 14a8e40440b..b7479ee182a 100644 --- a/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java +++ b/spring-web/src/test/java/org/springframework/web/filter/reactive/UrlHandlerFilterTests.java @@ -19,12 +19,14 @@ import java.net.URI; import java.util.List; +import org.jspecify.annotations.Nullable; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; import reactor.core.publisher.Mono; import org.springframework.http.HttpStatus; import org.springframework.http.server.reactive.ServerHttpRequest; +import org.springframework.util.StringUtils; import org.springframework.web.server.ServerWebExchange; import org.springframework.web.server.WebFilterChain; import org.springframework.web.server.WebHandler; @@ -75,18 +77,62 @@ void redirect() { assertThat(exchange.getResponse().getHeaders().getLocation()).isEqualTo(URI.create(path + "?" + queryString)); } + @Test // gh-35882 + void orderedUrlHandling() { + String path = "/path/123"; + MockServerHttpRequest original = MockServerHttpRequest.get(path + "/").build(); + ServerWebExchange exchange = MockServerWebExchange.from(original); + + HttpStatus status = HttpStatus.PERMANENT_REDIRECT; + + // Request mutation + UrlHandlerFilter filter = UrlHandlerFilter + .trailingSlashHandler("/path/**").redirect(status) + .trailingSlashHandler("/path/123/").mutateRequest() // most specific pattern + .trailingSlashHandler("/path/123/**").redirect(status) + .useSpecificityOrder(true) + .build(); + + ServerHttpRequest actual = invokeFilter(filter, exchange); + + assertThat(actual).isNotNull().isNotSameAs(original); + assertThat(actual.getPath().value()).isEqualTo(path); + + // Redirect + filter = UrlHandlerFilter + .trailingSlashHandler("/path/**").mutateRequest() + .trailingSlashHandler("/path/123/").redirect(status) // most specific pattern + .trailingSlashHandler("/path/123/**").mutateRequest() + .useSpecificityOrder(true) + .build(); + + UrlHandlerFilter finalFilter = filter; + assertThatThrownBy(() -> invokeFilter(finalFilter, exchange)) + .hasMessageContaining("No argument value was captured"); + + assertThat(exchange.getResponse().getStatusCode()).isEqualTo(status); + assertThat(exchange.getResponse().getHeaders().getLocation()).isEqualTo(URI.create(path)); + } + @Test void noUrlHandling() { - testNoUrlHandling("/path/**", "", "/path/123"); - testNoUrlHandling("/path/*", "", "/path/123"); - testNoUrlHandling("/**", "", "/"); // gh-33444 - testNoUrlHandling("/**", "/myApp", "/myApp/"); // gh-33565 + testNoUrlHandling("/path/**", null, "", "/path/123"); + testNoUrlHandling("/path/*", null, "", "/path/123"); + testNoUrlHandling("/**", null, "", "/"); // gh-33444 + testNoUrlHandling("/**", null, "/myApp", "/myApp/"); // gh-33565 + testNoUrlHandling("/path/**", "/path/123/**", "", "/path/123/"); // gh-35882 + testNoUrlHandling("/path/*/*", "/path/123/**", "", "/path/123/abc/"); // gh-35882 + testNoUrlHandling("/path/**", "/path/123/abc/", "", "/path/123/abc/"); // gh-35882 } - private static void testNoUrlHandling(String pattern, String contextPath, String path) { + private static void testNoUrlHandling(String pattern, @Nullable String excludePattern, String contextPath, + String path) { + + boolean hasExcludePattern = StringUtils.hasLength(excludePattern); // No request mutation - UrlHandlerFilter filter = UrlHandlerFilter.trailingSlashHandler(pattern).mutateRequest().build(); + UrlHandlerFilter.Builder builder = UrlHandlerFilter.trailingSlashHandler(pattern).mutateRequest(); + UrlHandlerFilter filter = hasExcludePattern ? builder.exclude(excludePattern).build() : builder.build(); MockServerHttpRequest request = MockServerHttpRequest.get(path).contextPath(contextPath).build(); ServerWebExchange exchange = MockServerWebExchange.from(request); @@ -98,7 +144,8 @@ private static void testNoUrlHandling(String pattern, String contextPath, String // No redirect HttpStatus status = HttpStatus.PERMANENT_REDIRECT; - filter = UrlHandlerFilter.trailingSlashHandler(pattern).redirect(status).build(); + builder = UrlHandlerFilter.trailingSlashHandler(pattern).redirect(status); + filter = hasExcludePattern ? builder.exclude(excludePattern).build() : builder.build(); request = MockServerHttpRequest.get(path).contextPath(contextPath).build(); exchange = MockServerWebExchange.from(request);