QPID-2662: Use actual SocketAddress instead of the String representation

Applied patch from Andrew Kennedy <andrew.international@gmail.com>


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

13 files changed, 68 insertions, 85 deletions

diff --git a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
index ae2baa95ca..a6ea9d261e 100644
--- a/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
+++ b/java/broker-plugins/firewall/src/main/java/org/apache/qpid/server/security/access/plugins/Firewall.java
@@ -21,13 +21,10 @@
 package org.apache.qpid.server.security.access.plugins;
 
 import java.net.InetAddress;
-import java.net.UnknownHostException;
-import java.util.List;
+import java.net.InetSocketAddress;
 
-import org.apache.commons.configuration.CompositeConfiguration;
 import org.apache.commons.configuration.Configuration;
 import org.apache.commons.configuration.ConfigurationException;
-import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.qpid.server.configuration.plugins.ConfigurationPlugin;
 import org.apache.qpid.server.security.AbstractPlugin;
 import org.apache.qpid.server.security.Result;
@@ -87,28 +84,19 @@ public class Firewall extends AbstractPlugin
         {
             return Result.ABSTAIN; // We are only interested in access to virtualhosts
         }
-        
-        // TODO alter 0-10 code path to expose the SocketAddress object?
-        String address = (String) instance;
-        
-        if (address == null || address.trim().length() == 0)
+
+        if (!(instance instanceof InetSocketAddress))
         {
-            return Result.ABSTAIN; // We need an address
+            return Result.ABSTAIN; // We need an internet address
         }
 
