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 index 2d0dfb1fca0..04163335475 100644 --- 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 @@ -563,18 +563,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()); + } } } } diff --git a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java index 3751e1d3cb1..e3222d7b490 100644 --- a/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java +++ b/activemq-unit-tests/src/test/java/org/apache/activemq/broker/jmx/MBeanTest.java @@ -67,6 +67,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.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -2067,7 +2068,7 @@ 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()); } @@ -2075,9 +2076,28 @@ public void testAddVmConnectorBlockedBrokerView() throws Exception { 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()); + } + } + } 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 index 6d14f8fcfb3..141e5c0ee82 100644 --- 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 @@ -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()); + } } }