ARTEMIS-5183 STOMP noLocal filter ignores legitimate messages

apache · Jan 9, 2025 · 70c84ad · 70c84ad
1 parent bf1b3b7
commit 70c84ad
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 3 deletions.
diff --git a/...otocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/StompConnection.java b/...otocol/src/main/java/org/apache/activemq/artemis/core/protocol/stomp/StompConnection.java
@@ -565,7 +565,7 @@ StompPostReceiptFunction subscribe(String destination,
       checkDestination(destination);
       checkRoutingSemantics(destination, subscriptionType);
       if (noLocal) {
-         String noLocalFilter = CONNECTION_ID_PROPERTY_NAME_STRING + " <> '" + getID().toString() + "'";
+         String noLocalFilter = "(" + CONNECTION_ID_PROPERTY_NAME_STRING + " <> '" + getID().toString() + "' OR " + CONNECTION_ID_PROPERTY_NAME_STRING + " IS NULL)";
          if (selector == null) {
             selector = noLocalFilter;
          } else {

diff --git a/...on-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java b/...on-tests/src/test/java/org/apache/activemq/artemis/tests/integration/stomp/StompTest.java
@@ -1480,18 +1480,18 @@ public void testDurableUnSubscribeLegacySubscriptionHeader() throws Exception {
    }
 
    @Test
-   public void testSubscribeToTopicWithNoLocal() throws Exception {
+   public void testSubscribeToTopicWithNoLocalSendWithJMS() throws Exception {
       conn.connect(defUser, defPass);
       subscribeTopic(conn, null, null, null, true, true);
 
       // send a message on the same connection => it should not be received is noLocal = true on subscribe
       send(conn, getTopicPrefix() + getTopicName(), null, "Hello World");
 
       ClientStompFrame frame = conn.receiveFrame(100);
-      logger.debug("Received frame: {}", frame);
       assertNull(frame, "No message should have been received since subscription was removed");
 
       // send message on another JMS connection => it should be received
+      // a JMS message automatically gets a property named __AMQ_CID
       sendJmsMessage(getName(), topic);
       frame = conn.receiveFrame(10000);
       assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
@@ -1501,6 +1501,35 @@ public void testSubscribeToTopicWithNoLocal() throws Exception {
       conn.disconnect();
    }
 
+   @Test
+   public void testSubscribeToTopicWithNoLocalSendWithStomp() throws Exception {
+      conn.connect(defUser, defPass);
+      StompClientConnection conn2 = StompClientConnectionFactory.createClientConnection(uri);
+      conn2.connect(defUser, defPass);
+
+      try {
+         subscribeTopic(conn, null, null, null, true, true);
+
+         // send a message on the same connection => it should not be received as noLocal = true on subscribe
+         send(conn, getTopicPrefix() + getTopicName(), null, "Hello World");
+
+         ClientStompFrame frame = conn.receiveFrame(100);
+         assertNull(frame, "No message should have been received since subscription specified noLocal");
+
+         // send message on another STOMP connection => it should be received
+         // a STOMP message does *not* automatically get a property named __AMQ_CID if there aren't any noLocal subscriptions on the connection
+         send(conn2, getTopicPrefix() + getTopicName(), null, getName());
+         frame = conn.receiveFrame(10000);
+         assertNotNull(frame);
+         assertEquals(Stomp.Responses.MESSAGE, frame.getCommand());
+         assertEquals(getTopicPrefix() + getTopicName(), frame.getHeader(Stomp.Headers.Send.DESTINATION));
+         assertEquals(getName(), frame.getBody());
+      } finally {
+         conn.disconnect();
+         conn2.disconnect();
+      }
+   }
+
    @Test
    public void testSubscribeToTopicWithNoLocalAndSelector() throws Exception {
       conn.connect(defUser, defPass);