You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "ayushtkn (via GitHub)" <gi...@apache.org> on 2026/09/19 13:27:12 UTC

[PR] Reject an unpaged listing that does not fit the configured maximum [polaris]

ayushtkn opened a new pull request, #5553:
URL: https://github.com/apache/polaris/pull/5553

   `LIST_PAGINATION_MAX_PAGE_SIZE` truncated a request that supplied neither `pageToken` nor `pageSize` and answered it with a continuation token. The Iceberg REST specification requires such a request to receive the complete result with a `null` `next-page-token`, so a client that does not follow continuations silently saw only the first page.
   
   Such a request is now rejected when the result does not fit. An empty `pageToken` starts a paginated listing and is still capped, so clients that page - including the Iceberg Java client, which always sends `pageToken=` and follows `next-page-token` - are unaffected. A maximum of zero or less, the default, leaves everything unchanged, and `LIST_PAGINATION_ENABLED=false` still ignores pagination parameters entirely.
   
   ## Checklist
   - [ ] ๐Ÿ›ก๏ธ Don't disclose security issues! (contact security@apache.org)
   - [ ] ๐Ÿ”— Clearly explained why the changes are needed, or linked related issues: Fixes #
   - [ ] ๐Ÿงช Added/updated tests with good coverage, or manually tested (and explained how)
   - [ ] ๐Ÿ’ก Added comments for complex logic
   - [ ] ๐Ÿงพ Updated `CHANGELOG.md` (if needed)
   - [ ] ๐Ÿ“š Updated documentation in `site/content/in-dev/unreleased` (if needed)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Reject an unpaged listing that does not fit the configured maximum [polaris]

Posted by "ayushtkn (via GitHub)" <gi...@apache.org>.
ayushtkn commented on PR #5553:
URL: https://github.com/apache/polaris/pull/5553#issuecomment-5788326752

   Had to rebase, due to a conflict in CHANGELOG, rest all stays as is


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Reject an unpaged listing that does not fit the configured maximum [polaris]

Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #5553:
URL: https://github.com/apache/polaris/pull/5553#discussion_r4053315126