+        InetAddress address = ((InetSocketAddress) instance).getAddress();
+        
         try
         {
-            int slash = address.indexOf('/');
-            int colon = address.indexOf(':');
-	        InetAddress addr = InetAddress.getByName(address.substring(slash == -1 ? 0 : slash + 1, colon == -1 ? address.length() : colon));
-            if (addr == null)
-            {
-                return Result.ABSTAIN; // Not a real address
-            }
-    
             for (FirewallRule rule : _rules)
             {
-                boolean match = rule.match(addr);
+                boolean match = rule.match(address);
                 if (match)
                 {
                     return rule.getAccess();
@@ -116,11 +104,6 @@ public class Firewall extends AbstractPlugin
             }
             return getDefault();
         }
-        catch (UnknownHostException uhe)
-        {
-            _logger.error("Address format invalid: " + address, uhe);
-            return Result.DENIED;
-        }
         catch (FirewallException fe)
         {
             return Result.DENIED;
diff --git a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java
index e688114461..ab8957e7ef 100644
--- a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java
+++ b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallConfigurationTest.java
@@ -24,33 +24,16 @@ import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.io.RandomAccessFile;
+import java.net.InetSocketAddress;
 
-import junit.framework.TestCase;
-
-import org.apache.qpid.server.protocol.AMQProtocolEngine;
-import org.apache.qpid.server.protocol.AMQProtocolSession;
 import org.apache.qpid.server.registry.ApplicationRegistry;
 import org.apache.qpid.server.registry.ConfigurationFileApplicationRegistry;
+import org.apache.qpid.server.util.InternalBrokerBaseCase;
 import org.apache.qpid.server.virtualhost.VirtualHost;
 import org.apache.qpid.server.virtualhost.VirtualHostRegistry;
-import org.apache.qpid.transport.TestNetworkDriver;
 
-public class FirewallConfigurationTest extends TestCase
+public class FirewallConfigurationTest extends InternalBrokerBaseCase
 {
-    @Override
-    public void setUp()
-    {
-        //Highlight that this test will cause a new AR to be created
-        //ApplicationRegistry.getInstance();
-    }
-
-    @Override
-    public void tearDown() throws Exception
-    {
-        //Correctly Close the AR we created
-        //ApplicationRegistry.remove();
-    }
-
     public void testFirewallConfiguration() throws Exception
     {
         // Write out config
@@ -65,8 +48,8 @@ public class FirewallConfigurationTest extends TestCase
             ApplicationRegistry.initialise(reg, 1);
 
             // Test config
-            assertFalse(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
-            assertTrue(reg.getSecurityManager().accessVirtualhost("test", "127.1.2.3"));
+            assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
+            assertTrue(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.1.2.3", 65535)));
         }
         finally
         {
@@ -94,6 +77,7 @@ public class FirewallConfigurationTest extends TestCase
         out = new FileWriter(fileA);
         out.write("<broker>\n");
         out.write("\t<plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>\n");
+        out.write("\t<cache-directory>${QPID_WORK}/cache</cache-directory>\n");
         out.write("\t<management><enabled>false</enabled></management>\n");
         out.write("\t<security>\n");
         out.write("\t\t<principal-databases>\n");
@@ -137,7 +121,7 @@ public class FirewallConfigurationTest extends TestCase
             ApplicationRegistry.initialise(reg, 1);
 
             // Test config
-            assertFalse(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
+            assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
         }
         finally
         {
@@ -160,14 +144,14 @@ public class FirewallConfigurationTest extends TestCase
             ApplicationRegistry.initialise(reg, 1);
 
             // Test config
-            assertFalse(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
+            assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
 
             // Switch to deny the connection
             writeConfigFile(mainFile, true);
 
             reg.getConfiguration().reparseConfigFileSecuritySections();
 
-            assertTrue(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
+            assertTrue(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
         }
         finally
         {
@@ -238,7 +222,7 @@ public class FirewallConfigurationTest extends TestCase
             ApplicationRegistry.initialise(reg, 1);
 
             // Test config
-            assertFalse(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
+            assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
 
             RandomAccessFile fileBRandom = new RandomAccessFile(fileB, "rw");
             fileBRandom.setLength(0);
@@ -253,7 +237,7 @@ public class FirewallConfigurationTest extends TestCase
 
             reg.getConfiguration().reparseConfigFileSecuritySections();
 
-            assertTrue(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
+            assertTrue(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
 
             fileBRandom = new RandomAccessFile(fileB, "rw");
             fileBRandom.setLength(0);
@@ -268,7 +252,7 @@ public class FirewallConfigurationTest extends TestCase
 
             reg.getConfiguration().reparseConfigFileSecuritySections();
 
-            assertFalse(reg.getSecurityManager().accessVirtualhost("test", "127.0.0.1"));
+            assertFalse(reg.getSecurityManager().accessVirtualhost("test", new InetSocketAddress("127.0.0.1", 65535)));
         }
         finally
         {
diff --git a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
index 89dba035e4..2b04962c89 100644
--- a/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
+++ b/java/broker-plugins/firewall/src/test/java/org/apache/qpid/server/security/access/FirewallPluginTest.java
@@ -23,11 +23,10 @@ import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.net.InetSocketAddress;
-
+import java.net.SocketAddress;
 
 import org.apache.commons.configuration.ConfigurationException;
 import org.apache.commons.configuration.XMLConfiguration;
-import org.apache.qpid.server.configuration.ServerConfiguration;
 import org.apache.qpid.server.security.Result;
 import org.apache.qpid.server.security.access.plugins.Firewall;
 import org.apache.qpid.server.security.access.plugins.FirewallConfiguration;
@@ -73,8 +72,15 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
     }
 
     // IP address
-    private String _address= "127.0.0.1";
+    private SocketAddress _address;
 
+    @Override
+    public void setUp() throws Exception
+    {
+        super.setUp();
+        
+        _address = new InetSocketAddress("127.0.0.1", 65535);
+    }
 
     private Firewall initialisePlugin(String defaultAction, RuleInfo[] rules) throws IOException, ConfigurationException
     {
@@ -139,7 +145,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "192.168.23.23";
+        _address = new InetSocketAddress("192.168.23.23", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
     
@@ -154,7 +160,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "192.168.23.23";
+        _address = new InetSocketAddress("192.168.23.23", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
 
@@ -167,7 +173,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         Firewall plugin = initialisePlugin("deny", new RuleInfo[]{rule});
 
         // Set IP so that we're connected from the right address
-        _address = "127.0.0.1";
+        _address = new InetSocketAddress("127.0.0.1", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
 
@@ -180,7 +186,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         Firewall plugin = initialisePlugin("deny", new RuleInfo[]{rule});
 
         // Set IP so that we're connected from the right address
-        _address = "127.0.0.1";
+        _address = new InetSocketAddress("127.0.0.1", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
     
@@ -203,7 +209,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "192.168.23.23";
+        _address = new InetSocketAddress("192.168.23.23", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
     
@@ -226,7 +232,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "192.168.23.23";
+        _address = new InetSocketAddress("192.168.23.23", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
 
@@ -240,7 +246,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "192.168.23.23";
+        _address = new InetSocketAddress("192.168.23.23", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
     
@@ -254,7 +260,7 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "192.168.23.23";
+        _address = new InetSocketAddress("192.168.23.23", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
     
@@ -266,11 +272,11 @@ public class FirewallPluginTest extends InternalBrokerBaseCase
         Firewall plugin = initialisePlugin("deny", new RuleInfo[]{firstRule});
         
         // Set IP so that we're connected from the right address
-        _address = "10.0.0.1";
+        _address = new InetSocketAddress("10.0.0.1", 65535);
         assertEquals(Result.DENIED, plugin.access(ObjectType.VIRTUALHOST, _address));
         
         // Set IP so that we're connected from the right address
-        _address = "127.0.0.1";
+        _address = new InetSocketAddress("127.0.0.1", 65535);
         assertEquals(Result.ALLOWED, plugin.access(ObjectType.VIRTUALHOST, _address));
     }
 }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java b/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
index 76d1e5378f..4f3f95bd6c 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/handler/ConnectionOpenMethodHandler.java
@@ -77,7 +77,7 @@ public class ConnectionOpenMethodHandler implements StateAwareMethodListener<Con
         else
         {
             // Check virtualhost access
-            if (!virtualHost.getSecurityManager().accessVirtualhost(virtualHostName, session.getRemoteAddress().toString()))
+            if (!virtualHost.getSecurityManager().accessVirtualhost(virtualHostName, session.getRemoteAddress()))
             {
                 throw body.getConnectionException(AMQConstant.ACCESS_REFUSED, "Permission denied: '" + virtualHost.getName() + "'");
             }
diff --git a/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java b/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
index ff28d76053..240be9efe7 100755
--- a/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/security/SecurityManager.java
@@ -21,6 +21,7 @@ package org.apache.qpid.server.security;
 import static org.apache.qpid.server.security.access.ObjectType.*;
 import static org.apache.qpid.server.security.access.Operation.*;
 
+import java.net.SocketAddress;
 import java.security.Principal;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -312,7 +313,7 @@ public class SecurityManager
 		});
     }
 
-    public boolean accessVirtualhost(final String vhostname, final String remoteAddress)
+    public boolean accessVirtualhost(final String vhostname, final SocketAddress remoteAddress)
     {
         return checkAllPlugins(new AccessCheck()
         {
diff --git a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
index 38040ecfce..429e4b5976 100644
--- a/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
+++ b/java/broker/src/main/java/org/apache/qpid/server/transport/ServerConnectionDelegate.java
@@ -22,6 +22,7 @@ package org.apache.qpid.server.transport;
 
 import org.apache.qpid.transport.*;
 
+import org.apache.qpid.protocol.ProtocolEngine;
 import org.apache.qpid.server.security.SecurityManager;
 import org.apache.qpid.server.registry.IApplicationRegistry;
 import org.apache.qpid.server.virtualhost.VirtualHost;
@@ -103,7 +104,7 @@ public class ServerConnectionDelegate extends ServerDelegate
         {
             sconn.setVirtualHost(vhost);
 
-            if (!vhost.getSecurityManager().accessVirtualhost(vhostName, sconn.getConfig().getAddress()))
+            if (!vhost.getSecurityManager().accessVirtualhost(vhostName, ((ProtocolEngine) sconn.getConfig()).getRemoteAddress()))
             {
                 sconn.invoke(new ConnectionClose(ConnectionCloseCode.CONNECTION_FORCED, "Permission denied '"+vhostName+"'"));
                 sconn.setState(Connection.State.CLOSING);
diff --git a/java/systests/etc/config-systests-ServerConfigurationTest-New.xml b/java/systests/etc/config-systests-ServerConfigurationTest-New.xml
index 1ce9e7d8fa..d1dce019dd 100644
--- a/java/systests/etc/config-systests-ServerConfigurationTest-New.xml
+++ b/java/systests/etc/config-systests-ServerConfigurationTest-New.xml
@@ -24,6 +24,8 @@
     <work>${QPID_WORK}</work>
     <conf>${QPID_HOME}/etc</conf>
     <passwordDir>${conf}</passwordDir>
+    <plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>
+    <cache-directory>${QPID_WORK}/cache</cache-directory>
     <connector>
         <transport>nio</transport>
         <port>5672</port>
diff --git a/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml b/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml
index 8a685c1b0d..1de0389533 100644
--- a/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml
+++ b/java/systests/etc/config-systests-ServerConfigurationTest-Old.xml
@@ -24,6 +24,8 @@
 <work>${QPID_WORK}</work>
 <conf>${QPID_HOME}/etc</conf>
 <passwordDir>${conf}</passwordDir>
+<plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>
+<cache-directory>${QPID_WORK}/cache</cache-directory>
 <connector>
 <transport>nio</transport>
 <port>5672</port>
diff --git a/java/systests/etc/config-systests-firewall-2.xml b/java/systests/etc/config-systests-firewall-2.xml
index 276f7eac71..38ff9ddbb0 100644
--- a/java/systests/etc/config-systests-firewall-2.xml
+++ b/java/systests/etc/config-systests-firewall-2.xml
@@ -23,6 +23,8 @@
     <prefix>${QPID_HOME}</prefix>
     <work>${QPID_WORK}</work>
     <conf>${prefix}/etc</conf>
+    <plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>
+    <cache-directory>${QPID_WORK}/cache</cache-directory>
     <connector>
         <!-- To enable SSL edit the keystorePath and keystorePassword
 	     and set enabled to true. 
@@ -87,9 +89,7 @@
             <principal-database>passwordfile</principal-database>
         </jmx>
 
-        <firewall default-action="allow">
-            <rule access="deny" network="127.0.0.1"/>
-        </firewall>
+        <firewall default-action="deny"/>
     </security>
 
     <virtualhosts>${conf}/virtualhosts-systests-firewall-2.xml</virtualhosts>
diff --git a/java/systests/etc/config-systests-firewall-3.xml b/java/systests/etc/config-systests-firewall-3.xml
index 28584de02f..6c0ee7ee3c 100644
--- a/java/systests/etc/config-systests-firewall-3.xml
+++ b/java/systests/etc/config-systests-firewall-3.xml
@@ -23,6 +23,8 @@
     <prefix>${QPID_HOME}</prefix>
     <work>${QPID_WORK}</work>
     <conf>${prefix}/etc</conf>
+    <plugin-directory>${QPID_HOME}/lib/plugins</plugin-directory>
+    <cache-directory>${QPID_WORK}/cache</cache-directory>
     <connector>
         <!-- To enable SSL edit the keystorePath and keystorePassword
 	     and set enabled to true. 
diff --git a/java/systests/etc/virtualhosts-systests-firewall-2.xml b/java/systests/etc/virtualhosts-systests-firewall-2.xml
index 7990483348..20908e6eb4 100644
--- a/java/systests/etc/virtualhosts-systests-firewall-2.xml
+++ b/java/systests/etc/virtualhosts-systests-firewall-2.xml
@@ -28,9 +28,6 @@
             <store>
                 <class>org.apache.qpid.server.store.MemoryMessageStore</class>
             </store>
-            <security>
-                <firewall default-action="allow"/>
-            </security>
         </test>
     </virtualhost>
 
@@ -40,6 +37,11 @@
             <store>
                 <class>org.apache.qpid.server.store.MemoryMessageStore</class>
             </store>
+            <security>
+		        <firewall default-action="deny">
+		            <rule access="allow" network="127.0.0.1"/>
+		        </firewall>
+            </security>
         </test2>
     </virtualhost>
 </virtualhosts>
\ No newline at end of file
diff --git a/java/systests/etc/virtualhosts-systests-firewall-3.xml b/java/systests/etc/virtualhosts-systests-firewall-3.xml
index 45a2ae5ef4..90377f345f 100644
--- a/java/systests/etc/virtualhosts-systests-firewall-3.xml
+++ b/java/systests/etc/virtualhosts-systests-firewall-3.xml
@@ -38,8 +38,8 @@
                 <class>org.apache.qpid.server.store.MemoryMessageStore</class>
             </store>
             <security>
-         <firewall default-action="deny"/>
-     </security>
+	            <firewall default-action="deny"/>
+	        </security>
         </test2>
     </virtualhost>
 </virtualhosts>
diff --git a/java/systests/src/main/java/org/apache/qpid/server/security/firewall/FirewallConfigTest.java b/java/systests/src/main/java/org/apache/qpid/server/security/firewall/FirewallConfigTest.java
index fae8ee4a0a..35dd757330 100644
--- a/java/systests/src/main/java/org/apache/qpid/server/security/firewall/FirewallConfigTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/server/security/firewall/FirewallConfigTest.java
@@ -100,26 +100,26 @@ public class FirewallConfigTest extends QpidBrokerTestCase
         try 
         {
             //Try to get a connection to the 'test2' vhost
-            //This is expected to fail as it is denied at the broker level
-            conn = getConnection(new AMQConnectionURL("amqp://username:password@clientid/test2?brokerlist='" + getBroker() + "'"));
-            fail("We expected the connection to fail");
+            //This is expected to succeed as it is allowed at the vhost level
+            conn = getConnection(new AMQConnectionURL("amqp://guest:guest@clientid/test2?brokerlist='" + getBroker() + "'"));
         } 
         catch (JMSException e)
         {
-            //ignore
+            e.getLinkedException().printStackTrace();
+            fail("The connection was expected to succeed: " + e.getMessage());
         }
         
         conn = null;
         try 
         {
             //Try to get a connection to the 'test' vhost
-            //This is expected to succeed as it is allowed at the vhost level
+            //This is expected to fail as it is denied at the broker level
             conn = getConnection();
+            fail("We expected the connection to fail");
         } 
         catch (JMSException e)
         {
-            e.getLinkedException().printStackTrace();
-            fail("The connection was expected to succeed: " + e.getMessage());
+            //ignore
         }
     }
     
@@ -141,8 +141,8 @@ public class FirewallConfigTest extends QpidBrokerTestCase
         {
             //Try to get a connection to the 'test2' vhost
             //This is expected to fail as it is denied at the vhost level
-            conn = getConnection(new AMQConnectionURL(
-                    "amqp://username:password@clientid/test2?brokerlist='" + getBroker() + "'"));
+            conn = getConnection(new AMQConnectionURL("amqp://guest:guest@clientid/test2?brokerlist='" + getBroker() + "'"));
+            fail("The connection was expected to fail");
         } 
         catch (JMSException e)
         {