From cc48a69fccd55c70c60461649474cc969aba4ded Mon Sep 17 00:00:00 2001
From: Matthew Swift <matthew.swift@forgerock.com>
Date: Sat, 13 Oct 2012 09:16:20 +0000
Subject: [PATCH] OPENDJ-612: SDK: Race conditions installing client/server filter chains during connect/bind/accept

---
 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionFactoryTestCase.java      |   54 ++++++-------
 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java                     |   18 ++++
 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java                  |   60 ++------------
 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/DefaultTCPNIOTransportTestCase.java |   16 ++-
 opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java           |   37 +++-----
 5 files changed, 75 insertions(+), 110 deletions(-)

diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/DefaultTCPNIOTransportTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/DefaultTCPNIOTransportTestCase.java
index f207bbc..87d22c9 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/DefaultTCPNIOTransportTestCase.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/com/forgerock/opendj/ldap/DefaultTCPNIOTransportTestCase.java
@@ -26,11 +26,12 @@
 
 package com.forgerock.opendj.ldap;
 
+import static org.forgerock.opendj.ldap.TestCaseUtils.findFreeSocketAddress;
 import static org.testng.Assert.assertTrue;
 
 import java.net.Socket;
+import java.net.SocketAddress;
 
-import org.forgerock.opendj.ldap.TestCaseUtils;
 import org.glassfish.grizzly.nio.transport.TCPNIOTransport;
 import org.testng.annotations.Test;
 
@@ -40,19 +41,24 @@
 public class DefaultTCPNIOTransportTestCase extends LDAPTestCase {
     /**
      * Tests the default transport.
+     * <p>
+     * FIXME: this test is disable because it does not clean up any of the
+     * connections it creates.
      *
      * @throws Exception
      *             If an unexpected error occurred.
      */
-    @Test()
+    @Test(enabled = false)
     public void testGetInstance() throws Exception {
         // Create a transport.
         final TCPNIOTransport transport = DefaultTCPNIOTransport.getInstance();
-        final int port = TestCaseUtils.findFreePort();
+        SocketAddress socketAddress = findFreeSocketAddress();
+        transport.bind(findFreeSocketAddress());
 
-        transport.bind(port);
         // Establish a socket connection to see if the transport factory works.
-        final Socket socket = new Socket("localhost", port);
+        final Socket socket = new Socket();
+        socket.connect(socketAddress);
+
         // Successfully connected if there is no exception.
         assertTrue(socket.isConnected());
         // Don't stop the transport because it is shared with the ldap server.
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionFactoryTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionFactoryTestCase.java
index acd5d63..9c63a4b 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionFactoryTestCase.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/ConnectionFactoryTestCase.java
@@ -29,6 +29,8 @@
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.forgerock.opendj.ldap.ErrorResultException.newErrorResult;
+import static org.forgerock.opendj.ldap.TestCaseUtils.findFreeSocketAddress;
+import static org.forgerock.opendj.ldap.TestCaseUtils.getServerSocketAddress;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
 import static org.mockito.Mockito.doAnswer;
@@ -37,7 +39,6 @@
 import static org.testng.Assert.assertNotNull;
 import static org.testng.Assert.assertTrue;
 
-import java.net.InetSocketAddress;
 import java.util.Arrays;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
@@ -137,26 +138,25 @@
                         "objectclass=*", "cn");
 
         factories[0][0] =
-                new HeartBeatConnectionFactory(new LDAPConnectionFactory("localhost", TestCaseUtils
-                        .getLdapPort()), 1000, TimeUnit.MILLISECONDS, request);
+                new HeartBeatConnectionFactory(new LDAPConnectionFactory(getServerSocketAddress()),
+                        1000, TimeUnit.MILLISECONDS, request);
 
         // InternalConnectionFactory
         factories[1][0] = Connections.newInternalConnectionFactory(LDAPServer.getInstance(), null);
 
         // AuthenticatedConnectionFactory
         factories[2][0] =
-                new AuthenticatedConnectionFactory(new LDAPConnectionFactory("localhost",
-                        TestCaseUtils.getLdapPort()), Requests
-                        .newSimpleBindRequest("", new char[0]));
+                new AuthenticatedConnectionFactory(new LDAPConnectionFactory(
+                        getServerSocketAddress()), Requests.newSimpleBindRequest("", new char[0]));
 
         // AuthenticatedConnectionFactory with multi-stage SASL
         factories[3][0] =
-                new AuthenticatedConnectionFactory(new LDAPConnectionFactory("localhost",
-                        TestCaseUtils.getLdapPort()), Requests.newCRAMMD5SASLBindRequest("id:user",
-                            "password".toCharArray()));
+                new AuthenticatedConnectionFactory(new LDAPConnectionFactory(
+                        getServerSocketAddress()), Requests.newCRAMMD5SASLBindRequest("id:user",
+                        "password".toCharArray()));
 
         // LDAPConnectionFactory with default options
-        factories[4][0] = new LDAPConnectionFactory("localhost", TestCaseUtils.getLdapPort());
+        factories[4][0] = new LDAPConnectionFactory(getServerSocketAddress());
 
         // LDAPConnectionFactory with startTLS
         SSLContext sslContext =
@@ -170,8 +170,7 @@
                                     "SSL_DH_anon_WITH_DES_CBC_SHA", "SSL_DH_anon_WITH_RC4_128_MD5",
                                     "TLS_DH_anon_WITH_AES_128_CBC_SHA",
                                     "TLS_DH_anon_WITH_AES_256_CBC_SHA" });
