Add more transport types to the denied list for JMX (apache#1949) (apache#1953)

Cherry-picked from 5.19.x: f7e4726

Author: jgallimore

activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java

@@ -44,7 +44,9 @@ public class BrokerView implements BrokerViewMBean {
 
     private static final Logger LOG = LoggerFactory.getLogger(BrokerView.class);
 
-    private static final Set<String> DENIED_TRANSPORT_SCHEMES = Set.of("vm", "http");
+    public static final Set<String> DENIED_TRANSPORT_SCHEMES = Set.of("vm", "http",
+            "multicast", "zeroconf", "discovery", "fanout", "mock", "peer", "failover",
+            "proxy", "reliable", "simple", "udp");
 
     ManagedRegionBroker broker;
 

activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java

@@ -16,8 +16,7 @@
  */
 package org.apache.activemq.broker.jmx;
 
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
+import static org.apache.activemq.broker.jmx.BrokerView.DENIED_TRANSPORT_SCHEMES;
 
 import java.io.BufferedReader;
 import java.io.InputStreamReader;
@@ -67,7 +66,6 @@
 import org.apache.activemq.util.JMXSupport;
 import org.apache.activemq.util.URISupport;
 import org.apache.activemq.util.Wait;
-import org.junit.Test;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -2033,16 +2031,13 @@ public void testSubscriptionViewProperties() throws Exception {
         assertTrue(subscription.isExclusive());
     }
 
-    // Test to verify http transport is not allowed to be added as a connector
+    // Test to verify blocked transport schemes are not allowed to be added as a connector
     // through the Broker MBean
-    public void testAddHttpConnectorBlockedBrokerView() throws Exception {
-        testAddTransportConnectorBlockedBrokerView("http");
-    }
-
-    // Test to verify vm transport is not allowed to be added as a connector
-    // through the Broker MBean
-    public void testAddVmConnectorBlockedBrokerView() throws Exception {
-        testAddTransportConnectorBlockedBrokerView("vm");
+    public void testAddConnectorBlockedBrokerView() throws Exception {
+        for (String deniedScheme : DENIED_TRANSPORT_SCHEMES) {
+            LOG.info("verify testAddConnectorBlockedBrokerView scheme: {}", deniedScheme);
+            testAddTransportConnectorBlockedBrokerView(deniedScheme);
+        }
     }
 
     protected void testAddTransportConnectorBlockedBrokerView(String scheme) throws Exception {
@@ -2051,23 +2046,23 @@ protected void testAddTransportConnectorBlockedBrokerView(String scheme) throws
 
         try {
             brokerView.addConnector(scheme + "://localhost");
-            fail("Should have failed trying to add connector");
+            fail("Should have failed trying to add connector with scheme: " + scheme);
         } catch (IllegalArgumentException e) {
             assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
         }
 
         try {
             // verify any composite URI is blocked as well
-            brokerView.addConnector("failover:(tcp://0.0.0.0:0," + scheme + "://" + brokerName + ")");
-            fail("Should have failed trying to add connector");
+            brokerView.addConnector("static:(tcp://0.0.0.0:0," + scheme + "://" + brokerName + ")");
+            fail("Should have failed trying to add connector with scheme: " + scheme);
         } catch (IllegalArgumentException e) {
             assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
         }
 
         try {
             // verify nested composite URI is blocked
-            brokerView.addConnector("failover:(failover:(failover:(" + scheme + "://localhost)))");
-            fail("Should have failed trying to add connector");
+            brokerView.addConnector("static:(static:(static:(" + scheme + "://localhost)))");
+            fail("Should have failed trying to add connector with scheme: " + scheme);
         } catch (IllegalArgumentException e) {
             assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
         }
@@ -2081,7 +2076,7 @@ public void testNestedAddTransportConnector() throws Exception {
         try {
             // verify nested composite URI with more than 5 levels is blocked
             brokerView.addConnector(
-                    "static:(failover:(failover:(failover:(failover:(failover:(tcp://localhost:0))))))");
+                    "static:(static:(static:(static:(static:(static:(tcp://localhost:0))))))");
             fail("Should have failed trying to add vm connector bridge");
         } catch (IllegalArgumentException e) {
             assertEquals("URI can't contain more than 5 nested composite URIs", e.getMessage());

activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java

@@ -20,12 +20,14 @@
 import org.apache.activemq.broker.jmx.BrokerViewMBean;
 import org.apache.activemq.broker.jmx.NetworkConnectorViewMBean;
 import org.junit.After;
-import org.junit.AfterClass;
 import org.junit.Before;
 import org.junit.Test;
 
 import javax.management.ObjectName;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import static org.apache.activemq.broker.jmx.BrokerView.DENIED_TRANSPORT_SCHEMES;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.fail;
@@ -36,6 +38,8 @@
  */
 public class JmxCreateNCTest {
 
+    private static final Logger LOG = LoggerFactory.getLogger(JmxCreateNCTest.class);
+
     private static final String BROKER_NAME = "jmx-broker";
 
     private BrokerService broker;
@@ -79,43 +83,34 @@ public void testBridgeRegistration() throws Exception {
     }
 
     @Test
-    public void testVmBridgeBlocked() throws Exception {
-        testDeniedBridgeBlocked("vm");
-    }
-
-    @Test
-    public void testHttpBridgeBlocked() throws Exception {
-        testDeniedBridgeBlocked("http");
+    public void testTransportSchemeBridgeBlocked() throws Exception {
+        for (String deniedScheme : DENIED_TRANSPORT_SCHEMES) {
+            LOG.info("verify testTransportSchemeBridgeBlocked scheme: {}", deniedScheme);
+            testTransportSchemeBridgeBlocked(deniedScheme);
+        }
     }
 
-    protected void testDeniedBridgeBlocked(String scheme) throws Exception {
+    protected void testTransportSchemeBridgeBlocked(String scheme) throws Exception {
         // Test composite network connector uri
         try {
             proxy.addNetworkConnector("static:(" + scheme + "://localhost)");
-            fail("Should have failed trying to add connector bridge");
-        } catch (IllegalArgumentException e) {
-            assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
-        }
-
-        try {
-            proxy.addNetworkConnector("multicast:(" + scheme + "://localhost)");
-            fail("Should have failed trying to add connector bridge");
+            fail("Should have failed trying to add connector bridge with scheme: " + scheme);
         } catch (IllegalArgumentException e) {
             assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
         }
 
         // verify direct connector as well
         try {
             proxy.addNetworkConnector(scheme + "://localhost");
-            fail("Should have failed trying to add connector bridge");
+            fail("Should have failed trying to add connector bridge with scheme: " + scheme);
         } catch (IllegalArgumentException e) {
             assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
         }
 
         try {
             // verify nested composite URI is blocked
-            proxy.addNetworkConnector("static:(failover:(failover:(tcp://localhost:0," + scheme + "://localhost)))");
-            fail("Should have failed trying to add connector bridge");
+            proxy.addNetworkConnector("static:(static:(static:(tcp://localhost:0," + scheme + "://localhost)))");
+            fail("Should have failed trying to add connector bridge with scheme: " + scheme);
         } catch (IllegalArgumentException e) {
             assertEquals("Transport scheme '" + scheme + "' is not allowed", e.getMessage());
         }
@@ -131,7 +126,7 @@ public void testAddNetworkConnectorMaxComposite() throws Exception {
             // verify nested composite URI with more than 5 levels is blocked. This has 6 nested
             // (not including first wrapper url
             proxy.addNetworkConnector(
-                    "static:(failover:(failover:(failover:(failover:(failover:(tcp://localhost:0))))))");
+                    "static:(static:(static:(static:(static:(static:(tcp://localhost:0))))))");
             fail("Should have failed trying to add more than 5 connector bridges");
         } catch (IllegalArgumentException e) {
             assertEquals("URI can't contain more than 5 nested composite URIs", e.getMessage());