This is an automated email from the ASF dual-hosted git repository.
cshannon pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq.git
The following commit(s) were added to refs/heads/main by this push:
new c9d3963f20 Improve FactoryFinder Validation (#1793)
c9d3963f20 is described below
commit c9d3963f20577cd1995fa2b442ec15f8c806e932
Author: Christopher L. Shannon <[email protected]>
AuthorDate: Wed Mar 18 09:40:15 2026 -0400
Improve FactoryFinder Validation (#1793)
Also modernizes with generics and adds improved testing
---
.../org/apache/activemq/broker/BrokerFactory.java | 6 +-
.../broker/region/group/GroupFactoryFinder.java | 6 +-
.../transport/auto/AutoTcpTransportServer.java | 12 +-
.../org/apache/activemq/util/osgi/Activator.java | 23 ++-
.../activemq/transport/TransportFactory.java | 12 +-
.../transport/discovery/DiscoveryAgentFactory.java | 6 +-
.../org/apache/activemq/util/FactoryFinder.java | 181 +++++++++++++---
.../apache/activemq/util/FactoryFinderTest.java | 202 ++++++++++++++++++
.../store/jdbc/JDBCPersistenceAdapter.java | 10 +-
.../transport/mqtt/MQTTProtocolConverter.java | 8 +-
activemq-spring/pom.xml | 2 +-
.../transport/stomp/ProtocolConverter.java | 75 ++++---
.../transport/stomp/ProtocolConverterTest.java | 192 +++++++++++++++++
.../apache/activemq/transport/stomp/StompTest.java | 117 +++++++++++
.../activemq/transport/frametranslator/stomp-test | 17 ++
.../transport/frametranslator/stomp-test-invalid | 17 ++
.../apache/activemq/web/QueueBrowseServlet.java | 31 +--
.../activemq/web/QueueBrowseServletTest.java | 230 +++++++++++++++++++++
.../org/apache/activemq/web/view/invalid-view | 19 ++
.../org/apache/activemq/web/view/valid-view | 19 ++
20 files changed, 1087 insertions(+), 98 deletions(-)
diff --git
a/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerFactory.java
b/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerFactory.java
index 9dfb85952e..3d635edf9a 100644
---
a/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerFactory.java
+++
b/activemq-broker/src/main/java/org/apache/activemq/broker/BrokerFactory.java
@@ -31,14 +31,16 @@ import org.apache.activemq.util.IOExceptionSupport;
*/
public final class BrokerFactory {
- private static final FactoryFinder BROKER_FACTORY_HANDLER_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/broker/");
+ private static final FactoryFinder<BrokerFactoryHandler>
BROKER_FACTORY_HANDLER_FINDER
+ = new
FactoryFinder<>("META-INF/services/org/apache/activemq/broker/",
BrokerFactoryHandler.class,
+ null);
private BrokerFactory() {
}
public static BrokerFactoryHandler createBrokerFactoryHandler(String type)
throws IOException {
try {
- return
(BrokerFactoryHandler)BROKER_FACTORY_HANDLER_FINDER.newInstance(type);
+ return BROKER_FACTORY_HANDLER_FINDER.newInstance(type);
} catch (Throwable e) {
throw IOExceptionSupport.create("Could not load " + type + "
factory:" + e, e);
}
diff --git
a/activemq-broker/src/main/java/org/apache/activemq/broker/region/group/GroupFactoryFinder.java
b/activemq-broker/src/main/java/org/apache/activemq/broker/region/group/GroupFactoryFinder.java
index 718b37d708..78092eb316 100644
---
a/activemq-broker/src/main/java/org/apache/activemq/broker/region/group/GroupFactoryFinder.java
+++
b/activemq-broker/src/main/java/org/apache/activemq/broker/region/group/GroupFactoryFinder.java
@@ -25,7 +25,9 @@ import org.apache.activemq.util.IntrospectionSupport;
import org.apache.activemq.util.URISupport;
public class GroupFactoryFinder {
- private static final FactoryFinder GROUP_FACTORY_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/groups/");
+ private static final FactoryFinder<MessageGroupMapFactory>
GROUP_FACTORY_FINDER =
+ new
FactoryFinder<>("META-INF/services/org/apache/activemq/groups/",
MessageGroupMapFactory.class,
+ null);
private GroupFactoryFinder() {
}
@@ -40,7 +42,7 @@ public class GroupFactoryFinder {
factoryType = factoryType.substring(0,p);
properties = URISupport.parseQuery(propertiesString);
}
- MessageGroupMapFactory result =
(MessageGroupMapFactory)GROUP_FACTORY_FINDER.newInstance(factoryType);
+ MessageGroupMapFactory result =
GROUP_FACTORY_FINDER.newInstance(factoryType);
if (properties != null && result != null){
IntrospectionSupport.setProperties(result,properties);
}
diff --git
a/activemq-broker/src/main/java/org/apache/activemq/transport/auto/AutoTcpTransportServer.java
b/activemq-broker/src/main/java/org/apache/activemq/transport/auto/AutoTcpTransportServer.java
index bcd73653d0..b2ea7c06c1 100644
---
a/activemq-broker/src/main/java/org/apache/activemq/transport/auto/AutoTcpTransportServer.java
+++
b/activemq-broker/src/main/java/org/apache/activemq/transport/auto/AutoTcpTransportServer.java
@@ -80,15 +80,19 @@ public class AutoTcpTransportServer extends
TcpTransportServer {
protected int maxConnectionThreadPoolSize = Integer.MAX_VALUE;
protected int protocolDetectionTimeOut = 30000;
- private static final FactoryFinder TRANSPORT_FACTORY_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/transport/");
+ private static final FactoryFinder<TransportFactory>
TRANSPORT_FACTORY_FINDER =
+ new
FactoryFinder<>("META-INF/services/org/apache/activemq/transport/",
TransportFactory.class,
+ null);
private final ConcurrentMap<String, TransportFactory> transportFactories =
new ConcurrentHashMap<String, TransportFactory>();
- private static final FactoryFinder WIREFORMAT_FACTORY_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/wireformat/");
+ private static final FactoryFinder<WireFormatFactory>
WIREFORMAT_FACTORY_FINDER =
+ new
FactoryFinder<>("META-INF/services/org/apache/activemq/wireformat/",
WireFormatFactory.class,
+ null);
public WireFormatFactory findWireFormatFactory(String scheme, Map<String,
Map<String, Object>> options) throws IOException {
WireFormatFactory wff = null;
try {
- wff =
(WireFormatFactory)WIREFORMAT_FACTORY_FINDER.newInstance(scheme);
+ wff = WIREFORMAT_FACTORY_FINDER.newInstance(scheme);
if (options != null) {
final Map<String, Object> wfOptions = new HashMap<>();
if (options.get(AutoTransportUtils.ALL) != null) {
@@ -117,7 +121,7 @@ public class AutoTcpTransportServer extends
TcpTransportServer {
if (tf == null) {
// Try to load if from a META-INF property.
try {
- tf =
(TransportFactory)TRANSPORT_FACTORY_FINDER.newInstance(scheme);
+ tf = TRANSPORT_FACTORY_FINDER.newInstance(scheme);
if (options != null) {
IntrospectionSupport.setProperties(tf, options);
}
diff --git
a/activemq-broker/src/main/java/org/apache/activemq/util/osgi/Activator.java
b/activemq-broker/src/main/java/org/apache/activemq/util/osgi/Activator.java
index 00ca2154b3..ddc2306c9b 100644
--- a/activemq-broker/src/main/java/org/apache/activemq/util/osgi/Activator.java
+++ b/activemq-broker/src/main/java/org/apache/activemq/util/osgi/Activator.java
@@ -172,7 +172,15 @@ public class Activator implements BundleActivator,
SynchronousBundleListener, Ob
// ================================================================
@Override
- public Object create(String path) throws IllegalAccessException,
InstantiationException, IOException, ClassNotFoundException {
+ public Object create(String path)
+ throws IllegalAccessException, InstantiationException,
IOException, ClassNotFoundException {
+ throw new UnsupportedOperationException("Create is not supported
without requiredType and allowed impls");
+ }
+
+ @SuppressWarnings("unchecked")
+ @Override
+ public <T> T create(String path, Class<T> requiredType, Set<Class<?
extends T>> allowedImpls)
+ throws IllegalAccessException, InstantiationException,
IOException, ClassNotFoundException {
Class<?> clazz = serviceCache.get(path);
if (clazz == null) {
StringBuilder warnings = new StringBuilder();
@@ -199,6 +207,10 @@ public class Activator implements BundleActivator,
SynchronousBundleListener, Ob
continue;
}
+ // no reason to cache if invalid so validate before caching
+ // the class inside of serviceCache
+ FactoryFinder.validateClass(clazz, requiredType, allowedImpls);
+
// Yay.. the class was found. Now cache it.
serviceCache.put(path, clazz);
wrapper.cachedServices.add(path);
@@ -214,10 +226,17 @@ public class Activator implements BundleActivator,
SynchronousBundleListener, Ob
}
throw new IOException(msg);
}
+ } else {
+ // Validate again (even for previously cached classes) in case
+ // a path is re-used with a different requiredType.
+ // This object factory is shared by all factory finder instances,
so it would be
+ // possible (although probably a mistake) to use the same
+ // path again with a different requiredType in a different
FactoryFinder
+ FactoryFinder.validateClass(clazz, requiredType, allowedImpls);
}
try {
- return clazz.getConstructor().newInstance();
+ return (T) clazz.getConstructor().newInstance();
} catch (InvocationTargetException | NoSuchMethodException e) {
throw new InstantiationException(e.getMessage());
}
diff --git
a/activemq-client/src/main/java/org/apache/activemq/transport/TransportFactory.java
b/activemq-client/src/main/java/org/apache/activemq/transport/TransportFactory.java
index acfc1b0389..184a5d6391 100644
---
a/activemq-client/src/main/java/org/apache/activemq/transport/TransportFactory.java
+++
b/activemq-client/src/main/java/org/apache/activemq/transport/TransportFactory.java
@@ -36,8 +36,12 @@ import org.apache.activemq.wireformat.WireFormatFactory;
public abstract class TransportFactory {
- private static final FactoryFinder TRANSPORT_FACTORY_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/transport/");
- private static final FactoryFinder WIREFORMAT_FACTORY_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/wireformat/");
+ private static final FactoryFinder<TransportFactory>
TRANSPORT_FACTORY_FINDER
+ = new
FactoryFinder<>("META-INF/services/org/apache/activemq/transport/",
TransportFactory.class,
+ null);
+ private static final FactoryFinder<WireFormatFactory>
WIREFORMAT_FACTORY_FINDER
+ = new
FactoryFinder<>("META-INF/services/org/apache/activemq/wireformat/",
WireFormatFactory.class,
+ null);
private static final ConcurrentMap<String, TransportFactory>
TRANSPORT_FACTORYS = new ConcurrentHashMap<String, TransportFactory>();
private static final String WRITE_TIMEOUT_FILTER = "soWriteTimeout";
@@ -179,7 +183,7 @@ public abstract class TransportFactory {
if (tf == null) {
// Try to load if from a META-INF property.
try {
- tf =
(TransportFactory)TRANSPORT_FACTORY_FINDER.newInstance(scheme);
+ tf = TRANSPORT_FACTORY_FINDER.newInstance(scheme);
TRANSPORT_FACTORYS.put(scheme, tf);
} catch (Throwable e) {
throw IOExceptionSupport.create("Transport scheme NOT
recognized: [" + scheme + "]", e);
@@ -201,7 +205,7 @@ public abstract class TransportFactory {
}
try {
- WireFormatFactory wff =
(WireFormatFactory)WIREFORMAT_FACTORY_FINDER.newInstance(wireFormat);
+ WireFormatFactory wff =
WIREFORMAT_FACTORY_FINDER.newInstance(wireFormat);
IntrospectionSupport.setProperties(wff, options, "wireFormat.");
return wff;
} catch (Throwable e) {
diff --git
a/activemq-client/src/main/java/org/apache/activemq/transport/discovery/DiscoveryAgentFactory.java
b/activemq-client/src/main/java/org/apache/activemq/transport/discovery/DiscoveryAgentFactory.java
index 65f3ee6933..46894d4729 100644
---
a/activemq-client/src/main/java/org/apache/activemq/transport/discovery/DiscoveryAgentFactory.java
+++
b/activemq-client/src/main/java/org/apache/activemq/transport/discovery/DiscoveryAgentFactory.java
@@ -26,7 +26,9 @@ import org.apache.activemq.util.IOExceptionSupport;
public abstract class DiscoveryAgentFactory {
- private static final FactoryFinder DISCOVERY_AGENT_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/transport/discoveryagent/");
+ private static final FactoryFinder<DiscoveryAgentFactory>
DISCOVERY_AGENT_FINDER =
+ new
FactoryFinder<>("META-INF/services/org/apache/activemq/transport/discoveryagent/",
+ DiscoveryAgentFactory.class, null);
private static final ConcurrentMap<String, DiscoveryAgentFactory>
DISCOVERY_AGENT_FACTORYS = new ConcurrentHashMap<String,
DiscoveryAgentFactory>();
/**
@@ -43,7 +45,7 @@ public abstract class DiscoveryAgentFactory {
if (daf == null) {
// Try to load if from a META-INF property.
try {
- daf =
(DiscoveryAgentFactory)DISCOVERY_AGENT_FINDER.newInstance(scheme);
+ daf = DISCOVERY_AGENT_FINDER.newInstance(scheme);
DISCOVERY_AGENT_FACTORYS.put(scheme, daf);
} catch (Throwable e) {
throw IOExceptionSupport.create("DiscoveryAgent scheme NOT
recognized: [" + scheme + "]", e);
diff --git
a/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
b/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
index 596d15ea00..594af7e257 100644
--- a/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
+++ b/activemq-client/src/main/java/org/apache/activemq/util/FactoryFinder.java
@@ -20,14 +20,22 @@ import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.InvocationTargetException;
+import java.net.UnknownServiceException;
+import java.nio.file.Path;
+import java.security.Security;
+import java.util.Arrays;
+import java.util.Objects;
+import java.util.Optional;
import java.util.Properties;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
+import java.util.stream.Collectors;
/**
*
*/
-public class FactoryFinder {
+public class FactoryFinder<T> {
/**
* The strategy that the FactoryFinder uses to find load and instantiate
Objects
@@ -44,51 +52,66 @@ public class FactoryFinder {
* @param path the full service path
* @return
*/
- public Object create(String path) throws IllegalAccessException,
InstantiationException, IOException, ClassNotFoundException;
+ Object create(String path) throws IllegalAccessException,
InstantiationException, IOException, ClassNotFoundException;
+ /**
+ * This method loads objects by searching for classes in the given
path.
+ * A requiredType and Set of allowed implementations are provided for
+ * implementations to use for validation. Note is up to the actual
implementations
+ * that implement {@link ObjectFactory} to decide how to use both
parameters.
+ * By default, the method just delegates to {@link #create(String)}
+ *
+ * @param path the full service path
+ * @param requiredType the requiredType any objects must implement
+ * @param allowedImpls The set of allowed impls
+ * @return
+ */
+ @SuppressWarnings("unchecked")
+ default <T> T create(String path, Class<T> requiredType, Set<Class<?
extends T>> allowedImpls)
+ throws IllegalAccessException, InstantiationException,
IOException, ClassNotFoundException {
+ return (T) create(path);
+ }
}
/**
* The default implementation of Object factory which works well in
standalone applications.
*/
protected static class StandaloneObjectFactory implements ObjectFactory {
- final ConcurrentMap<String, Class> classMap = new
ConcurrentHashMap<String, Class>();
+ final ConcurrentMap<String, Class<?>> classMap = new
ConcurrentHashMap<>();
+
+ @Override
+ public Object create(final String path)
+ throws IllegalAccessException, InstantiationException,
IOException, ClassNotFoundException {
+ throw new UnsupportedOperationException("Create is not supported
without requiredType and allowed impls");
+ }
+ @SuppressWarnings("unchecked")
@Override
- public Object create(final String path) throws InstantiationException,
IllegalAccessException, ClassNotFoundException, IOException {
- Class clazz = classMap.get(path);
+ public <T> T create(String path, Class<T> requiredType, Set<Class<?
extends T>> allowedImpls) throws InstantiationException,
IllegalAccessException, ClassNotFoundException, IOException {
+ Class<?> clazz = classMap.get(path);
if (clazz == null) {
clazz = loadClass(loadProperties(path));
+ // no reason to cache if invalid so validate before caching
+ validateClass(clazz, requiredType, allowedImpls);
classMap.put(path, clazz);
+ } else {
+ // Validate again (even for previously cached classes) in case
+ // a path is re-used with a different requiredType.
+ // This object factory is static and shared by all factory
finder instances by default,
+ // so it would be possible (although probably a mistake) to
use the same
+ // path again with a different requiredType in a different
FactoryFinder
+ validateClass(clazz, requiredType, allowedImpls);
}
-
+
try {
- return clazz.getConstructor().newInstance();
+ return (T) clazz.getConstructor().newInstance();
} catch (NoSuchMethodException | InvocationTargetException e) {
throw new InstantiationException(e.getMessage());
}
}
- static public Class loadClass(Properties properties) throws
ClassNotFoundException, IOException {
-
- String className = properties.getProperty("class");
- if (className == null) {
- throw new IOException("Expected property is missing: class");
- }
- Class clazz = null;
- ClassLoader loader =
Thread.currentThread().getContextClassLoader();
- if (loader != null) {
- try {
- clazz = loader.loadClass(className);
- } catch (ClassNotFoundException e) {
- // ignore
- }
- }
- if (clazz == null) {
- clazz =
FactoryFinder.class.getClassLoader().loadClass(className);
- }
-
- return clazz;
+ static Class<?> loadClass(Properties properties) throws
ClassNotFoundException, IOException {
+ return FactoryFinder.loadClass(properties.getProperty("class"));
}
static public Properties loadProperties(String uri) throws IOException
{
@@ -138,11 +161,45 @@ public class FactoryFinder {
// Instance methods and properties
// ================================================================
private final String path;
+ private final Class<T> requiredType;
+ private final Set<Class<? extends T>> allowedImpls;
- public FactoryFinder(String path) {
- this.path = path;
+ /**
+ *
+ * @param path The path to search for impls
+ * @param requiredType Required interface type that any impl must implement
+ * @param allowedImpls The list of allowed implementations. If null or
asterisk
+ * then all impls of the requiredType are allowed.
+ */
+ public FactoryFinder(String path, Class<T> requiredType, String
allowedImpls) {
+ this.path = Objects.requireNonNull(path);
+ this.requiredType = Objects.requireNonNull(requiredType);
+ this.allowedImpls = loadAllowedImpls(requiredType, allowedImpls);
}
+ @SuppressWarnings("unchecked")
+ private static <T> Set<Class<? extends T>> loadAllowedImpls(Class<T>
requiredType, String allowedImpls) {
+ // If allowedImpls is either null or an asterisk (allow all wild card)
then set to null so we don't filter
+ // If allowedImpls is only an empty string we return an empty set
meaning allow none
+ // Otherwise split/trim all values
+ return allowedImpls != null && !allowedImpls.equals("*") ?
+ Arrays.stream(allowedImpls.split("\\s*,\\s*"))
+ .filter(s -> !s.isEmpty())
+ .map(s -> {
+ try {
+ final Class<?> clazz = FactoryFinder.loadClass(s);
+ if (!requiredType.isAssignableFrom(clazz)) {
+ throw new IllegalArgumentException(
+ "Class " + clazz + " is not assignable to
" + requiredType);
+ }
+ return (Class<? extends T>)clazz;
+ } catch (ClassNotFoundException | IOException e) {
+ throw new IllegalArgumentException(e);
+ }
+ }).collect(Collectors.toUnmodifiableSet()) : null;
+ }
+
+
/**
* Creates a new instance of the given key
*
@@ -150,9 +207,71 @@ public class FactoryFinder {
* the factory name
* @return a newly created instance
*/
- public Object newInstance(String key) throws IllegalAccessException,
InstantiationException, IOException, ClassNotFoundException {
- return objectFactory.create(path+key);
+ public T newInstance(String key) throws IllegalAccessException,
InstantiationException, IOException, ClassNotFoundException {
+ return objectFactory.create(resolvePath(key), requiredType,
allowedImpls);
}
+ Set<Class<? extends T>> getAllowedImpls() {
+ return allowedImpls;
+ }
+
+ Class<T> getRequiredType() {
+ return requiredType;
+ }
+
+ private String resolvePath(final String key) throws InstantiationException
{
+ // Normalize the base path with the given key. This
+ // will resolve/remove any relative ".." sections of the path.
+ // Example: "/dir1/dir2/dir3/../file" becomes "/dir1/dir2/file"
+ final Path resolvedPath = Path.of(path).resolve(key).normalize();
+
+ // Validate the resolved path is still within the original defined
+ // root path and throw an error of it is not.
+ if (!resolvedPath.startsWith(path)) {
+ throw new InstantiationException("Provided key escapes the
FactoryFinder configured directory");
+ }
+
+ return resolvedPath.toString();
+ }
+
+ public static String buildAllowedImpls(Class<?>...classes) {
+ return Arrays.stream(Objects.requireNonNull(classes, "List of allowed
classes may not be null"))
+ .map(Class::getName).collect(Collectors.joining(","));
+ }
+
+ public static <T> void validateClass(Class<?> clazz, Class<T> requiredType,
+ Set<Class<? extends T>> allowedImpls) throws
InstantiationException {
+ // Validate the loaded class is an allowed impl
+ if (allowedImpls != null && !allowedImpls.contains(clazz)) {
+ throw new InstantiationException("Class " + clazz + " is not an
allowed implementation "
+ + "of type " + requiredType);
+ }
+ // Validate the loaded class is a subclass of the right type
+ // The allowedImpls may not be used so also check requiredType. Even
if set
+ // generics can be erased and this is an extra safety check
+ if (!requiredType.isAssignableFrom(clazz)) {
+ throw new InstantiationException("Class " + clazz + " is not
assignable to " + requiredType);
+ }
+ }
+
+ static Class<?> loadClass(String className) throws ClassNotFoundException,
IOException {
+ if (className == null) {
+ throw new IOException("Expected property is missing: class");
+ }
+ Class<?> clazz = null;
+ ClassLoader loader = Thread.currentThread().getContextClassLoader();
+ if (loader != null) {
+ try {
+ clazz = loader.loadClass(className);
+ } catch (ClassNotFoundException e) {
+ // ignore
+ }
+ }
+ if (clazz == null) {
+ clazz = FactoryFinder.class.getClassLoader().loadClass(className);
+ }
+
+ return clazz;
+ }
}
diff --git
a/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
b/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
new file mode 100644
index 0000000000..3e775b2161
--- /dev/null
+++
b/activemq-client/src/test/java/org/apache/activemq/util/FactoryFinderTest.java
@@ -0,0 +1,202 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.util;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.apache.activemq.transport.TransportFactory;
+import org.apache.activemq.transport.nio.NIOTransportFactory;
+import org.apache.activemq.transport.tcp.SslTransport;
+import org.apache.activemq.transport.tcp.SslTransportFactory;
+import org.apache.activemq.transport.tcp.TcpTransportFactory;
+import org.apache.activemq.util.FactoryFinder.ObjectFactory;
+import org.apache.activemq.wireformat.WireFormatFactory;
+import org.junit.Test;
+
+public class FactoryFinderTest {
+
+ private static final String TRANSPORT_FACTORY_PATH =
"META-INF/services/org/apache/activemq/transport/";
+ private static final String WIREFORMAT_FACTORY_PATH =
"META-INF/services/org/apache/activemq/wireformat/";
+
+ // Test path traversal attempts will throw an error
+ @Test
+ public void testPathTraversal() throws Exception {
+ FactoryFinder<TransportFactory> finder
+ = new FactoryFinder<>(TRANSPORT_FACTORY_PATH,
TransportFactory.class, null);
+ assertNull(finder.getAllowedImpls());
+
+ try {
+ finder.newInstance("../../tcp");
+ fail("should have failed instantiation");
+ } catch (InstantiationException e) {
+ assertEquals("Provided key escapes the FactoryFinder configured
directory",
+ e.getMessage());
+ }
+ }
+
+ // WireFormatFactory is not assignable to TransportFactory
+ // So the constructor should throw an IllegalArgumentException
+ @Test(expected = IllegalArgumentException.class)
+ public void testAllowedImplsMatchInterface() {
+ new FactoryFinder<>(TRANSPORT_FACTORY_PATH, TransportFactory.class,
+ FactoryFinder.buildAllowedImpls(WireFormatFactory.class));
+ }
+
+ @Test(expected = IllegalArgumentException.class)
+ public void testAllowedImplsNotFound() {
+ new FactoryFinder<>(TRANSPORT_FACTORY_PATH, TransportFactory.class,
"some.invalid.ClassName");
+ }
+
+ @Test(expected = IOException.class)
+ public void testLoadClassNull() throws IOException, ClassNotFoundException
{
+ FactoryFinder.loadClass(null);
+ }
+
+ @Test(expected = ClassNotFoundException.class)
+ public void testLoadClassNotFound() throws IOException,
ClassNotFoundException {
+ FactoryFinder.loadClass("some.invalid.ClassName");
+ }
+
+ // The default factory should throw UnsupportedOperationException
+ // if using legacy create() method
+ @Test(expected = UnsupportedOperationException.class)
+ public void testDefaultObjectFinder() throws Exception {
+ ObjectFactory factory = FactoryFinder.getObjectFactory();
+ factory.create("path");
+ }
+
+ @Test
+ public void testObjectFinderDefaultMethod() throws Exception {
+ AtomicBoolean called = new AtomicBoolean();
+ ObjectFactory factory = path -> {
+ called.set(true);
+ return null;
+ };
+ // test that new method defaults to legacy method if not implemented
+ factory.create(null, null, null);
+ assertTrue(called.get());
+ }
+
+ // Verify interface check works if not using an allowed list to at least
verify that
+ // any classes loaded will implement/extend the required type
+ @Test
+ public void testInterfaceMismatch() throws Exception {
+ // use wrong interface type, WIREFORMAT_FACTORY_PATH should be of type
WireFormatFactory
+ // and not TransportFactory
+ FactoryFinder<TransportFactory> factory = new
FactoryFinder<>(WIREFORMAT_FACTORY_PATH, TransportFactory.class,
+ null);
+ // This is a valid impl in the wireformat directory, but it isn't a
TransportFactory type
+ try {
+ factory.newInstance("default");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains("OpenWireFormatFactory is not
assignable to class " +
+ TransportFactory.class.getName()));
+ }
+ }
+
+ // Test filtering by allowed implementations of the given interface
+ @Test
+ public void testAllowedImplsFilter() throws Exception {
+ Set<Class<?>> allowedImpls = Set.of(TcpTransportFactory.class,
NIOTransportFactory.class);
+ FactoryFinder<TransportFactory> finder = new
FactoryFinder<>(TRANSPORT_FACTORY_PATH, TransportFactory.class,
+ FactoryFinder.buildAllowedImpls(allowedImpls.toArray(new
Class<?>[0])));
+ assertEquals(TransportFactory.class, finder.getRequiredType());
+ assertEquals(allowedImpls, finder.getAllowedImpls());
+ // tcp and nio are the only class in the allow list
+ assertNotNull(finder.newInstance("tcp"));
+
+ try {
+ // ssl transport factory was not in the allow list
+ finder.newInstance("ssl");
+ fail("should have failed instantiation");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains("is not an allowed
implementation of type"));
+ }
+ }
+
+ // empty array is used so we should deny everything
+ @Test
+ public void testAllowedImplsFilterDenyAll() throws Exception {
+ FactoryFinder<TransportFactory> finder = new
FactoryFinder<>(TRANSPORT_FACTORY_PATH,
+ TransportFactory.class, "");
+ assertEquals(TransportFactory.class, finder.getRequiredType());
+ assertEquals(Set.of(), finder.getAllowedImpls());
+
+ try {
+ // nothing allowed, tcp exists but should be blocked
+ finder.newInstance("tcp");
+ fail("should have failed instantiation");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains("is not an allowed
implementation of type"));
+ }
+ }
+
+ // Test that impls are not filtered if the list is null
+ @Test
+ public void testAllowedImplsFilterAllowAll() throws Exception {
+ // check with constructor that should default to null
+ FactoryFinder<TransportFactory> finder = new
FactoryFinder<>(TRANSPORT_FACTORY_PATH,
+ TransportFactory.class, null);
+ assertNull(finder.getAllowedImpls());
+ assertNotNull(finder.newInstance("tcp"));
+ assertNotNull(finder.newInstance("ssl"));
+
+ // check again with explicit null
+ finder = new FactoryFinder<>(TRANSPORT_FACTORY_PATH,
+ TransportFactory.class, null);
+ assertNull(finder.getAllowedImpls());
+ assertNotNull(finder.newInstance("tcp"));
+ assertNotNull(finder.newInstance("ssl"));
+
+ try {
+ // abc is allowed because we are not filtering by allowed impls but
+ // we should not be able to find it
+ finder.newInstance("abc");
+ fail("should have failed instantiation");
+ } catch (IOException e) {
+ assertTrue(e.getMessage().contains("Could not find factory class
for resource"));
+ }
+ }
+
+ @Test
+ public void testAllowedImplsTrimWhiteSpace() throws Exception {
+ // Build CSV with white space between all classes, including two white
space
+ // only elements
+ String implsWithWhiteSpace = String.join(", ",
+ TcpTransportFactory.class.getName(),
SslTransportFactory.class.getName(),
+ " ", NIOTransportFactory.class.getName(), "");
+
+ FactoryFinder<TransportFactory> finder = new
FactoryFinder<>(TRANSPORT_FACTORY_PATH,
+ TransportFactory.class, implsWithWhiteSpace);
+
+ // all white space should have been trimmed and all empty values
skipped
+ // leading to the 3 valid classes loaded
+ assertEquals(Set.of(TcpTransportFactory.class,
SslTransportFactory.class,
+ NIOTransportFactory.class), finder.getAllowedImpls());
+ assertNotNull(finder.newInstance("tcp"));
+ assertNotNull(finder.newInstance("ssl"));
+ assertNotNull(finder.newInstance("nio"));
+ }
+}
diff --git
a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCPersistenceAdapter.java
b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCPersistenceAdapter.java
index eaddfb8340..23f3bc48d4 100644
---
a/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCPersistenceAdapter.java
+++
b/activemq-jdbc-store/src/main/java/org/apache/activemq/store/jdbc/JDBCPersistenceAdapter.java
@@ -75,10 +75,10 @@ import org.slf4j.LoggerFactory;
public class JDBCPersistenceAdapter extends DataSourceServiceSupport
implements PersistenceAdapter {
private static final Logger LOG =
LoggerFactory.getLogger(JDBCPersistenceAdapter.class);
- private static FactoryFinder adapterFactoryFinder = new FactoryFinder(
- "META-INF/services/org/apache/activemq/store/jdbc/");
- private static FactoryFinder lockFactoryFinder = new FactoryFinder(
- "META-INF/services/org/apache/activemq/store/jdbc/lock/");
+ private static final FactoryFinder<JDBCAdapter> adapterFactoryFinder = new
FactoryFinder<>(
+ "META-INF/services/org/apache/activemq/store/jdbc/",
JDBCAdapter.class, null);
+ private static final FactoryFinder<Locker> lockFactoryFinder = new
FactoryFinder<>(
+ "META-INF/services/org/apache/activemq/store/jdbc/lock/",
Locker.class, null);
public static final long DEFAULT_LOCK_KEEP_ALIVE_PERIOD = 30 * 1000;
@@ -456,7 +456,7 @@ public class JDBCPersistenceAdapter extends
DataSourceServiceSupport implements
return adapter;
}
- private Object loadAdapter(FactoryFinder finder, String kind) throws
IOException {
+ private Object loadAdapter(FactoryFinder<?> finder, String kind) throws
IOException {
Object adapter = null;
TransactionContext c = getTransactionContext();
try {
diff --git
a/activemq-mqtt/src/main/java/org/apache/activemq/transport/mqtt/MQTTProtocolConverter.java
b/activemq-mqtt/src/main/java/org/apache/activemq/transport/mqtt/MQTTProtocolConverter.java
index 5e9b6234db..badd991217 100644
---
a/activemq-mqtt/src/main/java/org/apache/activemq/transport/mqtt/MQTTProtocolConverter.java
+++
b/activemq-mqtt/src/main/java/org/apache/activemq/transport/mqtt/MQTTProtocolConverter.java
@@ -126,7 +126,9 @@ public class MQTTProtocolConverter {
public int version;
- private final FactoryFinder STRATAGY_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/transport/strategies/");
+ private final FactoryFinder<MQTTSubscriptionStrategy> STRATEGY_FINDER =
+ new
FactoryFinder<>("META-INF/services/org/apache/activemq/transport/strategies/",
+ MQTTSubscriptionStrategy.class, null);
/*
* Subscription strategy configuration element.
@@ -861,7 +863,7 @@ public class MQTTProtocolConverter {
protected MQTTSubscriptionStrategy findSubscriptionStrategy() throws
IOException {
if (subsciptionStrategy == null) {
- synchronized (STRATAGY_FINDER) {
+ synchronized (STRATEGY_FINDER) {
if (subsciptionStrategy != null) {
return subsciptionStrategy;
}
@@ -869,7 +871,7 @@ public class MQTTProtocolConverter {
MQTTSubscriptionStrategy strategy = null;
if (subscriptionStrategyName != null &&
!subscriptionStrategyName.isEmpty()) {
try {
- strategy = (MQTTSubscriptionStrategy)
STRATAGY_FINDER.newInstance(subscriptionStrategyName);
+ strategy =
STRATEGY_FINDER.newInstance(subscriptionStrategyName);
LOG.debug("MQTT Using subscription strategy: {}",
subscriptionStrategyName);
if (strategy instanceof BrokerServiceAware) {
((BrokerServiceAware)strategy).setBrokerService(brokerService);
diff --git a/activemq-spring/pom.xml b/activemq-spring/pom.xml
index ed1a740610..f4b4b059d5 100644
--- a/activemq-spring/pom.xml
+++ b/activemq-spring/pom.xml
@@ -240,7 +240,7 @@
<schema>${basedir}/target/classes/activemq.xsd</schema>
<outputDir>${basedir}/target/classes</outputDir>
<generateSpringSchemasFile>false</generateSpringSchemasFile>
-
<excludedClasses>org.apache.activemq.broker.jmx.AnnotatedMBean,org.apache.activemq.broker.jmx.DestinationViewMBean</excludedClasses>
+
<excludedClasses>org.apache.activemq.broker.jmx.AnnotatedMBean,org.apache.activemq.broker.jmx.DestinationViewMBean,org.apache.activemq.util.FactoryFinder</excludedClasses>
</configuration>
<goals>
<goal>mapping</goal>
diff --git
a/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java
b/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java
index 70456a7cd6..4d96cf9eff 100644
---
a/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java
+++
b/activemq-stomp/src/main/java/org/apache/activemq/transport/stomp/ProtocolConverter.java
@@ -84,6 +84,10 @@ public class ProtocolConverter {
private static final String BROKER_VERSION;
private static final StompFrame ping = new
StompFrame(Stomp.Commands.KEEPALIVE);
+ public static final String DEFAULT_ALLOWED_TRANSLATORS =
FactoryFinder.buildAllowedImpls(
+ JmsFrameTranslator.class, LegacyFrameTranslator.class);
+ public static final String FRAME_TRANSLATOR_CLASSES_PROP =
"org.apache.activemq.stomp.FRAME_TRANSLATOR_CLASSES";
+
static {
String version = "5.6.0";
try(InputStream in =
ProtocolConverter.class.getResourceAsStream("/org/apache/activemq/version.txt"))
{
@@ -108,7 +112,7 @@ public class ProtocolConverter {
private final LongSequenceGenerator transactionIdGenerator = new
LongSequenceGenerator();
private final LongSequenceGenerator tempDestinationGenerator = new
LongSequenceGenerator();
- private final ConcurrentMap<Integer, ResponseHandler> resposeHandlers =
new ConcurrentHashMap<>();
+ private final ConcurrentMap<Integer, ResponseHandler> responseHandlers =
new ConcurrentHashMap<>();
private final ConcurrentMap<ConsumerId, StompSubscription>
subscriptionsByConsumerId = new ConcurrentHashMap<>();
private final ConcurrentMap<String, StompSubscription> subscriptions = new
ConcurrentHashMap<>();
private final ConcurrentMap<String, ActiveMQDestination> tempDestinations
= new ConcurrentHashMap<>();
@@ -121,13 +125,13 @@ public class ProtocolConverter {
// Read-Only view used in this class to enforce the separation of read vs
update of the global index.
private final Map<String, StompAckEntry> pendingAcks =
Collections.unmodifiableMap(pendingAcksTracker);
- private final Object commnadIdMutex = new Object();
+ private final Object commandIdMutex = new Object();
private int lastCommandId;
private final AtomicBoolean connected = new AtomicBoolean(false);
- private final FrameTranslator frameTranslator = new
LegacyFrameTranslator();
- private ConcurrentMap<String, FrameTranslator> jmsFrameTranslators=new
ConcurrentHashMap<String,FrameTranslator>();
-
- private final FactoryFinder FRAME_TRANSLATOR_FINDER = new
FactoryFinder("META-INF/services/org/apache/activemq/transport/frametranslator/");
+ private final FrameTranslator defaultFrameTranslator = new
LegacyFrameTranslator();
+ private final Map<String, FrameTranslator> jmsFrameTranslators = new
ConcurrentHashMap<>();
+
+ private final FactoryFinder<FrameTranslator> frameTranslatorFinder;
private final BrokerContext brokerContext;
private String version = "1.0";
private long hbReadInterval;
@@ -138,10 +142,12 @@ public class ProtocolConverter {
public ProtocolConverter(StompTransport stompTransport, BrokerContext
brokerContext) {
this.stompTransport = stompTransport;
this.brokerContext = brokerContext;
+ this.frameTranslatorFinder = new
FactoryFinder<>("META-INF/services/org/apache/activemq/transport/frametranslator/",
+ FrameTranslator.class,
System.getProperty(FRAME_TRANSLATOR_CLASSES_PROP, DEFAULT_ALLOWED_TRANSLATORS));
}
protected int generateCommandId() {
- synchronized (commnadIdMutex) {
+ synchronized (commandIdMutex) {
return lastCommandId++;
}
}
@@ -174,7 +180,7 @@ public class ProtocolConverter {
command.setCommandId(generateCommandId());
if (handler != null) {
command.setResponseRequired(true);
- resposeHandlers.put(command.getCommandId(), handler);
+ responseHandlers.put(command.getCommandId(), handler);
}
stompTransport.sendToActiveMQ(command);
}
@@ -188,29 +194,21 @@ public class ProtocolConverter {
}
protected FrameTranslator findTranslator(String header,
ActiveMQDestination destination, boolean advisory) {
- FrameTranslator translator = frameTranslator;
+ FrameTranslator translator = null;
try {
if (header != null) {
- translator=jmsFrameTranslators.get(header);
- if(translator==null) {
- LOG.info("Creating a new FrameTranslator to convert
"+header);
- translator = (FrameTranslator)
FRAME_TRANSLATOR_FINDER.newInstance(header);
- if(translator!=null) {
- LOG.info("Created a new FrameTranslator to
convert "+header);
- jmsFrameTranslators.put(header,translator);
- }else {
- LOG.error("Failed in creating FrameTranslator
to convert "+header);
- }
- }
- } else {
- if (destination != null && (advisory ||
AdvisorySupport.isAdvisoryTopic(destination))) {
- translator = new JmsFrameTranslator();
- }
+ translator = loadTranslator(header);
+ } else if (destination != null && (advisory ||
AdvisorySupport.isAdvisoryTopic(destination))) {
+ translator = new JmsFrameTranslator();
}
} catch (Exception ignore) {
// if anything goes wrong use the default translator
LOG.debug("Failed in getting a FrameTranslator to
convert ", ignore);
- translator = frameTranslator;
+ }
+
+ // fallback to default if we can't find/load one
+ if (translator == null) {
+ translator = defaultFrameTranslator;
}
if (translator instanceof BrokerContextAware) {
@@ -219,6 +217,23 @@ public class ProtocolConverter {
return translator;
}
+
+ protected FrameTranslator loadTranslator(String header) throws Exception {
+ FrameTranslator translator = jmsFrameTranslators.get(header);
+
+ if (translator == null) {
+ LOG.info("Creating a new FrameTranslator to convert {}", header);
+ translator = frameTranslatorFinder.newInstance(header);
+ if (translator != null) {
+ LOG.info("Created a new FrameTranslator to convert {}",
header);
+ jmsFrameTranslators.put(header, translator);
+ } else {
+ LOG.error("Failed in creating FrameTranslator to convert {}",
header);
+ }
+ }
+
+ return translator;
+ }
/**
* Convert a STOMP command
@@ -617,9 +632,11 @@ public class ProtocolConverter {
StompSubscription stompSubscription;
if (!consumerInfo.isBrowser()) {
- stompSubscription = new StompSubscription(this, subscriptionId,
consumerInfo, headers.get(Stomp.Headers.TRANSFORMATION), pendingAcksTracker);
+ stompSubscription = new StompSubscription(this, subscriptionId,
consumerInfo, headers.get(
+ Stomp.Headers.TRANSFORMATION), pendingAcksTracker);
} else {
- stompSubscription = new StompQueueBrowserSubscription(this,
subscriptionId, consumerInfo, headers.get(Stomp.Headers.TRANSFORMATION),
pendingAcksTracker);
+ stompSubscription = new StompQueueBrowserSubscription(this,
subscriptionId, consumerInfo, headers.get(
+ Stomp.Headers.TRANSFORMATION), pendingAcksTracker);
}
stompSubscription.setDestination(actualDest);
@@ -863,7 +880,7 @@ public class ProtocolConverter {
public void onActiveMQCommand(Command command) throws IOException,
JMSException {
if (command.isResponse()) {
Response response = (Response)command;
- ResponseHandler rh =
resposeHandlers.remove(response.getCorrelationId());
+ ResponseHandler rh =
responseHandlers.remove(response.getCorrelationId());
if (rh != null) {
rh.onResponse(this, response);
} else {
@@ -895,7 +912,7 @@ public class ProtocolConverter {
public StompFrame convertMessage(ActiveMQMessage message, boolean
ignoreTransformation) throws IOException, JMSException {
if (ignoreTransformation == true) {
- return frameTranslator.convertMessage(this, message);
+ return defaultFrameTranslator.convertMessage(this, message);
} else {
FrameTranslator translator = findTranslator(
message.getStringProperty(Stomp.Headers.TRANSFORMATION),
message.getDestination(), message.isAdvisory());
diff --git
a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
new file mode 100644
index 0000000000..17d293e3d8
--- /dev/null
+++
b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/ProtocolConverterTest.java
@@ -0,0 +1,192 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.transport.stomp;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+
+import org.apache.activemq.transport.stomp.StompTest.TestTranslator;
+import org.apache.activemq.util.FactoryFinder;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+@Category(ParallelTest.class)
+public class ProtocolConverterTest {
+
+ private final StompTransport transport = mock(StompTransport.class);
+ // Before each test reset to defaults
+ @Before
+ public void setUp() {
+ resetDefaultAllowedTranslators();
+ }
+
+ // clean up after all tests are done finished by resetting the property
back before exiting
+ @AfterClass
+ public static void tearDown() {
+ resetDefaultAllowedTranslators();
+ }
+
+ @Test
+ public void testLoadMissingTranslator() throws Exception {
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+
+ // Should fallback to default
+ assertEquals(LegacyFrameTranslator.class,
converter.findTranslator("abc").getClass());
+
+ try {
+ converter.loadTranslator("abc");
+ fail("should have failed");
+ } catch (Exception e) {
+ assertTrue(e.getMessage().contains("Could not find factory class
for resource"));
+ }
+ }
+
+ @Test
+ public void testAllowedTranslators() throws Exception {
+ // first test defaults
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+ assertNotNull(converter.loadTranslator("jms-json"));
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-json").getClass());
+ assertNotNull(converter.loadTranslator("jms-xml"));
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-xml").getClass());
+
+ // test specific allowed
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
+ JmsFrameTranslator.class.getName());
+ converter = new ProtocolConverter(transport, null);
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-json").getClass());
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-byte").getClass());
+
+ // test all allowed
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
"*");
+ converter = new ProtocolConverter(transport, null);
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-json").getClass());
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-byte").getClass());
+ // our custom is also allowed
+ assertNotNull(converter.loadTranslator("stomp-test"));
+ assertEquals(TestTranslator.class,
converter.findTranslator("stomp-test").getClass());
+ }
+
+ @Test
+ public void testAllowedTranslatorsErrors() throws Exception {
+ // Disable all and allow none
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
"");
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+
+ assertTranslatorNotAllowed(converter, "jms-json");
+
+ // should fall back to default legacy on error
+ assertEquals(LegacyFrameTranslator.class,
converter.findTranslator("jms-json").getClass());
+
+ // test custom translator not in allowed list
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
JmsFrameTranslator.class.getName());
+ converter = new ProtocolConverter(transport, null);
+ assertTranslatorNotAllowed(converter, "stomp-test");
+
+ }
+
+ @Test
+ public void testCustomAllowedTranslator() throws Exception {
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
+ FactoryFinder.buildAllowedImpls(JmsFrameTranslator.class,
TestTranslator.class));
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+
+ // JmsFrameTranslator is allowed
+ assertNotNull(converter.loadTranslator("jms-json"));
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-json").getClass());
+
+ // our custom is also allowed
+ assertNotNull(converter.loadTranslator("stomp-test"));
+ assertEquals(TestTranslator.class,
converter.findTranslator("stomp-test").getClass());
+ }
+
+ @Test
+ public void testWhiteSpaceAllowedTranslator() throws Exception {
+ // white space should be trimmed
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
+ String.join(", ", JmsFrameTranslator.class.getName(),
+ TestTranslator.class.getName()));
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+
+ // JmsFrameTranslator is allowed
+ assertNotNull(converter.loadTranslator("jms-json"));
+ assertEquals(JmsFrameTranslator.class,
converter.findTranslator("jms-json").getClass());
+
+ // our custom is also allowed
+ assertNotNull(converter.loadTranslator("stomp-test"));
+ assertEquals(TestTranslator.class,
converter.findTranslator("stomp-test").getClass());
+ }
+
+ @Test
+ public void testMissingInterface() throws Exception {
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
"*");
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+
+ try {
+ converter.loadTranslator("stomp-test-invalid");
+ fail("should have failed");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains(BadTranslator.class.getName() +
" is not assignable to interface "
+ + FrameTranslator.class.getName()));
+ }
+
+ // fallback
+ assertEquals(LegacyFrameTranslator.class,
converter.findTranslator("stomp-test-invalid").getClass());
+ }
+
+ @Test
+ public void testPathTraversal() throws Exception {
+ ProtocolConverter converter = new ProtocolConverter(transport, null);
+
+ try {
+ converter.loadTranslator("../stomp-test");
+ fail("should have failed");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains("rovided key escapes the
FactoryFinder configured directory"));
+ }
+
+ // fallback
+ assertEquals(LegacyFrameTranslator.class,
converter.findTranslator("../stomp-test").getClass());
+ }
+
+ private void assertTranslatorNotAllowed(ProtocolConverter converter,
String key) throws Exception {
+ try {
+ converter.loadTranslator(key);
+ fail("Should have failed");
+ } catch (InstantiationException e) {
+ assertTrue(e.getMessage().contains("is not an allowed
implementation of type interface "
+ + FrameTranslator.class.getName()));
+ }
+ // should fall back to default legacy on error
+ assertEquals(LegacyFrameTranslator.class,
converter.findTranslator(key).getClass());
+ }
+
+ private static void resetDefaultAllowedTranslators() {
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
+ ProtocolConverter.DEFAULT_ALLOWED_TRANSLATORS);
+ }
+
+ // don't implement the correct interface
+ public static class BadTranslator {
+
+ }
+}
diff --git
a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompTest.java
b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompTest.java
index 218dda6524..70fa9ed4f8 100644
---
a/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompTest.java
+++
b/activemq-stomp/src/test/java/org/apache/activemq/transport/stomp/StompTest.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
+import jakarta.jms.Destination;
import java.io.IOException;
import java.io.StringReader;
import java.net.SocketTimeoutException;
@@ -32,6 +33,7 @@ import java.util.List;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -59,6 +61,7 @@ import org.apache.activemq.broker.region.Subscription;
import org.apache.activemq.broker.region.policy.PolicyEntry;
import org.apache.activemq.broker.region.policy.PolicyMap;
import org.apache.activemq.command.ActiveMQDestination;
+import org.apache.activemq.command.ActiveMQMessage;
import org.apache.activemq.command.ActiveMQQueue;
import org.apache.activemq.command.ActiveMQTextMessage;
import org.apache.activemq.util.Wait;
@@ -1241,6 +1244,89 @@ public class StompTest extends StompTestSupport {
compareFrameXML(frame, xmlObject);
}
+ @Test(timeout = 60000)
+ public void testTransformationSuccess() throws Exception {
+ // transformation is allowed, use default configured transformers
+ assertStompTransformation(null,
+ Stomp.Transformations.JMS_OBJECT_XML.toString(),
+ frame -> assertTrue(frame.contains("<pojo>")));
+
+ // transformation is allowed
+
assertStompTransformation(ProtocolConverter.DEFAULT_ALLOWED_TRANSLATORS,
+ Stomp.Transformations.JMS_OBJECT_XML.toString(),
+ frame -> assertTrue(frame.contains("<pojo>")));
+
+ // transformation is allowed with wildcard, xml tag should exist
+ assertStompTransformation("*",
+ Stomp.Transformations.JMS_OBJECT_XML.toString(),
+ frame -> assertTrue(frame.contains("<pojo>")));
+
+ // verify TestTranslator was used and appended header
+ assertStompTransformation(TestTranslator.class.getName(),
+ "stomp-test",
+ frame ->
assertTrue(frame.contains("stomp-test-translator-header")));
+ }
+
+ @Test(timeout = 60000)
+ public void testTransformationFailuresFallbackToDefault() throws Exception
{
+ // path traversal is not allowed so given translator should fail
+
assertStompTransformation(ProtocolConverter.DEFAULT_ALLOWED_TRANSLATORS,
+ "../" + Stomp.Transformations.JMS_OBJECT_XML,
+ frame -> assertEquals(-1, frame.indexOf("<pojo>")));
+
+ // xml translator maps to JmsFrameTranslator.class which is not allowed
+ assertStompTransformation(LegacyFrameTranslator.class.getName(),
+ Stomp.Transformations.JMS_OBJECT_XML.toString(),
+ frame -> assertEquals(-1, frame.indexOf("<pojo>")));
+
+ // Allowed translators are set to none so translator should fail
+ assertStompTransformation("",
Stomp.Transformations.JMS_OBJECT_XML.toString(),
+ frame -> assertEquals(-1, frame.indexOf("<pojo>")));
+
+ // TestTranslator is not allowed so won't have our header is it will
fall back to
+ // the legacy default translator
+
assertStompTransformation(ProtocolConverter.DEFAULT_ALLOWED_TRANSLATORS,
+ "stomp-test",
+ frame ->
assertFalse(frame.contains("stomp-test-translator-header")));
+ }
+
+ private void assertStompTransformation(String allowedTranslators,
+ String transformation, Consumer<String> verifier) throws Exception
{
+ try {
+ if (allowedTranslators != null) {
+ tearDown();
+
System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
+ allowedTranslators);
+ setUp();
+ }
+
+ MessageProducer producer = session.createProducer(
+ new ActiveMQQueue("USERS." + getQueueName()));
+ ObjectMessage message = session.createObjectMessage(
+ new SamplePojo("Dejan", "Belgrade"));
+ producer.send(message);
+
+ String frame = "CONNECT\n" + "login:system\n" +
"passcode:manager\n\n" + Stomp.NULL;
+ stompConnection.sendFrame(frame);
+
+ frame = stompConnection.receiveFrame();
+ assertTrue(frame.startsWith("CONNECTED"));
+
+ frame = "SUBSCRIBE\n" + "destination:/queue/USERS." +
getQueueName() + "\n" + "ack:auto"
+ + "\n" + "transformation:" + transformation + "\n\n"
+ + Stomp.NULL;
+ stompConnection.sendFrame(frame);
+ frame = stompConnection.receiveFrame();
+
+ verifier.accept(frame);
+ } finally {
+ tearDown();
+ System.setProperty(ProtocolConverter.FRAME_TRANSLATOR_CLASSES_PROP,
+ ProtocolConverter.DEFAULT_ALLOWED_TRANSLATORS);
+ setUp();
+ }
+ }
+
private void compareFrameXML(String frame, String xmlObject) {
String xmlReceived = frame.trim().substring(frame.indexOf("<pojo>"));
@@ -2554,4 +2640,35 @@ public class StompTest extends StompTestSupport {
Map<String, String> map = (Map<String, String>)xstream.unmarshal(in);
return map;
}
+
+ public static class TestTranslator implements FrameTranslator {
+ private final JmsFrameTranslator translator = new JmsFrameTranslator();
+
+ @Override
+ public ActiveMQMessage convertFrame(ProtocolConverter converter,
StompFrame command)
+ throws JMSException, ProtocolException {
+ return translator.convertFrame(converter, command);
+ }
+
+ @Override
+ public StompFrame convertMessage(ProtocolConverter converter,
ActiveMQMessage message)
+ throws IOException, JMSException {
+ StompFrame frame = translator.convertMessage(converter, message);
+ // add a header to make it easy to detect if translator was used
+ frame.getHeaders().put("stomp-test-translator-header",
"header-value");
+ return frame;
+ }
+
+ @Override
+ public String convertDestination(ProtocolConverter converter,
Destination d) {
+ return translator.convertDestination(converter, d);
+ }
+
+ @Override
+ public ActiveMQDestination convertDestination(ProtocolConverter
converter, String name,
+ boolean forceFallback) throws ProtocolException {
+ return translator.convertDestination(converter, name,
forceFallback);
+ }
+ }
+
}
diff --git
a/activemq-stomp/src/test/resources/META-INF/services/org/apache/activemq/transport/frametranslator/stomp-test
b/activemq-stomp/src/test/resources/META-INF/services/org/apache/activemq/transport/frametranslator/stomp-test
new file mode 100644
index 0000000000..377559fb27
--- /dev/null
+++
b/activemq-stomp/src/test/resources/META-INF/services/org/apache/activemq/transport/frametranslator/stomp-test
@@ -0,0 +1,17 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## -----------------------------
+class=org.apache.activemq.transport.stomp.StompTest$TestTranslator
diff --git
a/activemq-stomp/src/test/resources/META-INF/services/org/apache/activemq/transport/frametranslator/stomp-test-invalid
b/activemq-stomp/src/test/resources/META-INF/services/org/apache/activemq/transport/frametranslator/stomp-test-invalid
new file mode 100644
index 0000000000..6a94af9406
--- /dev/null
+++
b/activemq-stomp/src/test/resources/META-INF/services/org/apache/activemq/transport/frametranslator/stomp-test-invalid
@@ -0,0 +1,17 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## -----------------------------
+class=org.apache.activemq.transport.stomp.ProtocolConverterTest$BadTranslator
diff --git
a/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java
b/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java
index 7215ddd1b5..435ffdc6c8 100644
--- a/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java
+++ b/activemq-web/src/main/java/org/apache/activemq/web/QueueBrowseServlet.java
@@ -35,6 +35,8 @@ import jakarta.servlet.http.HttpServletResponse;
import org.apache.activemq.util.FactoryFinder;
import org.apache.activemq.util.IntrospectionSupport;
import org.apache.activemq.web.view.MessageRenderer;
+import org.apache.activemq.web.view.RssMessageRenderer;
+import org.apache.activemq.web.view.SimpleMessageRenderer;
import org.apache.activemq.web.view.XmlMessageRenderer;
/**
@@ -46,10 +48,20 @@ import org.apache.activemq.web.view.XmlMessageRenderer;
* <li>selector - specifies the SQL 92 selector to apply to the queue</li>
* </ul>
*
- *
+ *
*/
public class QueueBrowseServlet extends HttpServlet {
- private static FactoryFinder factoryFinder = new
FactoryFinder("META-INF/services/org/apache/activemq/web/view/");
+
+ public static final String QUEUE_BROWSE_VIEWS_PROP =
"org.apache.activemq.web.view.QUEUE_BROWSE_CLASSES";
+ public static final String DEFAULT_ALLOWED_VIEWS =
FactoryFinder.buildAllowedImpls(
+ RssMessageRenderer.class, XmlMessageRenderer.class,
SimpleMessageRenderer.class);
+
+ private final FactoryFinder<MessageRenderer> factoryFinder;
+
+ public QueueBrowseServlet() {
+ this.factoryFinder = new
FactoryFinder<>("META-INF/services/org/apache/activemq/web/view/",
+ MessageRenderer.class,
System.getProperty(QUEUE_BROWSE_VIEWS_PROP, DEFAULT_ALLOWED_VIEWS));
+ }
// Implementation methods
//
-------------------------------------------------------------------------
@@ -96,24 +108,17 @@ public class QueueBrowseServlet extends HttpServlet {
style = "simple";
}
try {
- return (MessageRenderer) factoryFinder.newInstance(style);
- }
- catch (IllegalAccessException e) {
- throw new NoSuchViewStyleException(style, e);
- }
- catch (InstantiationException e) {
- throw new NoSuchViewStyleException(style, e);
+ return factoryFinder.newInstance(style);
}
- catch (ClassNotFoundException e) {
+ catch (IllegalAccessException | InstantiationException |
ClassNotFoundException e) {
throw new NoSuchViewStyleException(style, e);
}
}
- @SuppressWarnings("unchecked")
protected void configureRenderer(HttpServletRequest request,
MessageRenderer renderer) {
- Map<String, String> properties = new HashMap<String, String>();
+ Map<String, String> properties = new HashMap<>();
for (Enumeration<String> iter = request.getParameterNames();
iter.hasMoreElements();) {
- String name = (String) iter.nextElement();
+ String name = iter.nextElement();
properties.put(name, request.getParameter(name));
}
IntrospectionSupport.setProperties(renderer, properties);
diff --git
a/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
b/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
new file mode 100644
index 0000000000..8abea6d40a
--- /dev/null
+++
b/activemq-web/src/test/java/org/apache/activemq/web/QueueBrowseServletTest.java
@@ -0,0 +1,230 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.activemq.web;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import jakarta.jms.JMSException;
+import jakarta.jms.Message;
+import jakarta.jms.QueueBrowser;
+import jakarta.servlet.ServletException;
+import jakarta.servlet.http.HttpServletRequest;
+import jakarta.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.io.PrintWriter;
+import org.apache.activemq.util.FactoryFinder;
+import org.apache.activemq.web.view.MessageRenderer;
+import org.apache.activemq.web.view.RssMessageRenderer;
+import org.apache.activemq.web.view.SimpleMessageRenderer;
+import org.apache.activemq.web.view.XmlMessageRenderer;
+import org.junit.AfterClass;
+import org.junit.Before;
+import org.junit.Test;
+
+public class QueueBrowseServletTest {
+
+ private final HttpServletRequest request = mock(HttpServletRequest.class);
+
+ // Before each test reset to defaults
+ @Before
+ public void setUp() {
+ resetDefaultAllowedViews();
+ }
+
+ // clean up after all tests are done finished by resetting the property
back before exiting
+ @AfterClass
+ public static void tearDown() {
+ resetDefaultAllowedViews();
+ }
+
+ @Test
+ public void testDefaultViews() throws Exception {
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+
+ when(request.getParameter("view")).thenReturn("simple");
+ assertEquals(SimpleMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+
+ when(request.getParameter("view")).thenReturn(null);
+ assertEquals(SimpleMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+
+ when(request.getParameter("view")).thenReturn("xml");
+ assertEquals(XmlMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+
+ when(request.getParameter("view")).thenReturn("rss");
+ assertEquals(RssMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+ }
+
+ @Test
+ public void testPathTraversal() throws Exception {
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+ // illegal path traversal
+ when(request.getParameter("view")).thenReturn("../../simple");
+ try {
+ servlet.getMessageRenderer(request);
+ fail("Should have thrown an exception");
+ } catch (NoSuchViewStyleException e) {
+ Throwable rootCause = e.getRootCause();
+ assertTrue(rootCause instanceof InstantiationException);
+ // view is in allow list but wrong interface type
+ assertEquals(rootCause.getMessage(), "Provided key escapes the
FactoryFinder configured directory");
+ }
+ }
+
+ @Test
+ public void testViewAllowlistSimpleOnly() throws Exception {
+ // only allow simple and rss view
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP,
+ FactoryFinder.buildAllowedImpls(SimpleMessageRenderer.class,
+ RssMessageRenderer.class));
+
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+
+ // simple/rss are allowed
+ when(request.getParameter("view")).thenReturn("simple");
+ assertEquals(SimpleMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+ when(request.getParameter("view")).thenReturn("rss");
+ assertEquals(RssMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+
+ // XmlMessageRenderer is not in the allow list so this should be
blocked
+ assertViewAllowListFailure(servlet, "xml");
+ }
+
+ @Test
+ public void testViewAllowlistNone() throws Exception {
+ // Set to none allowed
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP, "");
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+ assertViewAllowListFailure(servlet, "simple");
+ assertViewAllowListFailure(servlet, "xml");
+ }
+
+ @Test
+ public void testViewAllowAll() throws Exception {
+ // Allow all
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP, "*");
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+
+ when(request.getParameter("view")).thenReturn("simple");
+ assertEquals(SimpleMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+ }
+
+ @Test
+ public void testMissingInterface() throws Exception {
+ // Add class with wrong interface to allow list
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP,
InvalidMessageRender.class.getName());
+
+ try {
+ // servlet should fail on creation
+ new QueueBrowseServlet();
+ fail("Should have thrown an exception");
+ } catch (IllegalArgumentException e) {
+
assertTrue(e.getMessage().contains(InvalidMessageRender.class.getName() +
+ " is not assignable to interface " +
MessageRenderer.class.getName()));
+ }
+ when(request.getParameter("view")).thenReturn("invalid-view");
+
+ // set to allow all
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP, "*");
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+
+ // should fail on lookup with same interface issue
+ try {
+ when(request.getParameter("view")).thenReturn("invalid-view");
+ servlet.getMessageRenderer(request);
+ fail("Should not be allowed");
+ } catch (NoSuchViewStyleException e) {
+ Throwable rootCause = e.getRootCause();
+ assertTrue(rootCause instanceof InstantiationException);
+ // view is in allow list but wrong interface type
+
assertTrue(rootCause.getMessage().contains(InvalidMessageRender.class.getName()
+
+ " is not assignable to interface " +
MessageRenderer.class.getName()));
+ }
+ }
+
+ @Test
+ public void testViewCustomAllow() throws Exception {
+ // default view settings
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+ when(request.getParameter("view")).thenReturn("valid-view");
+
+ // custom view is not in the allow list
+ assertViewAllowListFailure(servlet, "valid-view");
+
+ // reset with view added to allow list
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP,
ValidMessageRender.class.getName());
+ servlet = new QueueBrowseServlet();
+
+ assertEquals(ValidMessageRender.class,
servlet.getMessageRenderer(request).getClass());
+ }
+
+ @Test
+ public void testViewsWhiteSpace() throws Exception {
+ // add views with extra white space which should get trimmed
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP,
+ String.join(", ", SimpleMessageRenderer.class.getName(),
+ RssMessageRenderer.class.getName()));
+
+ QueueBrowseServlet servlet = new QueueBrowseServlet();
+ when(request.getParameter("view")).thenReturn("simple");
+ assertEquals(SimpleMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+ when(request.getParameter("view")).thenReturn("rss");
+ assertEquals(RssMessageRenderer.class,
servlet.getMessageRenderer(request).getClass());
+ }
+
+ private void assertViewAllowListFailure(QueueBrowseServlet servlet, String
view) throws Exception {
+ try {
+ when(request.getParameter("view")).thenReturn(view);
+ servlet.getMessageRenderer(request);
+ fail("Should not be allowed");
+ } catch (NoSuchViewStyleException e) {
+ Throwable rootCause = e.getRootCause();
+ assertTrue(rootCause instanceof InstantiationException);
+ assertTrue(rootCause.getMessage().contains("is not an allowed "
+ + "implementation of type interface " +
MessageRenderer.class.getName()));
+ }
+ }
+
+ private static void resetDefaultAllowedViews() {
+ System.setProperty(QueueBrowseServlet.QUEUE_BROWSE_VIEWS_PROP,
+ QueueBrowseServlet.DEFAULT_ALLOWED_VIEWS);
+ }
+
+ // Does not implement the right interface
+ public static class InvalidMessageRender {
+
+ }
+
+ public static class ValidMessageRender implements MessageRenderer {
+
+ @Override
+ public void renderMessages(HttpServletRequest request,
HttpServletResponse response,
+ QueueBrowser browser) throws IOException, JMSException,
ServletException {
+
+ }
+
+ @Override
+ public void renderMessage(PrintWriter writer, HttpServletRequest
request,
+ HttpServletResponse response, QueueBrowser browser, Message
message)
+ throws JMSException, ServletException {
+
+ }
+ }
+}
diff --git
a/activemq-web/src/test/resources/META-INF/services/org/apache/activemq/web/view/invalid-view
b/activemq-web/src/test/resources/META-INF/services/org/apache/activemq/web/view/invalid-view
new file mode 100644
index 0000000000..6338f3286f
--- /dev/null
+++
b/activemq-web/src/test/resources/META-INF/services/org/apache/activemq/web/view/invalid-view
@@ -0,0 +1,19 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+
+# use a class that does not implement MessageRenderer to test validation
+class=org.apache.activemq.web.QueueBrowseServletTest$InvalidMessageRender
diff --git
a/activemq-web/src/test/resources/META-INF/services/org/apache/activemq/web/view/valid-view
b/activemq-web/src/test/resources/META-INF/services/org/apache/activemq/web/view/valid-view
new file mode 100644
index 0000000000..af1e9e360b
--- /dev/null
+++
b/activemq-web/src/test/resources/META-INF/services/org/apache/activemq/web/view/valid-view
@@ -0,0 +1,19 @@
+## ---------------------------------------------------------------------------
+## Licensed to the Apache Software Foundation (ASF) under one or more
+## contributor license agreements. See the NOTICE file distributed with
+## this work for additional information regarding copyright ownership.
+## The ASF licenses this file to You under the Apache License, Version 2.0
+## (the "License"); you may not use this file except in compliance with
+## the License. You may obtain a copy of the License at
+##
+## http://www.apache.org/licenses/LICENSE-2.0
+##
+## Unless required by applicable law or agreed to in writing, software
+## distributed under the License is distributed on an "AS IS" BASIS,
+## WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+## See the License for the specific language governing permissions and
+## limitations under the License.
+## ---------------------------------------------------------------------------
+
+# use a class that does not implement MessageRenderer to test validation
+class=org.apache.activemq.web.QueueBrowseServletTest$ValidMessageRender
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