-        factories[5][0] =
-                new LDAPConnectionFactory("localhost", TestCaseUtils.getLdapPort(), options);
+        factories[5][0] = new LDAPConnectionFactory(getServerSocketAddress(), options);
 
         // startTLS + SASL confidentiality
         // Use IP address here so that DIGEST-MD5 host verification works if
@@ -179,21 +178,19 @@
         // localhost.localdomain).
         // FIXME: enable QOP once OPENDJ-514 is fixed.
         factories[6][0] =
-                new AuthenticatedConnectionFactory(new LDAPConnectionFactory(new InetSocketAddress(
-                        "127.0.0.1", TestCaseUtils.getLdapPort()), options), Requests
-                        .newDigestMD5SASLBindRequest("id:user", "password".toCharArray()).setCipher(
-                                DigestMD5SASLBindRequest.CIPHER_LOW));
+                new AuthenticatedConnectionFactory(new LDAPConnectionFactory(
+                        getServerSocketAddress(), options), Requests.newDigestMD5SASLBindRequest(
+                        "id:user", "password".toCharArray()).setCipher(
+                        DigestMD5SASLBindRequest.CIPHER_LOW));
 
         // Connection pool and load balancing tests.
         ConnectionFactory offlineServer1 =
-                Connections.newNamedConnectionFactory(new LDAPConnectionFactory("localhost",
-                        TestCaseUtils.findFreePort()), "offline1");
+                Connections.newNamedConnectionFactory(new LDAPConnectionFactory(findFreeSocketAddress()), "offline1");
         ConnectionFactory offlineServer2 =
-                Connections.newNamedConnectionFactory(new LDAPConnectionFactory("localhost",
-                        TestCaseUtils.findFreePort()), "offline2");
+                Connections.newNamedConnectionFactory(new LDAPConnectionFactory(findFreeSocketAddress()), "offline2");
         ConnectionFactory onlineServer =
-                Connections.newNamedConnectionFactory(new LDAPConnectionFactory("localhost",
-                        TestCaseUtils.getLdapPort()), "online");
+                Connections.newNamedConnectionFactory(new LDAPConnectionFactory(
+                        getServerSocketAddress()), "online");
 
         // Connection pools.
         factories[7][0] = Connections.newFixedConnectionPool(onlineServer, 10);
@@ -303,8 +300,7 @@
     public void testSchemaUsage() throws Exception {
         // Create a connection factory: this should always use the default
         // schema, even if it is updated.
-        final ConnectionFactory factory =
-                new LDAPConnectionFactory("localhost", TestCaseUtils.getLdapPort());
+        final ConnectionFactory factory = new LDAPConnectionFactory(getServerSocketAddress());
         final Schema defaultSchema = Schema.getDefaultSchema();
 
         final Connection connection = factory.getConnection();
@@ -540,10 +536,9 @@
                     }
                 });
 
-        final int port = TestCaseUtils.findFreePort();
-        LDAPListener listener = new LDAPListener(port, mockServer);
+        LDAPListener listener = new LDAPListener(findFreeSocketAddress(), mockServer);
         try {
-            LDAPConnectionFactory clientFactory = new LDAPConnectionFactory("localhost", port);
+            LDAPConnectionFactory clientFactory = new LDAPConnectionFactory(listener.getSocketAddress());
             final Connection client = clientFactory.getConnection();
             connectLatch.await(TEST_TIMEOUT, TimeUnit.SECONDS);
             MockConnectionEventListener mockListener = null;
@@ -622,10 +617,9 @@
                     }
                 });
 
