diff --git a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISchema.java b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISchema.java index 0d6940e543..25ce8e9f9b 100644 --- a/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISchema.java +++ b/artemis-commons/src/main/java/org/apache/activemq/artemis/utils/uri/URISchema.java @@ -21,6 +21,7 @@ import java.io.UnsupportedEncodingException; import java.net.URI; import java.net.URISyntaxException; import java.net.URLDecoder; +import java.net.URLEncoder; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -96,7 +97,16 @@ public abstract class URISchema { protected abstract T internalNewObject(URI uri, Map query, P param) throws Exception; - protected abstract URI internalNewURI(T bean) throws Exception; + /** This is the default implementation. + * Sub classes are should provide a proper implementation for their schemas. */ + protected URI internalNewURI(T bean) throws Exception { + String query = URISchema.getData(null, bean); + + return new URI(getSchemaName(), + null, + "//", query, null); + + } private static final BeanUtilsBean beanUtils = new BeanUtilsBean(); @@ -106,6 +116,14 @@ public abstract class URISchema { } } + public static String decodeURI(String value) throws UnsupportedEncodingException { + return URLDecoder.decode(value, "UTF-8"); + } + + public static String encodeURI(String value) throws UnsupportedEncodingException { + return URLEncoder.encode(value, "UTF-8"); + } + static { // This is to customize the BeanUtils to use Fluent Proeprties as well beanUtils.getPropertyUtils().addBeanIntrospector(new FluentPropertyBeanIntrospectorWithIgnores()); @@ -120,8 +138,8 @@ public abstract class URISchema { for (int i = 0; i < parameters.length; i++) { int p = parameters[i].indexOf("="); if (p >= 0) { - String name = URLDecoder.decode(parameters[i].substring(0, p), "UTF-8"); - String value = URLDecoder.decode(parameters[i].substring(p + 1), "UTF-8"); + String name = decodeURI(parameters[i].substring(0, p)); + String value = decodeURI(parameters[i].substring(p + 1)); rc.put(name, value); } else { @@ -193,6 +211,7 @@ public abstract class URISchema { public static String getData(List ignored, Object... beans) throws Exception { StringBuilder sb = new StringBuilder(); + boolean empty = true; synchronized (beanUtils) { for (Object bean : beans) { if (bean != null) { @@ -201,7 +220,11 @@ public abstract class URISchema { if (descriptor.getReadMethod() != null && isWriteable(descriptor, ignored)) { String value = beanUtils.getProperty(bean, descriptor.getName()); if (value != null) { - sb.append("&").append(descriptor.getName()).append("=").append(value); + if (!empty) { + sb.append("&"); + } + empty = false; + sb.append(descriptor.getName()).append("=").append(encodeURI(value)); } } } diff --git a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/URIParserTest.java b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/URIParserTest.java index f2fd107b8b..183d586f62 100644 --- a/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/URIParserTest.java +++ b/artemis-commons/src/test/java/org/apache/activemq/artemis/utils/URIParserTest.java @@ -44,6 +44,29 @@ public class URIParserTest { Assert.assertEquals("green", fruit.getColor()); Assert.assertEquals("something", fruit.getFluentName()); + } + + /** + * this is just a simple test to validate the model + * + * @throws Throwable + */ + @Test + public void testGenerateWithEncoding() throws Throwable { + FruitParser parser = new FruitParser(); + Fruit myFruit = new Fruit("tomato&fruit"); + myFruit.setHost("somehost&uui"); + // I'm trying to break things as you can see here with some weird encoding + myFruit.setFluentName("apples&bananas with &host=3344"); + URI uri = parser.createSchema("fruit", myFruit); + + Fruit newFruit = (Fruit)parser.newObject(uri, "something"); + + Assert.assertEquals(myFruit.getHost(), newFruit.getHost()); + Assert.assertEquals(myFruit.getFluentName(), newFruit.getFluentName()); + + + } /** @@ -96,10 +119,6 @@ public class URIParserTest { return setData(uri, new Fruit(getSchemaName()), query); } - @Override - protected URI internalNewURI(FruitBase bean) { - return null; - } } class FruitBaseSchema extends URISchema { @@ -113,11 +132,6 @@ public class URIParserTest { public FruitBase internalNewObject(URI uri, Map query, String fruitName) throws Exception { return setData(uri, new FruitBase(getSchemaName()), query); } - - @Override - protected URI internalNewURI(FruitBase bean) { - return null; - } } public static class FruitBase { diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java index 59a4c78ba5..4893f6cb85 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/core/client/impl/ServerLocatorImpl.java @@ -1630,6 +1630,14 @@ public final class ServerLocatorImpl implements ServerLocatorInternal, Discovery topology.removeClusterTopologyListener(listener); } + /** + * for tests only and not part of the public interface. Do not use it. + * @return + */ + public TransportConfiguration[] getInitialConnectors() { + return initialConnectors; + } + private void addFactory(ClientSessionFactoryInternal factory) { if (factory == null) { return; diff --git a/artemis-core-client/src/main/java/org/apache/activemq/artemis/uri/schema/serverLocator/TCPServerLocatorSchema.java b/artemis-core-client/src/main/java/org/apache/activemq/artemis/uri/schema/serverLocator/TCPServerLocatorSchema.java index b330d0ea0e..d141ee60df 100644 --- a/artemis-core-client/src/main/java/org/apache/activemq/artemis/uri/schema/serverLocator/TCPServerLocatorSchema.java +++ b/artemis-core-client/src/main/java/org/apache/activemq/artemis/uri/schema/serverLocator/TCPServerLocatorSchema.java @@ -102,19 +102,30 @@ public class TCPServerLocatorSchema extends AbstractServerLocatorSchema { return params.get("host") != null ? (String) params.get("host") : "localhost"; } - private static String createQuery(Map params, String query) { + private static String createQuery(Map params, String query) throws Exception { StringBuilder cb; + boolean empty; if (query == null) { cb = new StringBuilder(); + empty = true; } else { cb = new StringBuilder(query); + empty = false; } - for (String param : params.keySet()) { - if (cb.length() > 0) { - cb.append("&"); + + for (Map.Entry entry : params.entrySet()) { + if (entry.getValue() != null) { + if (!empty) { + cb.append("&"); + } + else { + empty = false; + } + cb.append(encodeURI(entry.getKey())); + cb.append("="); + cb.append(encodeURI(entry.getValue().toString())); } - cb.append(param).append("=").append(params.get(param)); } return cb.toString(); } diff --git a/artemis-jms-client/src/test/java/org/apache/activemq/artemis/uri/ConnectionFactoryURITest.java b/artemis-jms-client/src/test/java/org/apache/activemq/artemis/uri/ConnectionFactoryURITest.java index caf284d2d6..6f5a7cfbac 100644 --- a/artemis-jms-client/src/test/java/org/apache/activemq/artemis/uri/ConnectionFactoryURITest.java +++ b/artemis-jms-client/src/test/java/org/apache/activemq/artemis/uri/ConnectionFactoryURITest.java @@ -38,6 +38,8 @@ import org.apache.activemq.artemis.api.core.TransportConfiguration; import org.apache.activemq.artemis.api.core.UDPBroadcastEndpointFactory; import org.apache.activemq.artemis.api.jms.ActiveMQJMSClient; import org.apache.activemq.artemis.api.jms.JMSFactoryType; +import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; +import org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnector; import org.apache.activemq.artemis.core.remoting.impl.netty.NettyConnectorFactory; import org.apache.activemq.artemis.core.remoting.impl.netty.TransportConstants; import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; @@ -78,6 +80,32 @@ public class ConnectionFactoryURITest { } } + @Test + public void testWeirdEncodingsOnIP() throws Exception { + // This is to make things worse. Having & and = on the property shouldn't break it + final String BROKEN_PROPERTY = "e80::56ee:75ff:fe53:e6a7%25enp0s25&host=[fe80::56ee:75ff:fe53:e6a7]#"; + + Map params = new HashMap<>(); + params.put(TransportConstants.LOCAL_ADDRESS_PROP_NAME, BROKEN_PROPERTY); + + TransportConfiguration configuration = new TransportConfiguration(NettyConnector.class.getName(), params); + + + ActiveMQConnectionFactory factory = ActiveMQJMSClient.createConnectionFactoryWithHA(JMSFactoryType.CF, configuration); + + URI uri = factory.toURI(); + + ActiveMQConnectionFactory newFactory = ActiveMQJMSClient.createConnectionFactory(uri.toString(), "somefactory"); + + + TransportConfiguration[] initialConnectors = ((ServerLocatorImpl)newFactory.getServerLocator()).getInitialConnectors(); + + Assert.assertEquals(1, initialConnectors.length); + + + Assert.assertEquals(BROKEN_PROPERTY, initialConnectors[0].getParams().get(TransportConstants.LOCAL_ADDRESS_PROP_NAME).toString()); + } + @Test public void testIPv6NewURI() throws Exception { for (String IPV6 : V6IPs) {