QPID-4352: fix break in ConnectionFactory following SSL truststore/keystore password masking in URL toString().

Makes SSLTest use a ConnectionFactory instead of instantiating AMQConnections directly, exposing this issue.
Replaces horrible use of an AMQConnectionURL objects in MultipleBrokersFailoverTest and FailoverBehaviourTest (which both broke after this fix) with fully string-generated connection strings.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1393034 13f79535-47bb-0310-9956-ffa450edef68

4 files changed, 56 insertions, 30 deletions

diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java
index 8bc815d98e..a2d4b5ee17 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQConnectionFactory.java
@@ -140,7 +140,7 @@ public class AMQConnectionFactory implements ConnectionFactory, QueueConnectionF
         {
             try
             {
-                ConnectionURL connectionDetails = new AMQConnectionURL(_connectionDetails.toString());
+                ConnectionURL connectionDetails = new AMQConnectionURL(_connectionDetails.getURL());
                 connectionDetails.setUsername(userName);
                 connectionDetails.setPassword(password);
                 
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java
index b0d1750643..4f8a6ee54a 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/FailoverBehaviourTest.java
@@ -25,11 +25,12 @@ import org.apache.qpid.client.AMQSession;
 import org.apache.qpid.framing.AMQShortString;
 import org.apache.qpid.jms.BrokerDetails;
 import org.apache.qpid.jms.ConnectionListener;
-import org.apache.qpid.jms.ConnectionURL;
 import org.apache.qpid.jms.FailoverPolicy;
 import org.apache.qpid.test.utils.FailoverBaseCase;
+import org.apache.qpid.url.URLSyntaxException;
 
 import javax.jms.Connection;
+import javax.jms.ConnectionFactory;
 import javax.jms.Destination;
 import javax.jms.ExceptionListener;
 import javax.jms.JMSException;
@@ -929,18 +930,22 @@ public class FailoverBehaviourTest extends FailoverBaseCase implements Connectio
         return queue;
     }
 
-    private AMQConnection createConnectionWithFailover() throws NamingException, JMSException
+    private AMQConnection createConnectionWithFailover() throws NamingException, JMSException, URLSyntaxException
     {
-        AMQConnection connection;
-        AMQConnectionFactory connectionFactory = (AMQConnectionFactory)getConnectionFactory("default");
-        ConnectionURL connectionURL = connectionFactory.getConnectionURL();
-        connectionURL.setOption(ConnectionURL.OPTIONS_FAILOVER, "singlebroker");
-        connectionURL.setOption(ConnectionURL.OPTIONS_FAILOVER_CYCLE, "2");
-        BrokerDetails details = connectionURL.getBrokerDetails(0);
-        details.setProperty(BrokerDetails.OPTIONS_RETRY, "200");
-        details.setProperty(BrokerDetails.OPTIONS_CONNECT_DELAY, "1000");
+        BrokerDetails origBrokerDetails = ((AMQConnectionFactory) getConnectionFactory("default")).getConnectionURL().getBrokerDetails(0);
 
-        connection = (AMQConnection)connectionFactory.createConnection("admin", "admin");
+        String retries = "200";
+        String connectdelay = "1000";
+        String cycleCount = "2";
+
+        String newUrlFormat="amqp://username:password@clientid/test?brokerlist=" +
+                            "'tcp://%s:%s?retries='%s'&connectdelay='%s''&failover='singlebroker?cyclecount='%s''";
+
+        String newUrl = String.format(newUrlFormat, origBrokerDetails.getHost(), origBrokerDetails.getPort(),
+                                                    retries, connectdelay, cycleCount);
+
+        ConnectionFactory connectionFactory = new AMQConnectionFactory(newUrl);
+        AMQConnection connection = (AMQConnection) connectionFactory.createConnection("admin", "admin");
         connection.setConnectionListener(this);
         return connection;
     }
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/MultipleBrokersFailoverTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/MultipleBrokersFailoverTest.java
index be21fbb115..e9798f15b5 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/MultipleBrokersFailoverTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/client/failover/MultipleBrokersFailoverTest.java
@@ -50,10 +50,9 @@ public class MultipleBrokersFailoverTest extends QpidBrokerTestCase implements C
     {
         super.setUp();
 
+        int numBrokers = 4;
         int port = findFreePort();
-        _brokerPorts = new int[4];
-        _connectionURL = new AMQConnectionURL("amqp://guest:guest@test/" + FAILOVER_VIRTUAL_HOST
-                + "?&failover='roundrobin?cyclecount='1''");
+        _brokerPorts = new int[numBrokers];
 
         // we need to create 4 brokers:
         // 1st broker will be running in test JVM and will not have failover host (only tcp connection will established, amqp connection will be closed)
@@ -63,7 +62,7 @@ public class MultipleBrokersFailoverTest extends QpidBrokerTestCase implements C
 
         // the test should connect to the second broker first and fail over to the forth broker
         // after unsuccessful try to establish the connection to the 3d broker
-        for (int i = 0; i < _brokerPorts.length; i++)
+        for (int i = 0; i < numBrokers; i++)
         {
             if (i > 0)
             {
@@ -92,16 +91,38 @@ public class MultipleBrokersFailoverTest extends QpidBrokerTestCase implements C
 
             startBroker(port, testConfiguration, testVirtualhosts);
             revertSystemProperties();
-
-            _connectionURL.addBrokerDetails(new AMQBrokerDetails(String.format(BROKER_PORTION_FORMAT, port,
-                    FAILOVER_CONNECTDELAY, FAILOVER_RETRIES)));
         }
+
+        _connectionURL = new AMQConnectionURL(generateUrlString(numBrokers));
+
         _connection = getConnection(_connectionURL);
         ((AMQConnection) _connection).setConnectionListener(this);
         _failoverComplete = new CountDownLatch(1);
         _failoverStarted = new CountDownLatch(1);
     }
 
+    private String generateUrlString(int numBrokers)
+    {
+        String baseString = "amqp://guest:guest@test/" + FAILOVER_VIRTUAL_HOST
+                            + "?&failover='roundrobin?cyclecount='1''&brokerlist='";
+        StringBuffer buffer = new StringBuffer(baseString);
+
+        for(int i = 0; i< numBrokers ; i++)
+        {
+            if(i != 0)
+            {
+                buffer.append(";");
+            }
+
+            String broker = String.format(BROKER_PORTION_FORMAT, _brokerPorts[i],
+                                          FAILOVER_CONNECTDELAY, FAILOVER_RETRIES);
+            buffer.append(broker);
+        }
+        buffer.append("'");
+
+        return buffer.toString();
+    }
+
     public void tearDown() throws Exception
     {
         try
diff --git a/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java b/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
index 39689f5096..51d840629d 100644
--- a/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
+++ b/qpid/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
@@ -20,11 +20,11 @@
  */
 package org.apache.qpid.client.ssl;
 
-import org.apache.qpid.client.AMQConnection;
+import org.apache.qpid.client.AMQConnectionURL;
 import org.apache.qpid.client.AMQTestConnection_0_10;
 import org.apache.qpid.test.utils.QpidBrokerTestCase;
-import org.apache.qpid.transport.Connection;
 
+import javax.jms.Connection;
 import javax.jms.Session;
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
@@ -76,8 +76,8 @@ public class SSLTest extends QpidBrokerTestCase
             
             url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT,
                     KEYSTORE,KEYSTORE_PASSWORD,TRUSTSTORE,TRUSTSTORE_PASSWORD);
-            
-            AMQConnection con = new AMQConnection(url);
+
+            Connection con = getConnection(new AMQConnectionURL(url));
             assertNotNull("connection should be successful", con);
             Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
             assertNotNull("create session should be successful", ssn);
@@ -93,7 +93,7 @@ public class SSLTest extends QpidBrokerTestCase
 
             url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT);
             
-            AMQConnection con = new AMQConnection(url);
+            Connection con = getConnection(new AMQConnectionURL(url));
             assertNotNull("connection should be successful", con);
             Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
             assertNotNull("create session should be successful", ssn);
@@ -109,7 +109,7 @@ public class SSLTest extends QpidBrokerTestCase
             "?ssl='true'&ssl_cert_alias='" + CERT_ALIAS_APP1 + "''";
             
             AMQTestConnection_0_10 con = new AMQTestConnection_0_10(url);      
-            Connection transportCon = con.getConnection();
+            org.apache.qpid.transport.Connection transportCon = con.getConnection();
             String userID = transportCon.getSecurityLayer().getUserID();
             assertEquals("The correct certificate was not choosen","app1@acme.org",userID);
             con.close();
@@ -136,7 +136,7 @@ public class SSLTest extends QpidBrokerTestCase
             
             try
             {
-                AMQConnection con = new AMQConnection(url);
+                getConnection(new AMQConnectionURL(url));
                 fail("Hostname verification failed. No exception was thrown");
             }
             catch (Exception e)
@@ -158,8 +158,8 @@ public class SSLTest extends QpidBrokerTestCase
             QpidBrokerTestCase.DEFAULT_SSL_PORT + 
             "?ssl='true'&ssl_verify_hostname='true''";
 
-            AMQConnection con = new AMQConnection(url);
-            assertNotNull("connection should have been created", con); 
+            Connection con = getConnection(new AMQConnectionURL(url));
+            assertNotNull("connection should have been created", con);
         }
     }
     
@@ -171,7 +171,7 @@ public class SSLTest extends QpidBrokerTestCase
             QpidBrokerTestCase.DEFAULT_SSL_PORT + 
             "?ssl='true'&ssl_verify_hostname='true''";
 
-            AMQConnection con = new AMQConnection(url);
+            Connection con = getConnection(new AMQConnectionURL(url));
             assertNotNull("connection should have been created", con);
         }        
     }
@@ -193,7 +193,7 @@ public class SSLTest extends QpidBrokerTestCase
 
             url = String.format(url,QpidBrokerTestCase.DEFAULT_SSL_PORT, TRUSTSTORE,TRUSTSTORE_PASSWORD);
 
-            AMQConnection con = new AMQConnection(url);
+            Connection con = getConnection(new AMQConnectionURL(url));
             assertNotNull("connection should be successful", con);
             Session ssn = con.createSession(false,Session.AUTO_ACKNOWLEDGE); 
             assertNotNull("create session should be successful", ssn);