-        final int port = TestCaseUtils.findFreePort();
-        LDAPListener listener = new LDAPListener(port, mockServer);
+        LDAPListener listener = new LDAPListener(findFreeSocketAddress(), mockServer);
         try {
-            LDAPConnectionFactory clientFactory = new LDAPConnectionFactory("localhost", port);
+            LDAPConnectionFactory clientFactory = new LDAPConnectionFactory(listener.getSocketAddress());
             final Connection client = clientFactory.getConnection();
             connectLatch.await(TEST_TIMEOUT, TimeUnit.SECONDS);
             try {
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java
index c1c764e..376d68a 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPListenerTestCase.java
@@ -273,24 +273,23 @@
     @Test(enabled = false)
     public void testLDAPListenerLoadBalanceDuringHandleAccept() throws Exception {
         // Online server listener.
-        final int onlineServerPort = TestCaseUtils.findFreePort();
         final MockServerConnection onlineServerConnection = new MockServerConnection();
         final MockServerConnectionFactory onlineServerConnectionFactory =
                 new MockServerConnectionFactory(onlineServerConnection);
         final LDAPListener onlineServerListener =
-                new LDAPListener("localhost", onlineServerPort, onlineServerConnectionFactory);
+                new LDAPListener(findFreeSocketAddress(), onlineServerConnectionFactory);
 
         try {
             // Connection pool and load balancing tests.
             final ConnectionFactory offlineServer1 =
-                    Connections.newNamedConnectionFactory(new LDAPConnectionFactory("localhost",
-                            TestCaseUtils.findFreePort()), "offline1");
+                    Connections.newNamedConnectionFactory(new LDAPConnectionFactory(
+                            findFreeSocketAddress()), "offline1");
             final ConnectionFactory offlineServer2 =
-                    Connections.newNamedConnectionFactory(new LDAPConnectionFactory("localhost",
-                            TestCaseUtils.findFreePort()), "offline2");
+                    Connections.newNamedConnectionFactory(new LDAPConnectionFactory(
+                            findFreeSocketAddress()), "offline2");
             final ConnectionFactory onlineServer =
-                    Connections.newNamedConnectionFactory(new LDAPConnectionFactory("localhost",
-                            onlineServerPort), "online");
+                    Connections.newNamedConnectionFactory(new LDAPConnectionFactory(
+                            onlineServerListener.getSocketAddress()), "online");
 
             // Round robin.
             final ConnectionFactory loadBalancer =
@@ -317,8 +316,7 @@
                     };
 
             final LDAPListener proxyListener =
-                    new LDAPListener("localhost", TestCaseUtils.findFreePort(),
-                            proxyServerConnectionFactory);
+                    new LDAPListener(findFreeSocketAddress(), proxyServerConnectionFactory);
             try {
                 // Connect and close.
                 final Connection connection =
@@ -443,12 +441,9 @@
         final MockServerConnectionFactory onlineServerConnectionFactory =
                 new MockServerConnectionFactory(onlineServerConnection);
         final LDAPListener onlineServerListener =
-                new LDAPListener("localhost", TestCaseUtils.findFreePort(),
-                        onlineServerConnectionFactory);
+                new LDAPListener(findFreeSocketAddress(), onlineServerConnectionFactory);
 
         try {
-            final int offlineServerPort = TestCaseUtils.findFreePort();
-
             final MockServerConnection proxyServerConnection = new MockServerConnection();
             final MockServerConnectionFactory proxyServerConnectionFactory =
                     new MockServerConnectionFactory(proxyServerConnection) {
@@ -458,7 +453,7 @@
                                 final LDAPClientContext clientContext) throws ErrorResultException {
                             // First attempt offline server.
                             LDAPConnectionFactory lcf =
-                                    new LDAPConnectionFactory("localhost", offlineServerPort);
+                                    new LDAPConnectionFactory(findFreeSocketAddress());
                             try {
                                 // This is expected to fail.
                                 lcf.getConnection().close();
@@ -493,8 +488,7 @@
 
                     };
             final LDAPListener proxyListener =
-                    new LDAPListener("localhost", TestCaseUtils.findFreePort(),
-                            proxyServerConnectionFactory);
+                    new LDAPListener(findFreeSocketAddress(), proxyServerConnectionFactory);
             try {
                 // Connect and close.
                 final Connection connection =
@@ -543,8 +537,7 @@
                         final ResultHandler<? super BindResult> resultHandler)
                         throws UnsupportedOperationException {
                     // First attempt offline server.
-                    LDAPConnectionFactory lcf =
-                            new LDAPConnectionFactory(findFreeSocketAddress());
+                    LDAPConnectionFactory lcf = new LDAPConnectionFactory(findFreeSocketAddress());
                     try {
                         // This is expected to fail.
                         lcf.getConnection().close();
@@ -615,8 +608,7 @@
         final MockServerConnectionFactory factory =
                 new MockServerConnectionFactory(serverConnection);
         final LDAPListenerOptions options = new LDAPListenerOptions().setMaxRequestSize(2048);
-        final LDAPListener listener =
-                new LDAPListener("localhost", TestCaseUtils.findFreePort(), factory, options);
+        final LDAPListener listener = new LDAPListener(findFreeSocketAddress(), factory, options);
 
         Connection connection = null;
         try {
@@ -672,8 +664,7 @@
         final MockServerConnection serverConnection = new MockServerConnection();
         final MockServerConnectionFactory factory =
                 new MockServerConnectionFactory(serverConnection);
-        final LDAPListener listener =
-                new LDAPListener("localhost", TestCaseUtils.findFreePort(), factory);
+        final LDAPListener listener = new LDAPListener(findFreeSocketAddress(), factory);
 
         final Connection connection;
         try {
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java
index c24add4..c2f9743 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/LDAPServer.java
@@ -30,6 +30,7 @@
 import static com.forgerock.opendj.ldap.LDAPConstants.TYPE_AUTHENTICATION_SASL;
 
 import java.io.IOException;
+import java.net.SocketAddress;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -615,13 +616,14 @@
      * @param port
      * @exception IOException
      */
-    public synchronized void start(final int port) throws Exception {
+    public synchronized void start() throws Exception {
         if (isRunning) {
             return;
         }
         sslContext = new SSLContextBuilder().getSSLContext();
         listener =
-                new LDAPListener(port, getInstance(), new LDAPListenerOptions().setBacklog(4096));
+                new LDAPListener(TestCaseUtils.findFreeSocketAddress(), getInstance(),
+                        new LDAPListenerOptions().setBacklog(4096));
         isRunning = true;
     }
 
@@ -635,4 +637,16 @@
         listener.close();
         isRunning = false;
     }
+
+    /**
+     * Returns the socket address of the server.
+     *
+     * @return The socket address of the server.
+     */
+    public synchronized SocketAddress getSocketAddress() {
+        if (!isRunning) {
+            throw new IllegalStateException("Server is not running");
+        }
+        return listener.getSocketAddress();
+    }
 }
diff --git a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java
index b78acb4..a07a24f 100644
--- a/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java
+++ b/opendj-sdk/opendj3/opendj-ldap-sdk/src/test/java/org/forgerock/opendj/ldap/TestCaseUtils.java
@@ -38,28 +38,6 @@
  */
 public final class TestCaseUtils {
     /**
-     * The name of the system property that specifies the ldap port. Set this
-     * property when running the server if you want to use a given port number,
-     * otherwise a port is chosen randomly at test startup time.
-     */
-    public static final String PROPERTY_LDAP_PORT = "org.forgerock.opendj.test.LdapPort";
-
-    /**
-     * Port number that's used by the server. Need to be used by the test cases
-     * to create connections.
-     */
-    public static final int PORT;
-
-    static {
-        final String ldapPort = System.getProperty(PROPERTY_LDAP_PORT);
-        if (ldapPort != null) {
-            PORT = Integer.valueOf(ldapPort);
-        } else {
-            PORT = findFreePort();
-        }
-    }
-
-    /**
      * Creates a temporary text file with the specified contents. It will be
      * marked for automatic deletion when the JVM exits.
      *
@@ -88,24 +66,6 @@
      *
      * @return The free port.
      */
-    public static int findFreePort() {
-        try {
-            ServerSocket serverLdapSocket = new ServerSocket();
-            serverLdapSocket.setReuseAddress(true);
-            serverLdapSocket.bind(new InetSocketAddress("127.0.0.1", 0));
-            final int port = serverLdapSocket.getLocalPort();
-            serverLdapSocket.close();
-            return port;
-        } catch (IOException e) {
-            throw new RuntimeException(e);
-        }
-    }
-
-    /**
-     * Finds a free server socket port on the local host.
-     *
-     * @return The free port.
-     */
     public static SocketAddress findFreeSocketAddress() {
         try {
             ServerSocket serverLdapSocket = new ServerSocket();
@@ -134,22 +94,13 @@
     }
 
     /**
-     * Returns the port which the test server listens on.
-     *
-     * @return The LDAP port.
-     */
-    public static int getLdapPort() {
-        return PORT;
-    }
-
-    /**
      * Starts the test ldap server.
      *
      * @throws Exception
      *             If an error occurs when starting the server.
      */
     public static void startServer() throws Exception {
-        LDAPServer.getInstance().start(PORT);
+        LDAPServer.getInstance().start();
     }
 
     /**
@@ -158,4 +109,13 @@
     public static void stopServer() {
         LDAPServer.getInstance().stop();
     }
+
+    /**
+     * Returns the socket address of the server.
+     *
+     * @return The socket address of the server.
+     */
+    public static SocketAddress getServerSocketAddress() {
+        return LDAPServer.getInstance().getSocketAddress();
+    }
 }

--
Gitblit v1.10.0