##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java:
##########
@@ -91,6 +102,105 @@ protected PolarisResolutionManifest newResolutionManifest() {
   @SuppressWarnings("immutables:incompat")
   protected PolarisResolutionManifest resolutionManifest = null;
 
+  private boolean shouldDecodeToken() {
+    CatalogEntity catalogEntity = resolutionManifest.getResolvedCatalogEntity();
+    return catalogEntity == null
+        ? realmConfig().getConfig(FeatureConfiguration.LIST_PAGINATION_ENABLED)
+        : realmConfig().getConfig(FeatureConfiguration.LIST_PAGINATION_ENABLED, catalogEntity);
+  }
+
+  /** The configured page size ceiling; zero or less means unlimited. */
+  private int maxPageSize() {
+    CatalogEntity catalogEntity = resolutionManifest.getResolvedCatalogEntity();
+    return catalogEntity == null
+        ? realmConfig().getConfig(FeatureConfiguration.LIST_PAGINATION_MAX_PAGE_SIZE)
+        : realmConfig()
+            .getConfig(FeatureConfiguration.LIST_PAGINATION_MAX_PAGE_SIZE, catalogEntity);
+  }
+
+  /**
+   * Bounds the page size forwarded to a federated catalog, using the same rule as {@link
+   * PageTokenUtil#boundPageSize}. An absent page size becomes the maximum, so the remote result set
+   * is not returned whole.
+   */
+  private @Nullable Integer boundedPageSize(@Nullable Integer requestedPageSize) {
+    OptionalInt requested =
+        requestedPageSize == null ? OptionalInt.empty() : OptionalInt.of(requestedPageSize);
+    OptionalInt bounded = PageTokenUtil.boundPageSize(requested, maxPageSize());
+    return bounded.isPresent() ? bounded.getAsInt() : null;
+  }
+
+  /**
+   * The page token forwarded to a federated catalog. {@code CatalogHandlerUtils} returns the whole
+   * result set when no token is given, so when a maximum is configured the specification's initial
+   * page token is supplied instead. That bounds the response and surfaces a continuation token when
+   * the result would have overflowed, which {@link #rejectIncompleteListing} then turns into an
+   * error for a request that asked for the complete listing.
+   */
+  private @Nullable String boundedPageToken(@Nullable String pageToken) {
+    return pageToken == null && maxPageSize() > 0
+        ? CatalogHandlerUtils.INITIAL_PAGE_TOKEN
+        : pageToken;
+  }
+
+  /**
+   * A request that supplies neither {@code pageToken} nor {@code pageSize} asks for the complete
+   * listing, which the Iceberg REST specification answers with every result and a null
+   * next-page-token. When a configured maximum prevents that, fail rather than return a page that
+   * looks complete and is not. An empty {@code pageToken} starts a paginated listing, so it is
+   * capped like any other paginated request.
+   */
+  private void rejectIncompleteListing(
+      @Nullable String pageToken, @Nullable Integer pageSize, @Nullable String nextPageToken) {
+    if (pageToken == null && pageSize == null && nextPageToken != null) {
+      throw new BadRequestException(
+          "Listing does not fit the maximum page size of %s; supply a pageToken to paginate",
+          maxPageSize());

Review Comment:
   This new rejection changes the existing federated integration test `IcebergCatalogAdapterTest.testFederatedListingsWithoutAPageTokenAreBounded`: it configures a maximum of 100, creates 101 entries, and calls the listing with both `pageToken` and `pageSize` null, so this path now throws `BadRequestException` instead of returning 100 entries plus a token. Please update that test to assert the new 400 behavior and retain a separate empty-`pageToken` case for the capped response, so the module checks do not fail and both contracts remain covered.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Reject an unpaged listing that does not fit the configured maximum [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #5553:
URL: https://github.com/apache/polaris/pull/5553#discussion_r4066548367


##########
runtime/service/src/main/java/org/apache/polaris/service/catalog/common/CatalogHandler.java:
##########
@@ -91,6 +102,105 @@ protected PolarisResolutionManifest newResolutionManifest() {
   @SuppressWarnings("immutables:incompat")
   protected PolarisResolutionManifest resolutionManifest = null;
 
+  private boolean shouldDecodeToken() {
+    CatalogEntity catalogEntity = resolutionManifest.getResolvedCatalogEntity();
+    return catalogEntity == null
+        ? realmConfig().getConfig(FeatureConfiguration.LIST_PAGINATION_ENABLED)
+        : realmConfig().getConfig(FeatureConfiguration.LIST_PAGINATION_ENABLED, catalogEntity);
+  }
+
+  /** The configured page size ceiling; zero or less means unlimited. */
+  private int maxPageSize() {
+    CatalogEntity catalogEntity = resolutionManifest.getResolvedCatalogEntity();
+    return catalogEntity == null
+        ? realmConfig().getConfig(FeatureConfiguration.LIST_PAGINATION_MAX_PAGE_SIZE)
+        : realmConfig()
+            .getConfig(FeatureConfiguration.LIST_PAGINATION_MAX_PAGE_SIZE, catalogEntity);
+  }
+
+  /**
+   * Bounds the page size forwarded to a federated catalog, using the same rule as {@link
+   * PageTokenUtil#boundPageSize}. An absent page size becomes the maximum, so the remote result set
+   * is not returned whole.
+   */
+  private @Nullable Integer boundedPageSize(@Nullable Integer requestedPageSize) {
+    OptionalInt requested =
+        requestedPageSize == null ? OptionalInt.empty() : OptionalInt.of(requestedPageSize);
+    OptionalInt bounded = PageTokenUtil.boundPageSize(requested, maxPageSize());
+    return bounded.isPresent() ? bounded.getAsInt() : null;
+  }
+
+  /**
+   * The page token forwarded to a federated catalog. {@code CatalogHandlerUtils} returns the whole
+   * result set when no token is given, so when a maximum is configured the specification's initial
+   * page token is supplied instead. That bounds the response and surfaces a continuation token when
+   * the result would have overflowed, which {@link #rejectIncompleteListing} then turns into an
+   * error for a request that asked for the complete listing.
+   */
+  private @Nullable String boundedPageToken(@Nullable String pageToken) {
+    return pageToken == null && maxPageSize() > 0
+        ? CatalogHandlerUtils.INITIAL_PAGE_TOKEN
+        : pageToken;
+  }
+
+  /**
+   * A request that supplies neither {@code pageToken} nor {@code pageSize} asks for the complete
+   * listing, which the Iceberg REST specification answers with every result and a null
+   * next-page-token. When a configured maximum prevents that, fail rather than return a page that
+   * looks complete and is not. An empty {@code pageToken} starts a paginated listing, so it is
+   * capped like any other paginated request.
+   */
+  private void rejectIncompleteListing(
+      @Nullable String pageToken, @Nullable Integer pageSize, @Nullable String nextPageToken) {
+    if (pageToken == null && pageSize == null && nextPageToken != null) {
+      throw new BadRequestException(
+          "Listing does not fit the maximum page size of %s; supply a pageToken to paginate",
+          maxPageSize());
+    }
+  }
+
+  /**
+   * Runs a listing against a local catalog and checks the result. This is the only way to obtain a
+   * bounded {@link PageToken}, so a new listing endpoint cannot accidentally skip the completeness
+   * check that {@link #rejectIncompleteListing} performs.
+   */
+  protected <R> R paginatedListing(
+      @Nullable String pageToken,
+      @Nullable Integer pageSize,
+      Function<PageToken, R> listing,
+      Function<R, @Nullable String> nextPageToken) {
+    PageToken pageRequest =
+        PageToken.build(pageToken, pageSize, maxPageSize(), this::shouldDecodeToken);
+    return checkedListing(pageToken, pageSize, () -> listing.apply(pageRequest), nextPageToken);

Review Comment:
   Non-blocking: I'm fine with rejecting an unpaged request when the result exceeds the configured limit. Could we keep the existing listing calls and invoke a shared `rejectIncompleteListing(...)` helper after obtaining each result? That would preserve the check across all listing APIs with less refactoring; the extra callback layers in `paginatedListing`, `federatedListing`, and `checkedListing` make this small behavior change harder to follow, they are not strictly needed. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] Reject an unpaged listing that does not fit the configured maximum [polaris]

Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain merged PR #5553:
URL: https://github.com/apache/polaris/pull/5553


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org