Activemq 5.16.x tt.x apr patch

activemq-broker/src/main/java/org/apache/activemq/broker/BrokerService.java

@@ -1076,19 +1076,22 @@ public String getBrokerName() {
         return brokerName;
     }
 
+
+    // Matches a single character that is invalid in a broker name
+    private static final String INVALID_BROKER_NAME_CHAR_REG_EXP = "[^a-zA-Z0-9._\\-:]";
+
     /**
      * Sets the name of this broker; which must be unique in the network
      *
      * @param brokerName
      */
-    private static final String brokerNameReplacedCharsRegExp = "[^a-zA-Z0-9\\.\\_\\-\\:]";
     public void setBrokerName(String brokerName) {
         if (brokerName == null) {
             throw new NullPointerException("The broker name cannot be null");
         }
-        String str = brokerName.replaceAll(brokerNameReplacedCharsRegExp, "_");
+        String str = brokerName.replaceAll(INVALID_BROKER_NAME_CHAR_REG_EXP, "_");
         if (!str.equals(brokerName)) {
-            LOG.error("Broker Name: {} contained illegal characters matching regExp: {} - replaced with {}", brokerName, brokerNameReplacedCharsRegExp, str);
+            LOG.error("Broker Name: {} contained illegal characters matching regExp: {} - replaced with {}", brokerName, INVALID_BROKER_NAME_CHAR_REG_EXP, str);
         }
         this.brokerName = str.trim();
     }
@@ -2431,7 +2434,6 @@ protected Broker createRegionBroker(DestinationInterceptor destinationIntercepto
         }
         destinationFactory.setRegionBroker(regionBroker);
         regionBroker.setKeepDurableSubsActive(keepDurableSubsActive);
