Fix of-by-one error in IncrementingPortFinder (#553)

Fixes #552
kiwiproject · Oct 17, 2024 · 171b2b6 · 171b2b6
1 parent 76f03b7
commit 171b2b6
Show file tree

Hide file tree

Showing 2 changed files with 68 additions and 4 deletions.
diff --git a/src/main/java/org/kiwiproject/dropwizard/util/startup/IncrementingFreePortFinder.java b/src/main/java/org/kiwiproject/dropwizard/util/startup/IncrementingFreePortFinder.java
@@ -56,7 +56,7 @@ public ServicePorts find(AllowablePortRange portRange) {
 
         var ports = IntStream.iterate(
                         portRange.getMinPortNumber(),
-                        port -> isBelowMaxPort(portRange, port),
+                        port -> isBelowOrEqualToMaxPort(portRange, port),
                         port -> port + 1)
                 .filter(localPortChecker::isPortAvailable)
                 .limit(2)
@@ -72,7 +72,7 @@ public ServicePorts find(AllowablePortRange portRange) {
         throw new NoAvailablePortException(message);
     }
 
-    private static boolean isBelowMaxPort(AllowablePortRange portRange, int port) {
-        return port < portRange.getMaxPortNumber();
+    private static boolean isBelowOrEqualToMaxPort(AllowablePortRange portRange, int port) {
+        return port <= portRange.getMaxPortNumber();
     }
 }
diff --git a/src/test/java/org/kiwiproject/dropwizard/util/startup/IncrementingFreePortFinderTest.java b/src/test/java/org/kiwiproject/dropwizard/util/startup/IncrementingFreePortFinderTest.java
@@ -100,6 +100,70 @@ void shouldFindNextApplicationAndAdminPorts_WhenSomeAreInUse() {
         verifyNoMoreInteractions(checker);
     }
 
+    @Test
+    void shouldFindLastTwoPortsInRange_WhenAllOthersAreInUse() {
+        var checker = mock(LocalPortChecker.class);
+        var portFinder = new IncrementingFreePortFinder(checker);
+
+        when(checker.isPortAvailable(anyInt()))
+                .thenReturn(false)  // 9000
+                .thenReturn(false)  // 9001
+                .thenReturn(false)  // 9002
+                .thenReturn(false)  // 9003
+                .thenReturn(true)   // 9004
+                .thenReturn(true);  // 9005
+
+        var minPortNumber = 9_000;
+        var maxPortNumber = 9_005;
+
+        var servicePorts = portFinder.find(new AllowablePortRange(minPortNumber, maxPortNumber));
+
+        assertAll(
+                () -> assertThat(servicePorts.applicationPort()).isEqualTo(9_004),
+                () -> assertThat(servicePorts.adminPort()).isEqualTo(9_005)
+        );
+
+        verify(checker).isPortAvailable(9_000);
+        verify(checker).isPortAvailable(9_001);
+        verify(checker).isPortAvailable(9_002);
+        verify(checker).isPortAvailable(9_003);
+        verify(checker).isPortAvailable(9_004);
+        verify(checker).isPortAvailable(9_005);
+        verifyNoMoreInteractions(checker);
+    }
+
+    @Test
+    void shouldThrowNoAvailablePortException_WhenAllButLastPortAreInUse() {
+        var checker = mock(LocalPortChecker.class);
+        var portFinder = new IncrementingFreePortFinder(checker);
+
+        when(checker.isPortAvailable(anyInt()))
+                .thenReturn(false)  // 9000
+                .thenReturn(false)  // 9001
+                .thenReturn(false)  // 9002
+                .thenReturn(false)  // 9003
+                .thenReturn(false)  // 9004
+                .thenReturn(true);  // 9005
+
+        var minPortNumber = 9_000;
+        var maxPortNumber = 9_005;
+
+        var portRange = new AllowablePortRange(minPortNumber, maxPortNumber);
+
+        assertThatExceptionOfType(NoAvailablePortException.class)
+                .isThrownBy(() -> portFinder.find(portRange))
+                .withMessage("Could not find two open ports between %d and %d",
+                        minPortNumber, maxPortNumber);
+
+        verify(checker).isPortAvailable(9_000);
+        verify(checker).isPortAvailable(9_001);
+        verify(checker).isPortAvailable(9_002);
+        verify(checker).isPortAvailable(9_003);
+        verify(checker).isPortAvailable(9_004);
+        verify(checker).isPortAvailable(9_005);
+        verifyNoMoreInteractions(checker);
+    }
+
     @Test
     void shouldThrowNoAvailablePortException_WhenNoPortsCanBeFound() {
         var checker = mock(LocalPortChecker.class);
@@ -116,7 +180,7 @@ void shouldThrowNoAvailablePortException_WhenNoPortsCanBeFound() {
                 .withMessage("Could not find two open ports between %d and %d",
                         minPortNumber, maxPortNumber);
 
-        var expectedNumberOfInvocations = maxPortNumber - minPortNumber;
+        var expectedNumberOfInvocations = portRange.getNumPortsInRange();
         verify(checker, times(expectedNumberOfInvocations)).isPortAvailable(anyInt());
         verifyNoMoreInteractions(checker);
     }