Minor bug fix for BrokerView#validateAllowedUri (apache#1900)

cshannon · web-flow · commit b787b0ed11cf · 2026-04-10T13:50:34.000-04:00
The wrong variable is being referenced in the nested loop. This does not cause the validation to actually break due to other checks done during the recursive call, but is incorrect either way. With this fix the counter needed tweaking for limiting the number of nested components as well. Follow on to apache#1847
diff --git a/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java b/activemq-broker/src/main/java/org/apache/activemq/broker/jmx/BrokerView.java
@@ -610,7 +610,8 @@ private static void validateAllowedUrl(String uriString) throws URISyntaxExcepti
     // 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) {
+        // If we are greater than 4 then this is the 5th level of composite
+        if (depth > 4) {
             throw new IllegalArgumentException("URI can't contain more than 5 nested composite URIs");
         }
 
@@ -625,10 +626,10 @@ private static void validateAllowedUri(URI uri, int depth) throws URISyntaxExcep
                 // 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)) {
+                if (URISupport.isCompositeURI(component)) {
                     validateAllowedUri(component, depth);
                 } else {
-                    validateAllowedScheme(uri.getScheme());
+                    validateAllowedScheme(component.getScheme());
                 }
             }
         }
diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/jmx/JmxCreateNCTest.java
@@ -116,11 +116,16 @@ public void testVmBridgeBlocked() throws Exception {
 
     @Test
     public void testAddNetworkConnectorMaxComposite() throws Exception {
+        // Should allow 5 nested (excludes first wrapper) so no exception thrown
+        assertNotNull(proxy.addNetworkConnector(
+                "static:(static:(static:(static:(static:(bad://localhost)))))"));
+
         try {
-            // verify nested composite URI with more than 5 levels is blocked
+            // 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))))))");
-            fail("Should have failed trying to add vm connector bridge");
+            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());
         }

Original file line number	Diff line number	Diff line change
`@@ -610,7 +610,8 @@ private static void validateAllowedUrl(String uriString) throws URISyntaxExcepti`
`610`	`610`	`// Validate the URI does not contain VM transport`
`611`	`611`	`private static void validateAllowedUri(URI uri, int depth) throws URISyntaxException {`
`612`	`612`	`// Don't allow more than 5 nested URIs to prevent blowing the stack`
`613`		`- if (depth > 5) {`
	`613`	`+ // If we are greater than 4 then this is the 5th level of composite`
	`614`	`+ if (depth > 4) {`
`614`	`615`	`throw new IllegalArgumentException("URI can't contain more than 5 nested composite URIs");`
`615`	`616`	`}`
`616`	`617`
`@@ -625,10 +626,10 @@ private static void validateAllowedUri(URI uri, int depth) throws URISyntaxExcep`
`625`	`626`	`// Each URI could be a nested composite URI so call validateAllowedUri()`
`626`	`627`	`// to validate it. This check if composite first so we don't add to`
`627`	`628`	`// the recursive stack depth if there's a lot of URIs that are not composite`
`628`		`- if (URISupport.isCompositeURI(uri)) {`
	`629`	`+ if (URISupport.isCompositeURI(component)) {`
`629`	`630`	`validateAllowedUri(component, depth);`
`630`	`631`	`} else {`
`631`		`- validateAllowedScheme(uri.getScheme());`
	`632`	`+ validateAllowedScheme(component.getScheme());`
`632`	`633`	`}`
`633`	`634`	`}`
`634`	`635`	`}`