Fix reliability of NetworkUtils#freePort()

SonarSource · Oct 13, 2015 · b69d37d · b69d37d
1 parent 472b20d
commit b69d37d
Show file tree

Hide file tree

Showing 2 changed files with 86 additions and 29 deletions.
diff --git a/server/sonar-process/src/main/java/org/sonar/process/NetworkUtils.java b/server/sonar-process/src/main/java/org/sonar/process/NetworkUtils.java
@@ -19,34 +19,59 @@
  */
 package org.sonar.process;
 
-import org.apache.commons.io.IOUtils;
-
 import java.io.IOException;
 import java.net.InetSocketAddress;
 import java.net.ServerSocket;
+import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.ArrayUtils;
 
-public class NetworkUtils {
+public final class NetworkUtils {
+
+  private static final RandomPortFinder RANDOM_PORT_FINDER = new RandomPortFinder();
 
   private NetworkUtils() {
-    // only static stuff
+    // only statics
   }
 
-  /**
-   * Get an unused port
-   */
   public static int freePort() {
-    ServerSocket socket = null;
+    return RANDOM_PORT_FINDER.getNextAvailablePort();
-    try  {
+  }
-      socket = new ServerSocket();
+
-      socket.setReuseAddress(true);
+  static class RandomPortFinder {
-      socket.bind(new InetSocketAddress("localhost", 0));
+    private static final int MAX_TRY = 10;
-      return socket.getLocalPort();
+    // Firefox blocks some reserved ports : http://www-archive.mozilla.org/projects/netlib/PortBanning.html
-
+    private static final int[] BLOCKED_PORTS = {2049, 4045, 6000};
-    } catch (IOException e) {
+
-      throw new IllegalStateException("Can not find a free network port", e);
+    public int getNextAvailablePort() {
-
+      for (int i = 0; i < MAX_TRY; i++) {
-    } finally {
+        try {
-      IOUtils.closeQuietly(socket);
+          int port = getRandomUnusedPort();
+          if (isValidPort(port)) {
+            return port;
+          }
+        } catch (Exception e) {
+          throw new IllegalStateException("Can't find an open network port", e);
+        }
+      }
+
+      throw new IllegalStateException("Can't find an open network port");
+    }
+
+    public int getRandomUnusedPort() throws IOException {
+      ServerSocket socket = null;
+      try {
+        socket = new ServerSocket();
+        socket.bind(new InetSocketAddress("localhost", 0));
+        return socket.getLocalPort();
+      } catch (IOException e) {
+        throw new IllegalStateException("Can not find a free network port", e);
+      } finally {
+        IOUtils.closeQuietly(socket);
+      }
+    }
+
+    public static boolean isValidPort(int port) {
+      return port > 1023 && !ArrayUtils.contains(BLOCKED_PORTS, port);
     }
   }
 }
diff --git a/server/sonar-process/src/test/java/org/sonar/process/NetworkUtilsTest.java b/server/sonar-process/src/test/java/org/sonar/process/NetworkUtilsTest.java
@@ -19,29 +19,61 @@
  */
 package org.sonar.process;
 
+import java.io.IOException;
 import org.junit.Test;
-import org.sonar.test.TestUtils;
+import org.sonar.process.NetworkUtils.RandomPortFinder;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.spy;
 
 public class NetworkUtilsTest {
 
   @Test
-  public void find_free_port() {
+  public void shouldGetAvailablePortWithoutLockingHost() {
-    int port = NetworkUtils.freePort();
+    for (int i = 0; i < 1000; i++) {
-    assertThat(port).isGreaterThan(0);
+      /*
+       * The Well Known Ports are those from 0 through 1023.
+       * DCCP Well Known ports SHOULD NOT be used without IANA registration.
+       */
+      assertThat(NetworkUtils.freePort()).isGreaterThan(1023);
+    }
   }
 
   @Test
-  public void find_multiple_free_port() {
+  public void shouldGetRandomPort() {
-    int port1 = NetworkUtils.freePort();
+    assertThat(NetworkUtils.freePort()).isNotEqualTo(NetworkUtils.freePort());
-    int port2 = NetworkUtils.freePort();
+  }
 
-    assertThat(port1).isNotSameAs(port2);
+  @Test
+  public void shouldNotBeValidPorts() {
+    assertThat(RandomPortFinder.isValidPort(0)).isFalse();// <=1023
+    assertThat(RandomPortFinder.isValidPort(50)).isFalse();// <=1023
+    assertThat(RandomPortFinder.isValidPort(1023)).isFalse();// <=1023
+    assertThat(RandomPortFinder.isValidPort(2049)).isFalse();// NFS
+    assertThat(RandomPortFinder.isValidPort(4045)).isFalse();// lockd
   }
 
   @Test
-  public void private_constructor() {
+  public void shouldBeValidPorts() {
-    assertThat(TestUtils.hasOnlyPrivateConstructors(NetworkUtils.class)).isTrue();
+    assertThat(RandomPortFinder.isValidPort(1059)).isTrue();
+  }
+
+  @Test(expected = IllegalStateException.class)
+  public void shouldFailWhenNoValidPortIsAvailable() throws IOException {
+    RandomPortFinder randomPortFinder = spy(new RandomPortFinder());
+    doReturn(0).when(randomPortFinder).getRandomUnusedPort();
+
+    randomPortFinder.getNextAvailablePort();
   }
+
+  @Test(expected = IllegalStateException.class)
+  public void shouldFailWhenItsNotPossibleToOpenASocket() throws IOException {
+    RandomPortFinder randomPortFinder = spy(new RandomPortFinder());
+    doThrow(new IOException("Not possible")).when(randomPortFinder).getRandomUnusedPort();
+
+    randomPortFinder.getNextAvailablePort();
+  }
+
 }