ARTEMIS-801 Dealing properly with Spaces and Special Characters on broker

artemis-cli/pom.xml

@@ -98,6 +98,15 @@
          <version>${project.version}</version>
          <scope>test</scope>
       </dependency>
+      <dependency>
+         <groupId>org.apache.activemq</groupId>
+         <artifactId>artemis-commons</artifactId>
+         <version>${project.version}</version>
+         <scope>test</scope>
+         <type>test-jar</type>
+      </dependency>
+
+
    </dependencies>
 
    <build>

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) {

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:")) {

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 {
+                                                     String artemisInstance,
-      return createBrokerConfiguration(new URI(configuration), artemisHome, artemisInstance);
+                                                     URI artemisURIInstance) throws Exception {
-   }
+      return createBrokerConfiguration(new URI(configuration), artemisHome, artemisInstance, artemisURIInstance);
-
-   static String fixupFileURI(String value) {
-      if (value != null && value.startsWith("file:")) {
-         value = value.substring("file:".length());
-         value = new File(value).toURI().toString();
-      }
-      return value;
    }
 
    public static Broker createServer(ServerDTO brokerDTO, ActiveMQSecurityManager security) throws Exception {

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;
 }

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);
    }
 }

artemis-cli/src/main/resources/org/apache/activemq/artemis/cli/commands/etc/bootstrap.xml

@@ -20,7 +20,9 @@
 
    <jaas-security domain="activemq"/>
 
-   <server configuration="file:${artemis.instance}/etc/broker.xml"/>
+   <!-- artemis.URI.instance is parsed from artemis.instance by the CLI startup.
+        This is to avoid situations where you could have spaces or special characters on this URI -->
+   <server configuration="${artemis.URI.instance}/etc/broker.xml"/>
 
 ${bootstrap-web-settings}
 

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() {
+   public void setup() throws Exception {
-      System.setProperty("java.security.auth.login.config", temporaryFolder.getRoot().getAbsolutePath() + "/etc/login.config");
+      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");
+      Artemis.main("create", instanceFolder.getAbsolutePath(), "--force", "--silent", "--no-web", "--queues", queues, "--topics", topics, "--no-autotune", "--require-login");
-      System.setProperty("artemis.instance", temporaryFolder.getRoot().getAbsolutePath());
+      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);

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);

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

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
+./validate-instalation.sh with\ spaces\ And\ Weird\ %26\ Characters
-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

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;
-   }
 
 }

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> T decode(Class<T> 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> T decode(Class<T> 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);

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<ReloadCallback> callbacks = new LinkedList<>();
 
       ReloadRegistry(URL uri)  {
-         this.file = new File(uri.getPath());
+         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;
       }

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);