Refactor PathPatternMessageMatcher

Signed-off-by: Pat McCusker <[email protected]>

Author: pat-mccusker

config/src/test/java/org/springframework/security/config/annotation/web/socket/WebSocketMessageBrokerSecurityConfigurationTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2025 the original author or authors.
+ * Copyright 2002-2024 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.

messaging/src/main/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManager.java

@@ -26,21 +26,23 @@
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.core.log.LogMessage;
+import org.springframework.http.server.PathContainer;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.simp.SimpMessageType;
 import org.springframework.security.authorization.AuthenticatedAuthorizationManager;
 import org.springframework.security.authorization.AuthorityAuthorizationManager;
 import org.springframework.security.authorization.AuthorizationDecision;
 import org.springframework.security.authorization.AuthorizationManager;
 import org.springframework.security.core.Authentication;
-import org.springframework.security.messaging.util.matcher.DestinationPathPatternMessageMatcher;
 import org.springframework.security.messaging.util.matcher.MessageMatcher;
+import org.springframework.security.messaging.util.matcher.PathPatternMessageMatcher;
 import org.springframework.security.messaging.util.matcher.SimpDestinationMessageMatcher;
 import org.springframework.security.messaging.util.matcher.SimpMessageTypeMatcher;
 import org.springframework.util.AntPathMatcher;
 import org.springframework.util.Assert;
 import org.springframework.util.PathMatcher;
 import org.springframework.util.function.SingletonSupplier;
