SEC-2827: Clean up MessageMatcher Ambiguities

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

@@ -33,7 +33,15 @@ import org.springframework.util.PathMatcher;
  * @author Rob Winch
  */
 public final class SimpDestinationMessageMatcher implements MessageMatcher<Object> {
+    public static final MessageMatcher<Object> NULL_DESTINATION_MATCHER = new MessageMatcher<Object>() {
+        public boolean matches(Message<? extends Object> message) {
+            String destination = SimpMessageHeaderAccessor.getDestination(message.getHeaders());
+            return destination == null;
+        }
+    };
+
     private final PathMatcher matcher;
+
     /**
      * The {@link MessageMatcher} that determines if the type matches. If the
      * type was null, this matcher will match every Message.
@@ -76,19 +84,16 @@ public final class SimpDestinationMessageMatcher implements MessageMatcher<Objec
         this(pattern, null);
     }
 
-
     /**
      * <p>
-     * Creates a new instance with the specified pattern and a {@link AntPathMatcher} created from the default
-     * constructor.
+     * Creates a new instance with the specified pattern and {@link PathMatcher}.
      * </p>
      *
      * @param pattern the pattern to use
-     * @param type the {@link SimpMessageType} to match on or null if any {@link SimpMessageType} should be matched.
      * @param pathMatcher the {@link PathMatcher} to use.
      */
-    public SimpDestinationMessageMatcher(String pattern, SimpMessageType type) {
-        this(pattern, type, new AntPathMatcher());
+    public SimpDestinationMessageMatcher(String pattern, PathMatcher pathMatcher) {
+        this(pattern, null, pathMatcher);
     }
 
     /**
@@ -100,9 +105,13 @@ public final class SimpDestinationMessageMatcher implements MessageMatcher<Objec
      * @param type the {@link SimpMessageType} to match on or null if any {@link SimpMessageType} should be matched.
      * @param pathMatcher the {@link PathMatcher} to use.
      */
-    public SimpDestinationMessageMatcher(String pattern, SimpMessageType type, PathMatcher pathMatcher) {
+    private SimpDestinationMessageMatcher(String pattern, SimpMessageType type, PathMatcher pathMatcher) {
         Assert.notNull(pattern, "pattern cannot be null");
         Assert.notNull(pathMatcher, "pathMatcher cannot be null");
+        if(!isTypeWithDestination(type)) {
+            throw new IllegalArgumentException("SimpMessageType " + type + " does not contain a destination and so cannot be matched on.");
+        }
+
         this.matcher = pathMatcher;
         this.messageTypeMatcher = type == null ? ANY_MESSAGE : new SimpMessageTypeMatcher(type);
         this.pattern = pattern;
@@ -117,16 +126,45 @@ public final class SimpDestinationMessageMatcher implements MessageMatcher<Objec
         return destination != null && matcher.match(pattern, destination);
     }
 
-
     public MessageMatcher<Object> getMessageTypeMatcher() {
         return messageTypeMatcher;
     }
 
-
     @Override
     public String toString() {
         return "SimpDestinationMessageMatcher [matcher=" + matcher
                 + ", messageTypeMatcher=" + messageTypeMatcher + ", pattern="
                 + pattern + "]";
     }
+
+    private boolean isTypeWithDestination(SimpMessageType type) {
+        if(type == null) {
+            return true;
+        }
+        return SimpMessageType.MESSAGE.equals(type) || SimpMessageType.SUBSCRIBE.equals(type);
+    }
+
+    /**
+     * <p>
+     * Creates a new instance with the specified pattern, {@code SimpMessageType.SUBSCRIBE}, and {@link PathMatcher}.
+     * </p>
+     *
+     * @param pattern the pattern to use
+     * @param pathMatcher the {@link PathMatcher} to use.
+     */
+    public static SimpDestinationMessageMatcher createSubscribeMatcher(String pattern, PathMatcher matcher) {
+        return new SimpDestinationMessageMatcher(pattern, SimpMessageType.SUBSCRIBE, matcher);
+    }
+
+    /**
+     * <p>
+     * Creates a new instance with the specified pattern, {@code SimpMessageType.MESSAGE}, and {@link PathMatcher}.
+     * </p>
+     *
+     * @param pattern the pattern to use
+     * @param pathMatcher the {@link PathMatcher} to use.
+     */
+    public static SimpDestinationMessageMatcher createMessageMatcher(String pattern, PathMatcher matcher) {
+        return new SimpDestinationMessageMatcher(pattern, SimpMessageType.MESSAGE, matcher);
+    }
 }

messaging/src/test/java/org/springframework/security/messaging/util/matcher/SimpDestinationMessageMatcherTests.java

@@ -22,6 +22,8 @@ import org.junit.Test;
 import org.springframework.messaging.simp.SimpMessageHeaderAccessor;
 import org.springframework.messaging.simp.SimpMessageType;
 import org.springframework.messaging.support.MessageBuilder;
+import org.springframework.util.AntPathMatcher;
+import org.springframework.util.PathMatcher;
 
 
 public class SimpDestinationMessageMatcherTests {
@@ -29,10 +31,13 @@ public class SimpDestinationMessageMatcherTests {
 
     SimpDestinationMessageMatcher matcher;
 
+    PathMatcher pathMatcher;
+
     @Before
     public void setup() {
         messageBuilder = MessageBuilder.withPayload("M");
         matcher = new SimpDestinationMessageMatcher("/**");
+        pathMatcher = new AntPathMatcher();
     }
 
     @Test(expected = IllegalArgumentException.class)
@@ -72,7 +77,7 @@ public class SimpDestinationMessageMatcherTests {
 
     @Test
     public void matchesFalseMessageTypeNotDisconnectType() throws Exception {
-        matcher = new SimpDestinationMessageMatcher("/match", SimpMessageType.MESSAGE);
+        matcher = SimpDestinationMessageMatcher.createMessageMatcher("/match", pathMatcher);
 
         messageBuilder.setHeader(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, SimpMessageType.DISCONNECT);
 
@@ -81,7 +86,7 @@ public class SimpDestinationMessageMatcherTests {
 
     @Test
     public void matchesTrueMessageType() throws Exception {
-        matcher = new SimpDestinationMessageMatcher("/match", SimpMessageType.MESSAGE);
+        matcher = SimpDestinationMessageMatcher.createMessageMatcher("/match", pathMatcher);
 
         messageBuilder.setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER,"/match");
         messageBuilder.setHeader(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, SimpMessageType.MESSAGE);
@@ -89,9 +94,19 @@ public class SimpDestinationMessageMatcherTests {
         assertThat(matcher.matches(messageBuilder.build())).isTrue();
     }
 
+    @Test
+    public void matchesTrueSubscribeType() throws Exception {
+        matcher = SimpDestinationMessageMatcher.createSubscribeMatcher("/match", pathMatcher);
+
+        messageBuilder.setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER,"/match");
+        messageBuilder.setHeader(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, SimpMessageType.SUBSCRIBE);
+
+        assertThat(matcher.matches(messageBuilder.build())).isTrue();
+    }
+
     @Test
     public void matchesNullMessageType() throws Exception {
-        matcher = new SimpDestinationMessageMatcher("/match", null);
+        matcher = new SimpDestinationMessageMatcher("/match");
 
         messageBuilder.setHeader(SimpMessageHeaderAccessor.DESTINATION_HEADER,"/match");
         messageBuilder.setHeader(SimpMessageHeaderAccessor.MESSAGE_TYPE_HEADER, SimpMessageType.MESSAGE);
@@ -101,11 +116,11 @@ public class SimpDestinationMessageMatcherTests {
 
     @Test
     public void typeConstructorParameterIsTransmitted() throws Exception {
-        matcher = new SimpDestinationMessageMatcher("/match", SimpMessageType.MESSAGE);
+        matcher = SimpDestinationMessageMatcher.createMessageMatcher("/match", pathMatcher);
 
         MessageMatcher<Object> expectedTypeMatcher = new SimpMessageTypeMatcher(SimpMessageType.MESSAGE);
 
-	assertThat(matcher.getMessageTypeMatcher()).isEqualTo(expectedTypeMatcher);
+        assertThat(matcher.getMessageTypeMatcher()).isEqualTo(expectedTypeMatcher);
 
     }