From 6483123417ad1d58c7bc8a2c53ad98d1334c2303 Mon Sep 17 00:00:00 2001 From: Clebert Suconic Date: Mon, 17 Oct 2016 18:27:58 -0400 Subject: [PATCH] ARTEMIS-801 Dealing properly with Spaces and Special Characters on broker --- artemis-cli/pom.xml | 9 +++ .../artemis/cli/commands/ActionAbstract.java | 22 ++++++ .../artemis/cli/commands/Configurable.java | 4 +- .../artemis/factory/BrokerFactory.java | 23 +++--- .../artemis/factory/BrokerFactoryHandler.java | 2 +- .../factory/XmlBrokerFactoryHandler.java | 6 +- .../artemis/cli/commands/etc/bootstrap.xml | 4 +- .../apache/activemq/cli/test/ArtemisTest.java | 65 +++++++++++------ .../activemq/cli/test/FileBrokerTest.java | 4 +- .../src/test/scripts/validate-instalation.sh | 72 +++++++++++++++++++ .../src/test/scripts/validate-spaces.sh | 52 +------------- .../activemq/artemis/dto/ServerDTO.java | 11 +-- .../apache/activemq/artemis/dto/XmlUtil.java | 10 ++- .../core/server/reload/ReloadManagerImpl.java | 16 ++++- .../core/reload/ReloadManagerTest.java | 2 +- 15 files changed, 191 insertions(+), 111 deletions(-) create mode 100755 artemis-distribution/src/test/scripts/validate-instalation.sh diff --git a/artemis-cli/pom.xml b/artemis-cli/pom.xml index 1410b5d5ca..e8c8358920 100644 --- a/artemis-cli/pom.xml +++ b/artemis-cli/pom.xml @@ -98,6 +98,15 @@ ${project.version} test + + org.apache.activemq + artemis-commons + ${project.version} + test + test-jar + + + diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java index 431f2ab1e3..17888ad371 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/ActionAbstract.java @@ -17,6 +17,7 @@ package org.apache.activemq.artemis.cli.commands; import java.io.File; +import java.net.URI; import io.airlift.airline.Option; @@ -29,6 +30,8 @@ public abstract class ActionAbstract implements Action { private String brokerHome; + private URI brokerInstanceURI; + protected ActionContext context; @Override @@ -62,6 +65,25 @@ public abstract class ActionAbstract implements Action { return brokerInstance; } + + public URI getBrokerURIInstance() { + + if (brokerInstanceURI == null) { + String instanceProperty = getBrokerInstance(); + + File artemisInstance = null; + if (artemisInstance == null && instanceProperty != null) { + artemisInstance = new File(instanceProperty); + } + + if (artemisInstance != null) { + brokerInstanceURI = artemisInstance.toURI(); + } + } + return brokerInstanceURI; + } + + @Override public String getBrokerHome() { if (brokerHome == null) { diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java index 1df83934fe..316b2c2677 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/cli/commands/Configurable.java @@ -87,7 +87,7 @@ public abstract class Configurable extends ActionAbstract { fileConfiguration = new FileConfiguration(); FileJMSConfiguration jmsConfiguration = new FileJMSConfiguration(); - String serverConfiguration = getBrokerDTO().server.configuration; + String serverConfiguration = getBrokerDTO().server.getConfigurationURI().toASCIIString(); FileDeploymentManager fileDeploymentManager = new FileDeploymentManager(serverConfiguration); fileDeploymentManager.addDeployable(fileConfiguration).addDeployable(jmsConfiguration); fileDeploymentManager.readConfiguration(); @@ -102,7 +102,7 @@ public abstract class Configurable extends ActionAbstract { if (brokerDTO == null) { getConfiguration(); - brokerDTO = BrokerFactory.createBrokerConfiguration(configuration, getBrokerHome(), getBrokerInstance()); + brokerDTO = BrokerFactory.createBrokerConfiguration(configuration, getBrokerHome(), getBrokerInstance(), getBrokerURIInstance()); if (brokerConfig != null) { if (!brokerConfig.startsWith("file:")) { diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactory.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactory.java index 54567fd7a6..a188c61f39 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactory.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactory.java @@ -16,7 +16,6 @@ */ package org.apache.activemq.artemis.factory; -import java.io.File; import java.io.IOException; import java.net.URI; @@ -30,12 +29,13 @@ import org.apache.activemq.artemis.utils.FactoryFinder; public class BrokerFactory { public static BrokerDTO createBrokerConfiguration(URI configURI) throws Exception { - return createBrokerConfiguration(configURI, null, null); + return createBrokerConfiguration(configURI, null, null, null); } public static BrokerDTO createBrokerConfiguration(URI configURI, String artemisHome, - String artemisInstance) throws Exception { + String artemisInstance, + URI artemisURIInstance) throws Exception { if (configURI.getScheme() == null) { throw new ConfigurationException("Invalid configuration URI, no scheme specified: " + configURI); } @@ -48,25 +48,18 @@ public class BrokerFactory { throw new ConfigurationException("Invalid configuration URI, can't find configuration scheme: " + configURI.getScheme()); } - return factory.createBroker(configURI, artemisHome, artemisInstance); + return factory.createBroker(configURI, artemisHome, artemisInstance, artemisURIInstance); } public static BrokerDTO createBrokerConfiguration(String configuration) throws Exception { - return createBrokerConfiguration(new URI(configuration), null, null); + return createBrokerConfiguration(new URI(configuration), null, null, null); } public static BrokerDTO createBrokerConfiguration(String configuration, String artemisHome, - String artemisInstance) throws Exception { - return createBrokerConfiguration(new URI(configuration), artemisHome, artemisInstance); - } - - static String fixupFileURI(String value) { - if (value != null && value.startsWith("file:")) { - value = value.substring("file:".length()); - value = new File(value).toURI().toString(); - } - return value; + String artemisInstance, + URI artemisURIInstance) throws Exception { + return createBrokerConfiguration(new URI(configuration), artemisHome, artemisInstance, artemisURIInstance); } public static Broker createServer(ServerDTO brokerDTO, ActiveMQSecurityManager security) throws Exception { diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactoryHandler.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactoryHandler.java index 2c1cba3470..f81a0de74d 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactoryHandler.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/BrokerFactoryHandler.java @@ -24,5 +24,5 @@ public interface BrokerFactoryHandler { BrokerDTO createBroker(URI brokerURI) throws Exception; - BrokerDTO createBroker(URI brokerURI, String artemisHome, String artemisInstance) throws Exception; + BrokerDTO createBroker(URI brokerURI, String artemisHome, String artemisInstance, URI artemisURIInstance) throws Exception; } diff --git a/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/XmlBrokerFactoryHandler.java b/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/XmlBrokerFactoryHandler.java index 272e46dc14..0059c06e4f 100644 --- a/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/XmlBrokerFactoryHandler.java +++ b/artemis-cli/src/main/java/org/apache/activemq/artemis/factory/XmlBrokerFactoryHandler.java @@ -27,15 +27,15 @@ public class XmlBrokerFactoryHandler implements BrokerFactoryHandler { @Override public BrokerDTO createBroker(URI brokerURI) throws Exception { - return createBroker(brokerURI, null, null); + return createBroker(brokerURI, null, null, null); } @Override - public BrokerDTO createBroker(URI brokerURI, String artemisHome, String artemisInstance) throws Exception { + public BrokerDTO createBroker(URI brokerURI, String artemisHome, String artemisInstance, URI artemisURIInstance) throws Exception { File file = new File(brokerURI.getSchemeSpecificPart()); if (!file.exists()) { throw new ConfigurationException("Invalid configuration URI, can't find file: " + file.getName()); } - return XmlUtil.decode(BrokerDTO.class, file, artemisHome, artemisInstance); + return XmlUtil.decode(BrokerDTO.class, file, artemisHome, artemisInstance, artemisURIInstance); } } diff --git a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/bootstrap.xml b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/bootstrap.xml index ec4a489a4a..d359c25e3b 100644 --- a/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/bootstrap.xml +++ b/artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/bootstrap.xml @@ -20,7 +20,9 @@ - + + ${bootstrap-web-settings} diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java index 58e91bdf47..ba78fb23dd 100644 --- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/ArtemisTest.java @@ -29,6 +29,7 @@ import java.nio.file.Files; import java.util.concurrent.TimeUnit; import org.apache.activemq.artemis.api.core.SimpleString; +import org.apache.activemq.artemis.api.core.client.ActiveMQClient; import org.apache.activemq.artemis.api.core.client.ClientSession; import org.apache.activemq.artemis.api.core.client.ClientSessionFactory; import org.apache.activemq.artemis.api.core.client.ServerLocator; @@ -42,6 +43,8 @@ import org.apache.activemq.artemis.core.client.impl.ServerLocatorImpl; import org.apache.activemq.artemis.jlibaio.LibaioContext; import org.apache.activemq.artemis.jms.client.ActiveMQConnectionFactory; import org.apache.activemq.artemis.jms.client.ActiveMQDestination; +import org.apache.activemq.artemis.spi.core.security.jaas.PropertiesLoader; +import org.apache.activemq.artemis.utils.ThreadLeakCheckRule; import org.junit.After; import org.junit.Assert; import org.junit.Before; @@ -60,6 +63,9 @@ public class ArtemisTest { @Rule public TemporaryFolder temporaryFolder; + @Rule + public ThreadLeakCheckRule leakCheckRule = new ThreadLeakCheckRule(); + private String original = System.getProperty("java.security.auth.login.config"); public ArtemisTest() { @@ -69,12 +75,23 @@ public class ArtemisTest { } @Before - public void setup() { - System.setProperty("java.security.auth.login.config", temporaryFolder.getRoot().getAbsolutePath() + "/etc/login.config"); + public void setup() throws Exception { + setupAuth(); + Run.setEmbedded(true); + PropertiesLoader.resetUsersAndGroupsCache(); + } + + public void setupAuth() throws Exception { + setupAuth(temporaryFolder.getRoot()); + } + + public void setupAuth(File folder) throws Exception { + System.setProperty("java.security.auth.login.config", folder.getAbsolutePath() + "/etc/login.config"); } @After public void cleanup() { + ActiveMQClient.clearThreadPools(); System.clearProperty("artemis.instance"); Run.setEmbedded(false); @@ -123,6 +140,7 @@ public class ArtemisTest { @Test public void testWebConfig() throws Exception { + setupAuth(); Run.setEmbedded(true); //instance1: default using http File instance1 = new File(temporaryFolder.getRoot(), "instance1"); @@ -196,13 +214,34 @@ public class ArtemisTest { @Test public void testSimpleRun() throws Exception { + testSimpleRun("server"); + } + + @Test + public void testWeirdCharacter() throws Exception { + testSimpleRun("test%26%26x86_6"); + } + + + @Test + public void testSpaces() throws Exception { + testSimpleRun("with space"); + } + + + public void testSimpleRun(String folderName) throws Exception { + File instanceFolder = temporaryFolder.newFolder(folderName); + + setupAuth(instanceFolder); String queues = "q1,t2"; String topics = "t1,t2"; + // This is usually set when run from the command line via artemis.profile Run.setEmbedded(true); - Artemis.main("create", temporaryFolder.getRoot().getAbsolutePath(), "--force", "--silent", "--no-web", "--queues", queues, "--topics", topics, "--no-autotune", "--require-login"); - System.setProperty("artemis.instance", temporaryFolder.getRoot().getAbsolutePath()); + Artemis.main("create", instanceFolder.getAbsolutePath(), "--force", "--silent", "--no-web", "--queues", queues, "--topics", topics, "--no-autotune", "--require-login"); + System.setProperty("artemis.instance", instanceFolder.getAbsolutePath()); + // Some exceptions may happen on the initialization, but they should be ok on start the basic core protocol Artemis.internalExecute("run"); @@ -268,24 +307,6 @@ public class ArtemisTest { } } - @Test - public void testAnonymousAutoCreate() throws Exception { - // This is usually set when run from the command line via artemis.profile - - Run.setEmbedded(true); - Artemis.main("create", temporaryFolder.getRoot().getAbsolutePath(), "--force", "--silent", "--no-web", "--no-autotune", "--allow-anonymous", "--user", "a", "--password", "a", "--role", "a"); - System.setProperty("artemis.instance", temporaryFolder.getRoot().getAbsolutePath()); - // Some exceptions may happen on the initialization, but they should be ok on start the basic core protocol - Artemis.internalExecute("run"); - - try { - Assert.assertEquals(Integer.valueOf(100), Artemis.internalExecute("producer", "--message-count", "100")); - Assert.assertEquals(Integer.valueOf(100), Artemis.internalExecute("consumer", "--message-count", "100")); - } finally { - stopServer(); - } - } - private void testCli(String... args) { try { Artemis.main(args); diff --git a/artemis-cli/src/test/java/org/apache/activemq/cli/test/FileBrokerTest.java b/artemis-cli/src/test/java/org/apache/activemq/cli/test/FileBrokerTest.java index baef0fe3a2..296a3d2082 100644 --- a/artemis-cli/src/test/java/org/apache/activemq/cli/test/FileBrokerTest.java +++ b/artemis-cli/src/test/java/org/apache/activemq/cli/test/FileBrokerTest.java @@ -16,6 +16,7 @@ */ package org.apache.activemq.cli.test; +import java.io.File; import java.io.IOException; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; @@ -104,7 +105,8 @@ public class FileBrokerTest { Assert.assertNotNull(activeMQServer); Assert.assertTrue(activeMQServer.isStarted()); Assert.assertTrue(broker.isStarted()); - path = activeMQServer.getConfiguration().getConfigurationUrl().getPath(); + File file = new File(activeMQServer.getConfiguration().getConfigurationUrl().toURI()); + path = file.getPath(); Assert.assertNotNull(activeMQServer.getConfiguration().getConfigurationUrl()); Thread.sleep(activeMQServer.getConfiguration().getConfigurationFileRefreshPeriod() * 2); diff --git a/artemis-distribution/src/test/scripts/validate-instalation.sh b/artemis-distribution/src/test/scripts/validate-instalation.sh new file mode 100755 index 0000000000..85773a41d1 --- /dev/null +++ b/artemis-distribution/src/test/scripts/validate-instalation.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env sh +# 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. + +# This script will validate the distribution works with folders with spaces on Linux machines + +echo validating instalation on $1 +rm -rf target +mkdir target +mkdir target/"$1" + + +# Setting the script to fail if anything goes wrong +set -e + + +export TEST_TARGET="./target/$1" + +. ./installHome.sh + + +export ARTEMIS_INSTANCE="$CURRENT_DIR/target/$1/artemis_instance" +echo home used is $ARTEMIS_HOME +echo artemis instance is $ARTEMIS_HOME + + +cd "$ARTEMIS_HOME/bin" +./artemis create --silent --force "$ARTEMIS_INSTANCE" + +cd "$ARTEMIS_INSTANCE/bin" +pwd + +./artemis run & + +sleep 5 + +./artemis producer +./artemis consumer + +./artemis stop + +sleep 5 +./artemis data print > data.log +./artemis data compact +./artemis data exp + + +./artemis-service start + +sleep 5 + +./artemis producer +./artemis consumer + +./artemis-service stop + +cd $CURRENT_DIR +rm -rf target diff --git a/artemis-distribution/src/test/scripts/validate-spaces.sh b/artemis-distribution/src/test/scripts/validate-spaces.sh index 97b5528a3a..391773335d 100755 --- a/artemis-distribution/src/test/scripts/validate-spaces.sh +++ b/artemis-distribution/src/test/scripts/validate-spaces.sh @@ -18,54 +18,4 @@ # This script will validate the distribution works with folders with spaces on Linux machines -rm -rf target -mkdir target -mkdir target/with\ space - - -# Setting the script to fail if anything goes wrong -set -e - - -export TEST_TARGET="./target/with space" - -. ./installHome.sh - - -export ARTEMIS_INSTANCE="$CURRENT_DIR/target/with space/artemis_instance" -echo home used is $ARTEMIS_HOME -echo artemis instance is $ARTEMIS_HOME - - -cd "$ARTEMIS_HOME/bin" -./artemis create --silent --force "$ARTEMIS_INSTANCE" - -cd "$ARTEMIS_INSTANCE/bin" -pwd - -./artemis run & - -sleep 5 - -./artemis producer -./artemis consumer - -./artemis stop - -sleep 5 -./artemis data print > data.log -./artemis data compact -./artemis data exp - - -./artemis-service start - -sleep 5 - -./artemis producer -./artemis consumer - -./artemis-service stop - -cd $CURRENT_DIR -rm -rf target +./validate-instalation.sh with\ spaces\ And\ Weird\ %26\ Characters diff --git a/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/ServerDTO.java b/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/ServerDTO.java index d09bef1a28..2cf06645f5 100644 --- a/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/ServerDTO.java +++ b/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/ServerDTO.java @@ -36,7 +36,7 @@ public class ServerDTO { public URI getConfigurationURI() throws Exception { if (configurationURI == null) { - configurationURI = new URI(fixupFileURI(configuration)); + configurationURI = new URI(configuration); } return configurationURI; @@ -44,17 +44,10 @@ public class ServerDTO { public File getConfigurationFile() throws Exception { if (configurationFile == null) { - configurationFile = new File(new URI(fixupFileURI(configuration)).getSchemeSpecificPart()); + configurationFile = new File(getConfigurationURI()); } return configurationFile; } - private static String fixupFileURI(String value) { - if (value != null && value.startsWith("file:")) { - value = value.substring("file:".length()); - value = new File(value).toURI().toString(); - } - return value; - } } diff --git a/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/XmlUtil.java b/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/XmlUtil.java index 1af8542ea7..8ed7642458 100644 --- a/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/XmlUtil.java +++ b/artemis-dto/src/main/java/org/apache/activemq/artemis/dto/XmlUtil.java @@ -28,6 +28,7 @@ import javax.xml.validation.SchemaFactory; import java.io.File; import java.io.FileInputStream; import java.io.InputStream; +import java.net.URI; import java.util.Properties; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -75,7 +76,7 @@ public class XmlUtil { private static final XMLInputFactory factory = XMLInputFactory.newInstance(); public static T decode(Class clazz, File configuration) throws Exception { - return decode(clazz, configuration, null, null); + return decode(clazz, configuration, null, null, null); } /** @@ -84,7 +85,8 @@ public class XmlUtil { public static T decode(Class clazz, File configuration, String artemisHome, - String artemisInstance) throws Exception { + String artemisInstance, + URI artemisURIInstance) throws Exception { JAXBContext jaxbContext = JAXBContext.newInstance("org.apache.activemq.artemis.dto"); Unmarshaller unmarshaller = jaxbContext.createUnmarshaller(); @@ -104,6 +106,10 @@ public class XmlUtil { props.put("artemis.instance", artemisInstance); } + if (artemisURIInstance != null) { + props.put("artemis.URI.instance", artemisURIInstance.toString()); + } + XMLStreamReader reader = factory.createXMLStreamReader(new FileInputStream(configuration)); reader = new PropertiesFilter(reader, props); diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java index a8c8b69abc..cad3f330c0 100644 --- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java +++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/reload/ReloadManagerImpl.java @@ -85,14 +85,24 @@ public class ReloadManagerImpl extends ActiveMQScheduledComponent implements Rel class ReloadRegistry { - private final File file; + private File file; private final URL uri; private long lastModified; private final List callbacks = new LinkedList<>(); - ReloadRegistry(URL uri) { - this.file = new File(uri.getPath()); + ReloadRegistry(URL uri) { + try { + file = new File(uri.toURI()); + } catch (Exception e) { + logger.warn(e.getMessage(), e); + file = new File(uri.getPath()); + } + + if (!file.exists()) { + logger.warn("File " + file + " does not exist"); + } + this.lastModified = file.lastModified(); this.uri = uri; } diff --git a/artemis-server/src/test/java/org/apache/activemq/artemis/core/reload/ReloadManagerTest.java b/artemis-server/src/test/java/org/apache/activemq/artemis/core/reload/ReloadManagerTest.java index e75ebc85ff..fa82ae0221 100644 --- a/artemis-server/src/test/java/org/apache/activemq/artemis/core/reload/ReloadManagerTest.java +++ b/artemis-server/src/test/java/org/apache/activemq/artemis/core/reload/ReloadManagerTest.java @@ -68,7 +68,7 @@ public class ReloadManagerTest extends ActiveMQTestBase { @Test public void testUpdateWithSpace() throws Exception { - File spaceDir = new File(getTemporaryDir(), "./with space"); + File spaceDir = new File(getTemporaryDir(), "./with %25space"); spaceDir.mkdirs(); File file = new File(spaceDir, "checkFile.tst"); internalTest(manager, file);