From 171b2b6701261b350c48248d946b6de3cc6a35bf Mon Sep 17 00:00:00 2001
From: Scott Leberknight <174812+sleberknight@users.noreply.github.com>
Date: Thu, 17 Oct 2024 13:38:25 -0400
Subject: [PATCH] Fix of-by-one error in IncrementingPortFinder (#553)

Fixes #552
---
 .../startup/IncrementingFreePortFinder.java   |  6 +-
 .../IncrementingFreePortFinderTest.java       | 66 ++++++++++++++++++-
 2 files changed, 68 insertions(+), 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
index 2b050abe..84fc4b15 100644
--- 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
index e00fc220..93abc9aa 100644
--- 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);
     }