Also validate nested composite URIs used with BrokerView (#1847)

Add a check for VM transports that are in a nested composite URI

This is a follow on to #1840

Author: cshannon

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

@@ -603,18 +603,33 @@ public long getTotalMaxUncommittedExceededCount() {
         return safeGetBroker().getDestinationStatistics().getMaxUncommittedExceededCount().getCount();
 	}
 
-
-    // Validate the Url does not contain VM transport
     private static void validateAllowedUrl(String uriString) throws URISyntaxException {
-        URI uri = new URI(uriString);
+        validateAllowedUri(new URI(uriString), 0);
+    }
+
+    // Validate the URI does not contain VM transport
+    private static void validateAllowedUri(URI uri, int depth) throws URISyntaxException {
+        // Don't allow more than 5 nested URIs to prevent blowing the stack
+        if (depth > 5) {
+            throw new IllegalArgumentException("URI can't contain more than 5 nested composite URIs");
+        }
+
         // First check the main URI scheme
         validateAllowedScheme(uri.getScheme());
 
-        // If composite, also check all schemes for each component
+        // If composite, iterate and check each of the composite URIs
         if (URISupport.isCompositeURI(uri)) {
             URISupport.CompositeData data = URISupport.parseComposite(uri);
+            depth++;
             for (URI component : data.getComponents()) {
-                validateAllowedScheme(component.getScheme());
+                // Each URI could be a nested composite URI so call validateAllowedUri()
+                // to validate it. This check if composite first so we don't add to
+                // the recursive stack depth if there's a lot of URIs that are not composite
+                if (URISupport.isCompositeURI(uri)) {
+                    validateAllowedUri(component, depth);
+                } else {
+                    validateAllowedScheme(uri.getScheme());
+                }
             }
         }
     }

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

@@ -68,6 +68,7 @@
 import org.apache.activemq.util.JMXSupport;
 import org.apache.activemq.util.URISupport;
 import org.apache.activemq.util.Wait;
+import org.junit.Test;
 import org.junit.experimental.categories.Category;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -2069,17 +2070,36 @@ public void testAddVmConnectorBlockedBrokerView() throws Exception {
 
         try {
             brokerView.addConnector("vm://localhost");
-            fail("Should have failed trying to add vm connector bridge");
+            fail("Should have failed trying to add vm connector");
         } catch (IllegalArgumentException e) {
             assertEquals("VM scheme is not allowed", e.getMessage());
         }
 
         try {
             // verify any composite URI is blocked as well
             brokerView.addConnector("failover:(tcp://0.0.0.0:0,vm://" + brokerName + ")");
-            fail("Should have failed trying to add vm connector bridge");
+            fail("Should have failed trying to add vm connector");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+
+        try {
+            // verify nested composite URI is blocked
+            brokerView.addConnector("failover:(failover:(failover:(vm://localhost)))");
+            fail("Should have failed trying to add vm connector");
         } catch (IllegalArgumentException e) {
             assertEquals("VM scheme is not allowed", e.getMessage());
         }
+
+        try {
+            // verify nested composite URI with more than 5 levels is blocked
+            brokerView.addConnector(
+                    "static:(failover:(failover:(failover:(failover:(failover:(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

@@ -104,5 +104,25 @@ public void testVmBridgeBlocked() throws Exception {
         } catch (IllegalArgumentException e) {
             assertEquals("VM scheme is not allowed", e.getMessage());
         }
+
+        try {
+            // verify nested composite URI is blocked
+            proxy.addNetworkConnector("static:(failover:(failover:(tcp://localhost:0,vm://localhost)))");
+            fail("Should have failed trying to add vm connector bridge");
+        } catch (IllegalArgumentException e) {
+            assertEquals("VM scheme is not allowed", e.getMessage());
+        }
+    }
+
+    @Test
+    public void testAddNetworkConnectorMaxComposite() throws Exception {
+        try {
+            // verify nested composite URI with more than 5 levels is blocked
+            proxy.addNetworkConnector(
+                    "static:(failover:(failover:(failover:(failover:(failover:(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());
+        }
     }
 }