Support handler ordering in UrlHandlerFilter

Author: jamesmissen

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;
@@ -39,6 +41,7 @@
 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;
@@ -54,25 +57,26 @@
  * UrlHandlerFilter filter = UrlHandlerFilter
  *    .trailingSlashHandler("/path1/**").redirect(HttpStatus.PERMANENT_REDIRECT)
  *    .trailingSlashHandler("/path2/**").wrapRequest()
+ *    .exclude("/path1/foo/bar/**", "/path2/baz/")
  *    .build();
  * </pre>
  *
  * <p>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 {
 
 	private static final Log logger = LogFactory.getLog(UrlHandlerFilter.class);
 
 
-	private final MultiValueMap<Handler, PathPattern> handlers;
+	private final HandlerRegistry handlerRegistry;
 
 
-	private UrlHandlerFilter(MultiValueMap<Handler, PathPattern> handlers) {
-		this.handlers = new LinkedMultiValueMap<>(handlers);
+	private UrlHandlerFilter(HandlerRegistry handlerRegistry) {
+		this.handlerRegistry = handlerRegistry;
 	}
 
 
@@ -83,33 +87,26 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse
 		RequestPath path = (ServletRequestPathUtils.hasParsedRequestPath(request) ?
 				ServletRequestPathUtils.getParsedRequestPath(request) :
 				ServletRequestPathUtils.parse(request));
-
-		for (Map.Entry<Handler, List<PathPattern>> 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,
 	 * <code>"/path/&#42;"</code>, <code>"/path/&#42;&#42;"</code>,
 	 * <code>"/path/foo/"</code>.
 	 * @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);
 	}
 
 
@@ -119,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,
 		 * <code>"/path/&#42;"</code>, <code>"/path/&#42;&#42;"</code>,
 		 * <code>"/path/foo/"</code>.
 		 * @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.
+		 * <code>"/path/foo/&#42;"</code>, <code>"/path/foo/&#42;&#42;"</code>,
+		 * <code>"/path/foo/bar/"</code>.
+		 * @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.
+		 * <p>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.
@@ -169,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<Handler, String> handlers = new LinkedMultiValueMap<>();
 
-		private final MultiValueMap<Handler, PathPattern> 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<PathPattern> 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<Handler, List<String>> 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<PathPattern> pathPatterns;
+			private final String[] pathPatterns;
 
 			private @Nullable Consumer<HttpServletRequest> 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
@@ -210,19 +279,18 @@ public TrailingSlashSpec intercept(Consumer<HttpServletRequest> consumer) {
 			@Override
 			public Builder redirect(HttpStatusCode statusCode) {
 				Handler handler = new RedirectTrailingSlashHandler(statusCode, this.interceptor);
-				return DefaultBuilder.this.addHandler(this.pathPatterns, handler);
+				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.
 	 */
@@ -281,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();
+		}
 	}
 
 
@@ -312,6 +385,11 @@ public void handleInternal(HttpServletRequest request, HttpServletResponse respo
 			response.setHeader(HttpHeaders.LOCATION, location);
 			response.flushBuffer();
 		}
+
+		@Override
+		public String toString() {
+			return getClass().getSimpleName() + " {statusCode=" + this.statusCode.value() + "}";
+		}
 	}
 
 
@@ -405,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<Handler, PathPattern> 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<Handler, List<PathPattern>> 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<PathPattern, Handler> 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<PathPattern, Handler> 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;
+		}
+	}
+
 }