This is an automated email from the ASF dual-hosted git repository.
yuqi1129 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 9e55c98204 [#13185] fix(lance): prevent pagination index overflow
(#13188)
9e55c98204 is described below
commit 9e55c98204a8d01f1117145edea0a40570fff112
Author: StormSpirit <[email protected]>
AuthorDate: Thu Sep 17 08:52:28 2026 +0800
[#13185] fix(lance): prevent pagination index overflow (#13188)
### What changes were proposed in this pull request?
Use wider arithmetic for the Lance REST pagination end index before
clamping it to the list size, and add focused unit coverage for normal,
boundary, validation, and `Integer.MAX_VALUE` pagination cases.
### Why are the changes needed?
`PageUtil.splitPage` currently evaluates `startIndex + pageSize` as an
`int`. A valid continuation token combined with `Integer.MAX_VALUE` can
wrap the end index to a negative value and fail the list operation. The
change preserves the existing page-token format, limit validation,
sorting, and next-token behavior.
Fix: #13185
### Does this PR introduce _any_ user-facing change?
It does not change public APIs or REST routes. Valid pagination requests
that previously failed because of integer overflow now return the
expected page; ordinary pagination behavior remains unchanged.
### How was this patch tested?
- `./gradlew :lance:lance-common:test --tests
'org.apache.gravitino.lance.common.ops.gravitino.TestPageUtil'
-PskipITs` — passed with 14 tests, 0 skipped, 0 failures, and 0 errors.
- `./gradlew :lance:lance-rest-server:test -PskipITs` — passed; this
module holds the callers of the pagination helper, and its
`TestGravitinoLanceNamespaceListFiltering` exercises namespace and table
listing with `limit=1` and `limit=10` through them (integration tests
are excluded by `-PskipITs`).
- `./gradlew :lance:lance-common:spotlessCheck`, `./gradlew rat`, and
`git diff --check` — passed.
- A pre-fix Docker REST reproduction against the Gravitino 1.3.0
deployment confirmed the overflow response and was cleaned up; the unit
tests above ran on the current fix worktree based on main, and no
post-fix Docker run was performed because this change is confined to a
deterministic in-memory pagination helper.
Signed-off-by: jiangxt2 <[email protected]>
---
.../lance/common/ops/gravitino/PageUtil.java | 2 +-
.../lance/common/ops/gravitino/TestPageUtil.java | 120 +++++++++++++++++++++
2 files changed, 121 insertions(+), 1 deletion(-)
diff --git
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/PageUtil.java
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/PageUtil.java
index dab04e8987..8bfafa57db 100644
---
a/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/PageUtil.java
+++
b/lance/lance-common/src/main/java/org/apache/gravitino/lance/common/ops/gravitino/PageUtil.java
@@ -61,7 +61,7 @@ class PageUtil {
pageToken,
sortedItems.size());
- int endIndex = Math.min(startIndex + pageSize, sortedItems.size());
+ int endIndex = (int) Math.min((long) startIndex + pageSize,
sortedItems.size());
List<String> pageItems =
startIndex == endIndex
? Collections.emptyList()
diff --git
a/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestPageUtil.java
b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestPageUtil.java
new file mode 100644
index 0000000000..2f5a98d122
--- /dev/null
+++
b/lance/lance-common/src/test/java/org/apache/gravitino/lance/common/ops/gravitino/TestPageUtil.java
@@ -0,0 +1,120 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.lance.common.ops.gravitino;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+import java.util.List;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+
+class TestPageUtil {
+
+ private static final List<String> ITEMS = List.of("a", "b", "c");
+
+ @Test
+ void testNormalizePageSizeUsesDefaultForNullLimit() {
+ assertEquals(1000, PageUtil.normalizePageSize(null));
+ }
+
+ @Test
+ void testNormalizePageSizePreservesPositiveLimit() {
+ assertEquals(25, PageUtil.normalizePageSize(25));
+ }
+
+ @ParameterizedTest
+ @ValueSource(ints = {0, -1, Integer.MIN_VALUE})
+ void testNormalizePageSizeRejectsNonPositiveLimit(int limit) {
+ assertThrows(IllegalArgumentException.class, () ->
PageUtil.normalizePageSize(limit));
+ }
+
+ @Test
+ void testSplitPageReturnsFirstPageAndNextToken() {
+ PageUtil.Page page = PageUtil.splitPage(ITEMS, null, 2);
+
+ assertEquals(List.of("a", "b"), page.items());
+ assertEquals("2", page.nextPageToken());
+ }
+
+ @Test
+ void testSplitPageReturnsLastPageWithoutNextToken() {
+ PageUtil.Page page = PageUtil.splitPage(ITEMS, "2", 2);
+
+ assertEquals(List.of("c"), page.items());
+ assertNull(page.nextPageToken());
+ }
+
+ @Test
+ void testSplitPageReturnsEmptyPageForEmptyItems() {
+ PageUtil.Page page = PageUtil.splitPage(List.of(), null, 2);
+
+ assertEquals(List.of(), page.items());
+ assertNull(page.nextPageToken());
+ }
+
+ @Test
+ void testSplitPageReturnsEmptyPageForTokenAtEnd() {
+ PageUtil.Page page = PageUtil.splitPage(ITEMS,
String.valueOf(ITEMS.size()), 2);
+
+ assertEquals(List.of(), page.items());
+ assertNull(page.nextPageToken());
+ }
+
+ @Test
+ void testSplitPageAvoidsOverflowForMaximumPageSize() {
+ List<String> items = List.of("a", "b");
+ PageUtil.Page firstPage = PageUtil.splitPage(items, null, 1);
+
+ assertEquals(List.of("a"), firstPage.items());
+ assertEquals("1", firstPage.nextPageToken());
+
+ PageUtil.Page secondPage =
+ PageUtil.splitPage(items, firstPage.nextPageToken(),
Integer.MAX_VALUE);
+
+ assertEquals(List.of("b"), secondPage.items());
+ assertNull(secondPage.nextPageToken());
+ }
+
+ @Test
+ void testSplitPageReturnsEmptyPageForEndTokenWithMaximumPageSize() {
+ List<String> items = List.of("a", "b");
+ PageUtil.Page page = PageUtil.splitPage(items,
String.valueOf(items.size()), Integer.MAX_VALUE);
+
+ assertEquals(List.of(), page.items());
+ assertNull(page.nextPageToken());
+ }
+
+ @Test
+ void testSplitPageRejectsInvalidPageToken() {
+ assertThrows(IllegalArgumentException.class, () ->
PageUtil.splitPage(ITEMS, "invalid", 2));
+ }
+
+ @Test
+ void testSplitPageRejectsNegativePageToken() {
+ assertThrows(IllegalArgumentException.class, () ->
PageUtil.splitPage(ITEMS, "-1", 2));
+ }
+
+ @Test
+ void testSplitPageRejectsOutOfRangePageToken() {
+ assertThrows(IllegalArgumentException.class, () ->
PageUtil.splitPage(ITEMS, "4", 2));
+ }
+}