+import org.springframework.web.util.pattern.PathPatternParser;
 
 public final class MessageMatcherDelegatingAuthorizationManager implements AuthorizationManager<Message<?>> {
 
@@ -136,7 +138,7 @@ public Builder.Constraint anyMessage() {
 		 * @return the Expression to associate
 		 */
 		public Builder.Constraint nullDestMatcher() {
-			return matchers(DestinationPathPatternMessageMatcher.NULL_DESTINATION_MATCHER);
+			return matchers(PathPatternMessageMatcher.NULL_DESTINATION_MATCHER);
 		}
 
 		/**
@@ -172,9 +174,9 @@ public Builder.Constraint simpDestMatchers(String... patterns) {
 		 * destinations match the provided {@code patterns}.
 		 * <p>
 		 * The matching of each pattern is performed by a
-		 * {@link DestinationPathPatternMessageMatcher} instance that matches
-		 * irrespectively of {@link SimpMessageType}. If no destination is found on the
-		 * {@code Message}, then each {@code Matcher} returns false.
+		 * {@link PathPatternMessageMatcher} instance that matches irrespectively of
+		 * {@link SimpMessageType}. If no destination is found on the {@code Message},
+		 * then each {@code Matcher} returns false.
 		 * </p>
 		 * @param patterns the destination path patterns to which the security
 		 * {@code Constraint} will be applicable
@@ -204,10 +206,9 @@ public Builder.Constraint simpMessageDestMatchers(String... patterns) {
 		 * {@code patterns}.
 		 * <p>
 		 * The matching of each pattern is performed by a
-		 * {@link DestinationPathPatternMessageMatcher}. If no destination is found on the
+		 * {@link PathPatternMessageMatcher}. If no destination is found on the
 		 * {@code Message}, then each {@code Matcher} returns false.
-		 * @param patterns the patterns to create
-		 * {@link DestinationPathPatternMessageMatcher} from.
+		 * @param patterns the patterns to create {@link PathPatternMessageMatcher} from.
 		 * @since 6.5
 		 */
 		public Builder.Constraint simpTypeMessageDestinationPatterns(String... patterns) {
@@ -234,10 +235,9 @@ public Builder.Constraint simpSubscribeDestMatchers(String... patterns) {
 		 * provided {@code patterns}.
 		 * <p>
 		 * The matching of each pattern is performed by a
-		 * {@link DestinationPathPatternMessageMatcher}. If no destination is found on the
+		 * {@link PathPatternMessageMatcher}. If no destination is found on the
 		 * {@code Message}, then each {@code Matcher} returns false.
-		 * @param patterns the patterns to create
-		 * {@link DestinationPathPatternMessageMatcher} from.
+		 * @param patterns the patterns to create {@link PathPatternMessageMatcher} from.
 		 * @since 6.5
 		 */
 		public Builder.Constraint simpTypeSubscribeDestinationPatterns(String... patterns) {
@@ -272,13 +272,12 @@ private Builder.Constraint simpDestMatchers(SimpMessageType type, String... patt
 		 * {@code patterns}.
 		 * <p>
 		 * The matching of each pattern is performed by a
-		 * {@link DestinationPathPatternMessageMatcher}. If no destination is found on the
+		 * {@link PathPatternMessageMatcher}. If no destination is found on the
 		 * {@code Message}, then each {@code Matcher} returns false.
 		 * </p>
 		 * @param type the {@link SimpMessageType} to match on. If null, the
 		 * {@link SimpMessageType} is not considered for matching.
-		 * @param patterns the patterns to create
-		 * {@link DestinationPathPatternMessageMatcher} from.
+		 * @param patterns the patterns to create {@link PathPatternMessageMatcher} from.
 		 * @return the {@link Builder.Constraint} that is associated to the
 		 * {@link MessageMatcher}s
 		 * @since 6.5
@@ -515,19 +514,21 @@ Map<String, String> extractPathVariables(Message<?> message) {
 
 		private static final class LazySimpDestinationPatternMessageMatcher implements MessageMatcher<Object> {
 
-			private final Supplier<DestinationPathPatternMessageMatcher> delegate;
+			private final Supplier<PathPatternMessageMatcher> delegate;
 
 			private LazySimpDestinationPatternMessageMatcher(String pattern, SimpMessageType type,
 					boolean useHttpPathSeparator) {
 				this.delegate = SingletonSupplier.of(() -> {
-					DestinationPathPatternMessageMatcher.Builder builder = (useHttpPathSeparator)
-							? DestinationPathPatternMessageMatcher.withDefaults()
-							: DestinationPathPatternMessageMatcher.messageRoute();
+					PathPatternParser dotSeparatedPathParser = new PathPatternParser();
+					dotSeparatedPathParser.setPathOptions(PathContainer.Options.MESSAGE_ROUTE);
+					PathPatternMessageMatcher.Builder builder = (useHttpPathSeparator)
+							? PathPatternMessageMatcher.withDefaults()
+							: PathPatternMessageMatcher.withPathPatternParser(dotSeparatedPathParser);
 					if (type == null) {
 						return builder.matcher(pattern);
 					}
 					if (SimpMessageType.MESSAGE == type || SimpMessageType.SUBSCRIBE == type) {
-						return builder.messageType(type).matcher(pattern);
+						return builder.matcher(pattern, type);
 					}
 					throw new IllegalStateException(type + " is not supported since it does not have a destination");
 				});

messaging/src/main/java/org/springframework/security/messaging/util/matcher/DestinationPathPatternMessageMatcher.java renamed to messaging/src/main/java/org/springframework/security/messaging/util/matcher/PathPatternMessageMatcher.java

@@ -19,61 +19,55 @@
 import java.util.Collections;
 import java.util.Map;
 
+import org.springframework.http.server.PathContainer;
 import org.springframework.messaging.Message;
 import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
 import org.springframework.messaging.simp.SimpMessageType;
 import org.springframework.util.Assert;
-import org.springframework.util.RouteMatcher;
+import org.springframework.web.util.pattern.PathPattern;
 import org.springframework.web.util.pattern.PathPatternParser;
-import org.springframework.web.util.pattern.PathPatternRouteMatcher;
 
 /**
- * Match {@link Message}s based on the message destination pattern, delegating the
- * matching to a {@link PathPatternRouteMatcher}. There is also support for optionally
- * matching on a specified {@link SimpMessageType}.
+ * Match {@link Message}s based on the message destination pattern using a
+ * {@link PathPattern}. There is also support for optionally matching on a specified
+ * {@link SimpMessageType}.
  *
  * @author Pat McCusker
  * @since 6.5
  */
-public final class DestinationPathPatternMessageMatcher implements MessageMatcher<Object> {
+public final class PathPatternMessageMatcher implements MessageMatcher<Object> {
 
 	public static final MessageMatcher<Object> NULL_DESTINATION_MATCHER = (message) -> getDestination(message) == null;
 
-	private static final PathPatternRouteMatcher SLASH_SEPARATED_ROUTE_MATCHER = new PathPatternRouteMatcher(
-			PathPatternParser.defaultInstance);
+	private final PathPattern pattern;
 
-	private static final PathPatternRouteMatcher DOT_SEPARATED_ROUTE_MATCHER = new PathPatternRouteMatcher();
-
-	private final String patternToMatch;
-
-	private final PathPatternRouteMatcher delegate;
+	private final PathPatternParser parser;
 
 	/**
 	 * The {@link MessageMatcher} that determines if the type matches. If the type was
 	 * null, this matcher will match every Message.
 	 */
 	private MessageMatcher<Object> messageTypeMatcher = ANY_MESSAGE;
 
-	private DestinationPathPatternMessageMatcher(String pattern, PathPatternRouteMatcher matcher) {
-		this.patternToMatch = pattern;
-		this.delegate = matcher;
+	private PathPatternMessageMatcher(PathPattern pattern, PathPatternParser parser) {
+		this.parser = parser;
+		this.pattern = pattern;
 	}
 
 	/**
-	 * Initialize this builder with a {@link PathPatternRouteMatcher} configured with the
+	 * Initialize this builder with the {@link PathPatternParser#defaultInstance} that is
+	 * configured with the
 	 * {@link org.springframework.http.server.PathContainer.Options#HTTP_PATH} separator
 	 */
 	public static Builder withDefaults() {
-		return new Builder(SLASH_SEPARATED_ROUTE_MATCHER);
+		return new Builder(PathPatternParser.defaultInstance);
 	}
 
 	/**
-	 * Initialize this builder with a {@link PathPatternRouteMatcher} configured with the
-	 * {@link org.springframework.http.server.PathContainer.Options#MESSAGE_ROUTE}
-	 * separator
+	 * Initialize this builder with the provided {@link PathPatternParser}
 	 */
-	public static Builder messageRoute() {
-		return new Builder(DOT_SEPARATED_ROUTE_MATCHER);
+	public static Builder withPathPatternParser(PathPatternParser parser) {
+		return new Builder(parser);
 	}
 
 	void setMessageTypeMatcher(MessageMatcher<Object> messageTypeMatcher) {
@@ -89,13 +83,13 @@ public boolean matches(Message<?> message) {
 			return false;
 		}
 
-		final String destination = getDestination(message);
+		String destination = getDestination(message);
 		if (destination == null) {
 			return false;
 		}
 
-		final RouteMatcher.Route destinationRoute = this.delegate.parseRoute(destination);
-		return this.delegate.match(this.patternToMatch, destinationRoute);
+		PathContainer destinationPathContainer = PathContainer.parsePath(destination, this.parser.getPathOptions());
+		return this.pattern.matches(destinationPathContainer);
 	}
 
 	/**
@@ -106,18 +100,18 @@ public boolean matches(Message<?> message) {
 	 * @throws IllegalStateException if the path does not match.
 	 */
 	public Map<String, String> extractPathVariables(Message<?> message) {
-		final String destination = getDestination(message);
+		String destination = getDestination(message);
 		if (destination == null) {
 			return Collections.emptyMap();
 		}
 
-		final RouteMatcher.Route destinationRoute = this.delegate.parseRoute(destination);
-		Map<String, String> pathMatchInfo = this.delegate.matchAndExtract(this.patternToMatch, destinationRoute);
+		PathContainer destinationPathContainer = PathContainer.parsePath(destination, this.parser.getPathOptions());
+		PathPattern.PathMatchInfo pathMatchInfo = this.pattern.matchAndExtract(destinationPathContainer);
 
 		Assert.state(pathMatchInfo != null,
-				"Pattern \"" + this.patternToMatch + "\" is not a match for \"" + destination + "\"");
+				"Pattern \"" + this.pattern.getPatternString() + "\" is not a match for \"" + destination + "\"");
 
-		return pathMatchInfo;
+		return pathMatchInfo.getUriVariables();
 	}
 
 	private static String getDestination(Message<?> message) {
@@ -126,30 +120,31 @@ private static String getDestination(Message<?> message) {
 
 	public static class Builder {
 
-		private final PathPatternRouteMatcher routeMatcher;
+		private final PathPatternParser parser;
 
 		private MessageMatcher<Object> messageTypeMatcher = ANY_MESSAGE;
 
-		Builder(PathPatternRouteMatcher matcher) {
-			this.routeMatcher = matcher;
-		}
-
-		public Builder messageType(SimpMessageType type) {
-			Assert.notNull(type, "Type must not be null");
-			this.messageTypeMatcher = new SimpMessageTypeMatcher(type);
-			return this;
+		Builder(PathPatternParser parser) {
+			this.parser = parser;
 		}
 
-		public DestinationPathPatternMessageMatcher matcher(String pattern) {
+		public PathPatternMessageMatcher matcher(String pattern) {
 			Assert.notNull(pattern, "Pattern must not be null");
-			DestinationPathPatternMessageMatcher matcher = new DestinationPathPatternMessageMatcher(pattern,
-					this.routeMatcher);
+			PathPattern pathPattern = this.parser.parse(pattern);
+			PathPatternMessageMatcher matcher = new PathPatternMessageMatcher(pathPattern, this.parser);
 			if (this.messageTypeMatcher != ANY_MESSAGE) {
 				matcher.setMessageTypeMatcher(this.messageTypeMatcher);
 			}
 			return matcher;
 		}
 
+		public PathPatternMessageMatcher matcher(String pattern, SimpMessageType type) {
+			Assert.notNull(type, "Type must not be null");
+			this.messageTypeMatcher = new SimpMessageTypeMatcher(type);
+
+			return matcher(pattern);
+		}
+
 	}
 
 }

messaging/src/main/java/org/springframework/security/messaging/util/matcher/SimpDestinationMessageMatcher.java

@@ -35,7 +35,7 @@
  *
  * @author Rob Winch
  * @since 4.0
- * @deprecated use {@link DestinationPathPatternMessageMatcher}
+ * @deprecated use {@link PathPatternMessageMatcher}
  */
 @Deprecated
 public final class SimpDestinationMessageMatcher implements MessageMatcher<Object> {

messaging/src/test/java/org/springframework/security/messaging/access/intercept/MessageMatcherDelegatingAuthorizationManagerTests.java

@@ -43,17 +43,17 @@ public final class MessageMatcherDelegatingAuthorizationManagerTests {
 	void checkWhenPermitAllThenPermits() {
 		AuthorizationManager<Message<?>> authorizationManager = builder().anyMessage().permitAll().build();
 		Message<?> message = new GenericMessage<>(new Object());
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isTrue();
 	}
 
 	@Test
 	void checkWhenAnyMessageHasRoleThenRequires() {
 		AuthorizationManager<Message<?>> authorizationManager = builder().anyMessage().hasRole("USER").build();
 		Message<?> message = new GenericMessage<>(new Object());
 		Authentication user = new TestingAuthenticationToken("user", "password", "ROLE_USER");
-		assertThat(authorizationManager.authorize(() -> user, message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(() -> user, message).isGranted()).isTrue();
 		Authentication admin = new TestingAuthenticationToken("user", "password", "ROLE_ADMIN");
-		assertThat(authorizationManager.authorize(() -> admin, message).isGranted()).isFalse();
+		assertThat(authorizationManager.check(() -> admin, message).isGranted()).isFalse();
 	}
 
 	@Test
@@ -66,7 +66,7 @@ void checkWhenSimpDestinationMatchesThenUses() {
 		MessageHeaders headers = new MessageHeaders(
 				Map.of(SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination"));
 		Message<?> message = new GenericMessage<>(new Object(), headers);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isTrue();
 	}
 
 	@Test
@@ -77,11 +77,11 @@ void checkWhenNullDestinationHeaderMatchesThenUses() {
 			.denyAll()
 			.build();
 		Message<?> message = new GenericMessage<>(new Object());
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isTrue();
 		MessageHeaders headers = new MessageHeaders(
 				Map.of(SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination"));
 		message = new GenericMessage<>(new Object(), headers);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isFalse();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isFalse();
 	}
 
 	@Test
@@ -94,7 +94,7 @@ void checkWhenSimpTypeMatchesThenUses() {
 		MessageHeaders headers = new MessageHeaders(
 				Map.of(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, SimpMessageType.CONNECT));
 		Message<?> message = new GenericMessage<>(new Object(), headers);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isTrue();
 	}
 
 	@Test
@@ -114,7 +114,7 @@ void checkWhenMessageTypeAndPathPatternMatches() {
 		MessageHeaders headers2 = new MessageHeaders(Map.of(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER,
 				SimpMessageType.SUBSCRIBE, SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination"));
 		Message<?> message2 = new GenericMessage<>(new Object(), headers2);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message2).isGranted()).isFalse();
+		assertThat(authorizationManager.check(mock(Supplier.class), message2).isGranted()).isFalse();
 	}
 
 	@Test
@@ -128,7 +128,7 @@ void checkWhenMessageRouteAndPatternMismatch() {
 		MessageHeaders headers = new MessageHeaders(
 				Map.of(SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination.sub.asdf"));
 		Message<?> message = new GenericMessage<>(new Object(), headers);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isFalse();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isFalse();
 	}
 
 	// gh-12540
@@ -142,7 +142,7 @@ void checkWhenSimpDestinationMatchesThenVariablesExtracted() {
 		MessageHeaders headers = new MessageHeaders(
 				Map.of(SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination/3"));
 		Message<?> message = new GenericMessage<>(new Object(), headers);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isTrue();
 	}
 
 	@Test
@@ -156,7 +156,7 @@ void checkWhenMessageRouteDestinationMatchesThenVariablesExtracted() {
 		MessageHeaders headers = new MessageHeaders(
 				Map.of(SimpMessageHeaderAccessor.DESTINATION_HEADER, "/destination.3"));
 		Message<?> message = new GenericMessage<>(new Object(), headers);
-		assertThat(authorizationManager.authorize(mock(Supplier.class), message).isGranted()).isTrue();
+		assertThat(authorizationManager.check(mock(Supplier.class), message).isGranted()).isTrue();
 	}
 
 	private MessageMatcherDelegatingAuthorizationManager.Builder builder() {