bugfix(pagination): prevent integer overflow and clamp page/size safely

Author: bsayli

customer-service/src/main/java/io/github/bsayli/customerservice/common/api/response/Page.java

@@ -18,10 +18,17 @@ public record Page<T>(
     boolean hasPrev) {
 
   public static <T> Page<T> of(List<T> content, int page, int size, long totalElements) {
-    List<T> safeContent = content == null ? List.of() : List.copyOf(content);
-    int totalPages = (int) Math.ceil((double) totalElements / size);
-    boolean hasNext = page + 1 < totalPages;
-    boolean hasPrev = page > 0;
-    return new Page<>(safeContent, page, size, totalElements, totalPages, hasNext, hasPrev);
+    List<T> safeContent = (content == null) ? List.of() : List.copyOf(content);
+
+    int p = Math.clamp(page, 0, Integer.MAX_VALUE);
+    int s = Math.clamp(size, 1, Integer.MAX_VALUE);
+
+    long totalPagesL = (totalElements <= 0L) ? 0L : ((totalElements + s - 1L) / s);
+    int totalPages = (totalPagesL > Integer.MAX_VALUE) ? Integer.MAX_VALUE : (int) totalPagesL;
+
+    boolean hasNext = p < totalPages - 1;
+    boolean hasPrev = p > 0;
+
+    return new Page<>(safeContent, p, s, totalElements, totalPages, hasNext, hasPrev);
   }
 }

customer-service/src/main/java/io/github/bsayli/customerservice/service/impl/CustomerServiceImpl.java

@@ -20,6 +20,7 @@
 @Service
 public class CustomerServiceImpl implements CustomerService {
 
+  private static final int MAX_PAGE_SIZE = 10;
   private final AtomicInteger idSeq = new AtomicInteger(0);
   private final NavigableMap<Integer, CustomerDto> store = new ConcurrentSkipListMap<>();
 
@@ -115,10 +116,17 @@ private Comparator<CustomerDto> buildComparator(SortField sortBy, SortDirection
   }
 
   private Page<CustomerDto> paginate(List<CustomerDto> items, int page, int size) {
+    int p = Math.clamp(page, 0, Integer.MAX_VALUE);
+    int s = Math.clamp(size, 1, MAX_PAGE_SIZE);
+
     long total = items.size();
-    int from = Math.min(page * size, items.size());
-    int to = Math.min(from + size, items.size());
+    long fromL = Math.min((long) p * s, total);
+    long toL = Math.min(fromL + s, total);
+
+    int from = (int) fromL;
+    int to = (int) toL;
+
     var slice = items.subList(from, to);
-    return Page.of(slice, page, size, total);
+    return Page.of(slice, p, s, total);
   }
 }

customer-service/src/test/java/io/github/bsayli/customerservice/service/impl/CustomerServiceImplTest.java

@@ -28,19 +28,35 @@ void setUp() {
   }
 
   @Test
-  @DisplayName("Initial seed should contain 17 customers")
-  void initialSeed_shouldContainSeventeen() {
-    Page<CustomerDto> page =
+  @DisplayName("Initial seed pagination: total=17, page0 size=10, page1 size=7")
+  void initialSeed_paginationShouldBeCorrect() {
+    Page<CustomerDto> p0 =
         service.getCustomers(
             new CustomerSearchCriteria(null, null),
             0,
             100,
             SortField.CUSTOMER_ID,
             SortDirection.ASC);
 
-    assertEquals(17, page.totalElements());
-    assertEquals(17, page.content().size());
-    assertTrue(page.content().stream().allMatch(c -> c.customerId() != null && c.customerId() > 0));
+    assertEquals(17, p0.totalElements());
+    assertEquals(2, p0.totalPages());
+    assertEquals(10, p0.content().size());
+    assertTrue(p0.hasNext());
+    assertFalse(p0.hasPrev());
+
+    Page<CustomerDto> p1 =
+        service.getCustomers(
+            new CustomerSearchCriteria(null, null),
+            1,
+            100,
+            SortField.CUSTOMER_ID,
+            SortDirection.ASC);
+
+    assertEquals(17, p1.totalElements());
+    assertEquals(2, p1.totalPages());
+    assertEquals(7, p1.content().size());
+    assertFalse(p1.hasNext());
+    assertTrue(p1.hasPrev());
   }
 
   @Test
@@ -54,26 +70,27 @@ void createCustomer_shouldAssignIdAndStore() {
     assertEquals("Jane Doe", created.name());
     assertEquals("jane.doe@example.com", created.email());
 
-    Page<CustomerDto> page =
+    Page<CustomerDto> anyPage =
         service.getCustomers(
             new CustomerSearchCriteria(null, null),
             0,
-            200,
+            10,
             SortField.CUSTOMER_ID,
             SortDirection.ASC);
 
-    assertEquals(18, page.totalElements());
-    assertTrue(page.content().stream().anyMatch(c -> c.customerId().equals(created.customerId())));
+    assertEquals(18, anyPage.totalElements());
+    CustomerDto fetched = service.getCustomer(created.customerId());
+    assertEquals(created, fetched);
   }
 
   @Test
   @DisplayName("getCustomer should return existing customer")
   void getCustomer_shouldReturn() {
-    Page<CustomerDto> page =
+    Page<CustomerDto> p0 =
         service.getCustomers(
             new CustomerSearchCriteria(null, null), 0, 1, SortField.CUSTOMER_ID, SortDirection.ASC);
 
-    CustomerDto any = page.content().getFirst();
+    CustomerDto any = p0.content().getFirst();
     CustomerDto found = service.getCustomer(any.customerId());
     assertEquals(any, found);
   }
@@ -106,7 +123,7 @@ void updateCustomer_shouldThrowWhenMissing() {
   }
 
   @Test
-  @DisplayName("deleteCustomer should remove the record")
+  @DisplayName("deleteCustomer should remove the record and decrease total")
   void deleteCustomer_shouldRemove() {
     CustomerDto base =
         service.createCustomer(new CustomerCreateRequest("Mark Lee", "mark.lee@example.com"));
@@ -115,22 +132,21 @@ void deleteCustomer_shouldRemove() {
         service.getCustomers(
             new CustomerSearchCriteria(null, null),
             0,
-            200,
+            10,
             SortField.CUSTOMER_ID,
             SortDirection.ASC);
-    long sizeBefore = before.totalElements();
+    long totalBefore = before.totalElements();
 
     service.deleteCustomer(base.customerId());
 
     Page<CustomerDto> after =
         service.getCustomers(
             new CustomerSearchCriteria(null, null),
             0,
-            200,
+            10,
             SortField.CUSTOMER_ID,
             SortDirection.ASC);
-
-    assertEquals(sizeBefore - 1, after.totalElements());
-    assertTrue(after.content().stream().noneMatch(c -> c.customerId().equals(base.customerId())));
+    assertEquals(totalBefore - 1, after.totalElements());
+    assertThrows(NoSuchElementException.class, () -> service.getCustomer(base.customerId()));
   }
 }