Merge branch 'main' into broker-mbean-transport-filter

Author: cshannon

activemq-spring/src/main/java/org/apache/activemq/spring/Utils.java

@@ -19,6 +19,9 @@
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.net.MalformedURLException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Set;
 import org.springframework.core.io.ClassPathResource;
 import org.springframework.core.io.FileSystemResource;
 import org.springframework.core.io.Resource;
@@ -27,22 +30,69 @@
 
 public class Utils {
 
+    public static final String FILE_PROTOCOL = "file";
+    public static final String CLASSPATH_PROTOCOL = "classpath";
+
     public static Resource resourceFromString(String uri) throws MalformedURLException {
-        Resource resource;
-        File file = new File(uri);
-        if (file.exists()) {
+        // default allows all
+        return resourceFromString(uri, null);
+    }
+
+    public static Resource resourceFromString(String uri, Set<String> allowedProtocols) throws MalformedURLException {
+        // Empty set means nothing is allowed
+        if (allowedProtocols != null && allowedProtocols.isEmpty()) {
+            throw new IllegalArgumentException("No protocols are allowed for loading resources.");
+        }
+
+        final Resource resource;
+
+        // First, just try and load a local file (if it exists) and if "file"
+        // as part of the allow list. This preserves previous behavior of
+        // always optimistically trying a local file first.
+        if (isAllowFile(allowedProtocols) && new File(uri).exists()) {
             resource = new FileSystemResource(uri);
+        // If file isn't allowed, or if the file can't be found then check
+        // if the string is a valid URL. If it's valid, then we need
+        // to validate if it's allowed before loading the URL.
+        // isUrl() uses URI internally so it won't actually load anything
         } else if (ResourceUtils.isUrl(uri)) {
             try {
+                validateUrlAllowed(uri, allowedProtocols);
                 resource = new UrlResource(ResourceUtils.getURL(uri));
-            } catch (FileNotFoundException e) {
+            } catch (FileNotFoundException | URISyntaxException e) {
                 MalformedURLException malformedURLException = new MalformedURLException(uri);
                 malformedURLException.initCause(e);
-                throw  malformedURLException;
+                throw malformedURLException;
             }
-        } else {
+        // Fallback to trying on the classpath if not a valid Url, and we allow it which
+        // also preserves the previous behavior (if classpath is allowed)
+        } else if (isAllowClasspath(allowedProtocols)){
             resource = new ClassPathResource(uri);
+        // Catch all fail-safe if nothing else matches. This could happen if file is allowed
+        // but not classpath but the file doesn't exist
+        } else {
+            throw new IllegalArgumentException("URL [" + uri + "] can't be found or is not allowed"
+                    + " for loading resources");
         }
         return resource;
     }
+
+    static boolean isAllowFile(Set<String> allowedProtocols) {
+        return allowedProtocols == null || allowedProtocols.contains(FILE_PROTOCOL);
+    }
+
+    static boolean isAllowClasspath(Set<String> allowedProtocols) {
+        return allowedProtocols == null || allowedProtocols.contains(CLASSPATH_PROTOCOL);
+    }
+
+    private static void validateUrlAllowed(String uriString, Set<String> allowedProtocols)
+            throws URISyntaxException {
+        // Use new URI() to get the scheme
+        // This is important because ResourceUtils.getURL() actually searches
+        // the classpath which we don't want to do if not allowed
+        if (allowedProtocols != null && !allowedProtocols.contains(new URI(uriString).getScheme())) {
+            throw new IllegalArgumentException("URL [" + uriString +
+                    "] does not use an allowed protocol for loading URL resources");
+        }
+    }
 }

activemq-spring/src/main/java/org/apache/activemq/xbean/XBeanBrokerFactory.java

@@ -20,7 +20,9 @@
 import java.net.MalformedURLException;
 import java.net.URI;
 
-import org.apache.activemq.broker.BrokerContextAware;
+import java.util.Arrays;
+import java.util.Set;
+import java.util.stream.Collectors;
 import org.apache.activemq.broker.BrokerFactoryHandler;
 import org.apache.activemq.broker.BrokerService;
 import org.apache.activemq.spring.SpringBrokerContext;
@@ -35,20 +37,38 @@
 import org.springframework.beans.FatalBeanException;
 import org.springframework.beans.factory.xml.XmlBeanDefinitionReader;
 import org.springframework.context.ApplicationContext;
-import org.springframework.context.ApplicationContextAware;
 import org.springframework.core.io.Resource;
 
 /**
  * 
  */
 public class XBeanBrokerFactory implements BrokerFactoryHandler {
-    private static final transient Logger LOG = LoggerFactory.getLogger(XBeanBrokerFactory.class);
+    private static final Logger LOG = LoggerFactory.getLogger(XBeanBrokerFactory.class);
+
+    public static final String XBEAN_BROKER_FACTORY_PROTOCOLS_PROP =
+            "org.apache.activemq.xbean.XBEAN_BROKER_FACTORY_PROTOCOLS";
+    public static final String DEFAULT_ALLOWED_PROTOCOLS = Utils.FILE_PROTOCOL + "," + Utils.CLASSPATH_PROTOCOL;
+
+    private final Set<String> allowedProtocols;
 
     static {
         PropertyEditorManager.registerEditor(URI.class, URIEditor.class);
     }
 
+    public XBeanBrokerFactory() {
+        final String allowedProtocols = System.getProperty(XBEAN_BROKER_FACTORY_PROTOCOLS_PROP,
+                DEFAULT_ALLOWED_PROTOCOLS);
+
+        // Asterisk will map to null which will allow all and skip checking
+        // Empty string will map to an empty set and will deny all
+        this.allowedProtocols = !allowedProtocols.equals("*") ?
+                Arrays.stream(allowedProtocols.split("\\s*,\\s*"))
+                .filter(s -> !s.isBlank())
+                .collect(Collectors.toUnmodifiableSet()) : null;
+    }
+
     private boolean validate = true;
+
     public boolean isValidate() {
         return validate;
     }
@@ -75,12 +95,10 @@ public BrokerService createBroker(URI config) throws Exception {
         if (broker == null) {
             // lets try find by type
             String[] names = context.getBeanNamesForType(BrokerService.class);
-            for (int i = 0; i < names.length; i++) {
-                String name = names[i];
-                broker = (BrokerService)context.getBean(name);
-                if (broker != null) {
-                    break;
-                }
+            for (String name : names) {
+                // No need to check for null, this will throw an exception if not found
+                broker = (BrokerService) context.getBean(name);
+                break;
             }
         }
         if (broker == null) {
@@ -98,8 +116,8 @@ public BrokerService createBroker(URI config) throws Exception {
     }
 
     protected ApplicationContext createApplicationContext(String uri) throws MalformedURLException {
-        Resource resource = Utils.resourceFromString(uri);
-        LOG.debug("Using " + resource + " from " + uri);
+        Resource resource = Utils.resourceFromString(uri, allowedProtocols);
+        LOG.debug("Using {} from {}", resource, uri);
         try {
             return new ResourceXmlApplicationContext(resource) {
                 @Override
@@ -108,9 +126,14 @@ protected void initBeanDefinitionReader(XmlBeanDefinitionReader reader) {
                 }
             };
         } catch (FatalBeanException errorToLog) {
-            LOG.error("Failed to load: " + resource + ", reason: " + errorToLog.getLocalizedMessage(), errorToLog);
+            LOG.error("Failed to load: {}, reason: {}", resource, errorToLog.getLocalizedMessage(),
+                    errorToLog);
             throw errorToLog;
         }
     }
 
+    // Package scope for testing
+    Set<String> getAllowedProtocols() {
+        return allowedProtocols;
+    }
 }