From b69d37dd79c204256d793896d08871d0a9966057 Mon Sep 17 00:00:00 2001
From: Simon Brandhof <simon.brandhof@sonarsource.com>
Date: Mon, 12 Oct 2015 21:59:34 +0200
Subject: [PATCH] Fix reliability of NetworkUtils#freePort()

---
 .../java/org/sonar/process/NetworkUtils.java  | 63 +++++++++++++------
 .../org/sonar/process/NetworkUtilsTest.java   | 52 ++++++++++++---
 2 files changed, 86 insertions(+), 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
index 5dd5813917fd..6e5e567ec9d0 100644
--- 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;
-    try  {
-      socket = new ServerSocket();
-      socket.setReuseAddress(true);
-      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);
+    return RANDOM_PORT_FINDER.getNextAvailablePort();
+  }
+
+  static class RandomPortFinder {
+    private static final int MAX_TRY = 10;
+    // Firefox blocks some reserved ports : http://www-archive.mozilla.org/projects/netlib/PortBanning.html
+    private static final int[] BLOCKED_PORTS = {2049, 4045, 6000};
+
+    public int getNextAvailablePort() {
+      for (int i = 0; i < MAX_TRY; i++) {
+        try {
+          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
index e5723aa99cc9..38cc3f2c6ae8 100644
--- 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() {
-    int port = NetworkUtils.freePort();
-    assertThat(port).isGreaterThan(0);
+  public void shouldGetAvailablePortWithoutLockingHost() {
+    for (int i = 0; i < 1000; i++) {
+      /*
+       * 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() {
-    int port1 = NetworkUtils.freePort();
-    int port2 = NetworkUtils.freePort();
+  public void shouldGetRandomPort() {
+    assertThat(NetworkUtils.freePort()).isNotEqualTo(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() {
-    assertThat(TestUtils.hasOnlyPrivateConstructors(NetworkUtils.class)).isTrue();
+  public void shouldBeValidPorts() {
+    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();
+  }
+
 }