-        regionBroker.setBrokerName(getBrokerName());
         regionBroker.getDestinationStatistics().setEnabled(enableStatistics);
         regionBroker.setAllowTempAutoCreationOnSend(isAllowTempAutoCreationOnSend());
         if (brokerId != null) {

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

@@ -44,6 +44,11 @@ public class BrokerView implements BrokerViewMBean {
 
     private static final Logger LOG = LoggerFactory.getLogger(BrokerView.class);
 
+    public static final Set<String> DENIED_TRANSPORT_SCHEMES = Collections.unmodifiableSet(
+            new HashSet<>(Arrays.asList("vm", "http",
+            "multicast", "zeroconf", "discovery", "fanout", "mock", "peer", "failover",
+            "proxy", "reliable", "simple", "udp")));
+
     ManagedRegionBroker broker;
 
     private final BrokerService brokerService;
@@ -372,7 +377,7 @@ public ObjectName[] getDynamicDestinationProducers() {
 
     @Override
     public String addConnector(String discoveryAddress) throws Exception {
-        // Verify VM transport is not used
+        // Verify a denied transport scheme is not used
         validateAllowedUrl(discoveryAddress);
         TransportConnector connector = brokerService.addConnector(discoveryAddress);
         if (connector == null) {
@@ -384,7 +389,7 @@ public String addConnector(String discoveryAddress) throws Exception {
 
     @Override
     public String addNetworkConnector(String discoveryAddress) throws Exception {
-        // Verify VM transport is not used
+        // Verify a denied transport scheme is not used
         validateAllowedUrl(discoveryAddress);
         NetworkConnector connector = brokerService.addNetworkConnector(discoveryAddress);
         if (connector == null) {
@@ -552,10 +557,11 @@ private static void validateAllowedUrl(String uriString) throws URISyntaxExcepti
         validateAllowedUri(new URI(uriString), 0);
     }
 
-    // Validate the URI does not contain VM transport
+    // Validate the URI does not contain a denied transport scheme
     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");
         }
 
@@ -570,19 +576,22 @@ 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());
                 }
             }
         }
     }
 
-    // We don't allow VM transport scheme to be used
+    // Check all denied schemes
     private static void validateAllowedScheme(String scheme) {
-        if (scheme.equals("vm")) {
-            throw new IllegalArgumentException("VM scheme is not allowed");
+        for (String denied : DENIED_TRANSPORT_SCHEMES) {
+            // The schemes should be case-insensitive but ignore case as a precaution
+            if (scheme.equalsIgnoreCase(denied)) {
+                throw new IllegalArgumentException("Transport scheme '" + scheme + "' is not allowed");
+            }
         }
     }
 }

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

@@ -17,6 +17,7 @@
 package org.apache.activemq.broker.jmx;
 
 import java.io.IOException;
+import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -385,7 +386,7 @@ public String sendTextMessage(String body, String user, @Sensitive String passwo
     @Override
     public String sendTextMessage(Map<String, String> headers, String body, String userName, @Sensitive String password) throws Exception {
 
-        String brokerUrl = "vm://" + broker.getBrokerName();
+        URI brokerUrl = broker.getVmConnectorURI();
         ActiveMQDestination dest = destination.getActiveMQDestination();
 
         ActiveMQConnectionFactory cf = new ActiveMQConnectionFactory(brokerUrl);

activemq-broker/src/main/java/org/apache/activemq/broker/region/RegionBroker.java

@@ -24,6 +24,7 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.CopyOnWriteArrayList;
@@ -104,7 +105,7 @@ public class RegionBroker extends EmptyBroker {
 
     private final LongSequenceGenerator sequenceGenerator = new LongSequenceGenerator();
     private BrokerId brokerId;
-    private String brokerName;
+    private final String brokerName;
     private final Map<String, ConnectionContext> clientIdSet = new HashMap<String, ConnectionContext>();
     private final DestinationInterceptor destinationInterceptor;
     private ConnectionContext adminConnectionContext;
@@ -138,7 +139,8 @@ public void run() {
 
     public RegionBroker(BrokerService brokerService, TaskRunnerFactory taskRunnerFactory, SystemUsage memoryManager, DestinationFactory destinationFactory,
         DestinationInterceptor destinationInterceptor, Scheduler scheduler, ThreadPoolExecutor executor) throws IOException {
-        this.brokerService = brokerService;
+        this.brokerService = Objects.requireNonNull(brokerService);
+        this.brokerName = Objects.requireNonNull(brokerService.getBrokerName(), "The broker name cannot be null");
         this.executor = executor;
         this.scheduler = scheduler;
         if (destinationFactory == null) {
@@ -564,20 +566,9 @@ public void setBrokerId(BrokerId brokerId) {
 
     @Override
     public String getBrokerName() {
-        if (brokerName == null) {
-            try {
-                brokerName = InetAddressUtil.getLocalHostName().toLowerCase(Locale.ENGLISH);
-            } catch (Exception e) {
-                brokerName = "localhost";
-            }
-        }
         return brokerName;
     }
 
-    public void setBrokerName(String brokerName) {
-        this.brokerName = brokerName;
-    }
-
     public DestinationStatistics getDestinationStatistics() {
         return destinationStatistics;
     }

activemq-client/src/main/java/org/apache/activemq/transport/WriteTimeoutFilter.java

@@ -129,7 +129,12 @@ protected static boolean deRegisterWrite(WriteTimeoutFilter filter, boolean fail
 
     @Override
     public void start() throws Exception {
-        super.start();
+        try {
+            registerWrite(this);
+            super.start();
+        } finally {
+            deRegisterWrite(this, false, null);
+        }
     }
 
     @Override
@@ -157,8 +162,10 @@ public void run() {
                         while (run && filters.hasNext()) {
                             WriteTimeoutFilter filter = filters.next();
                             if (filter.getWriteTimeout()<=0) continue; //no timeout set
-                            long writeStart = filter.getWriter().getWriteTimestamp();
-                            long delta = (filter.getWriter().isWriting() && writeStart>0)?System.currentTimeMillis() - writeStart:-1;
+                            TimeStampStream writer = filter.getWriter();
+                            if (writer == null) continue; //stream not yet initialized
+                            long writeStart = writer.getWriteTimestamp();
+                            long delta = (writer.isWriting() && writeStart>0)?System.currentTimeMillis() - writeStart:-1;
                             if (delta>filter.getWriteTimeout()) {
                                 WriteTimeoutFilter.deRegisterWrite(filter, true,null);
                             }//if timeout

activemq-client/src/main/java/org/apache/activemq/util/IntrospectionSupport.java

@@ -169,6 +169,9 @@ public static boolean setProperty(Object target, String name, Object value) {
             if (target instanceof SSLServerSocket) {
                 // overcome illegal access issues with internal implementation class
                 clazz = SSLServerSocket.class;
+            } else if (target instanceof javax.net.ssl.SSLSocket) {
+                // overcome illegal access issues with internal implementation class
+                clazz = javax.net.ssl.SSLSocket.class;
             }
             Method setter = findSetterMethod(clazz, name);
             if (setter == null) {

activemq-jms-pool/src/main/java/org/apache/activemq/jms/pool/IntrospectionSupport.java

@@ -22,6 +22,7 @@
 import java.util.Map.Entry;
 
 import javax.net.ssl.SSLServerSocket;
+import javax.net.ssl.SSLSocket;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -60,6 +61,9 @@ public static boolean setProperty(Object target, String name, Object value) {
             if (target instanceof SSLServerSocket) {
                 // overcome illegal access issues with internal implementation class
                 clazz = SSLServerSocket.class;
+            } else if (target instanceof javax.net.ssl.SSLSocket) {
+                // overcome illegal access issues with internal implementation class
+                clazz = javax.net.ssl.SSLSocket.class;
             }
             Method setter = findSetterMethod(clazz, name);
             if (setter == null) {

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,101 @@
 
 public class Utils {
 
+    public static final String FILE_PROTOCOL = "file";
+    public static final String CLASSPATH_PROTOCOL = "classpath";
+    // Special marker to indicate we want to allow remote files such as
+    // Windows UNC, etc
+    public static final String REMOTE_FILE_PROTOCOL = "remote-" + FILE_PROTOCOL;
+
     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, uri) && 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 the protocol"
+                    + " is not allowed for loading resources");
         }
         return resource;
     }
+
+    // These method treats local files and remote files (that are pre-fixed
+    // with two forward/backward slashes) differently
+    static boolean isAllowFile(Set<String> allowedProtocols, String uri) {
+        if (allowedProtocols == null) {
+            return true;
+        }
+        return isUnqualifiedRemoteFile(uri) ? allowedProtocols.contains(REMOTE_FILE_PROTOCOL) :
+                allowedProtocols.contains(FILE_PROTOCOL);
+    }
+
+    static boolean isAllowClasspath(Set<String> allowedProtocols) {
+        return allowedProtocols == null || allowedProtocols.contains(CLASSPATH_PROTOCOL);
+    }
+
+    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) {
+            final String detectedProtocol = getProtocolFromScheme(uriString);
+            if (detectedProtocol == null) {
+                throw new IllegalArgumentException("Could not detect protocol in given URI [" + uriString + "]");
+            }
+            if (!allowedProtocols.contains(detectedProtocol)){
+                throw new IllegalArgumentException("URL [" + uriString +
+                        "] uses protocol '" + detectedProtocol + "' which is not allowed "
+                        + "for loading URL resources");
+            }
+        }
+    }
+
+    // If this is a qualified remote file then we return a special marker
+    // so that we know this is trying to access a remote resource as that will be
+    // validated differently
+    private static String getProtocolFromScheme(String uriString) throws URISyntaxException {
+        return isQualifiedRemoteFile(uriString) ? REMOTE_FILE_PROTOCOL :
+                new URI(uriString).getScheme();
+    }
+
+    private static boolean isUnqualifiedRemoteFile(String uri) {
+        return uri.startsWith("//") || uri.startsWith("\\\\");
+    }
+
+    private static boolean isQualifiedRemoteFile(String uri) {
+        return uri.startsWith(FILE_PROTOCOL + "://") ||  uri.startsWith(FILE_PROTOCOL + ":\\\\");
+    }
 }