fix: detect duplicate page tokens in REST catalog listing methods - #17767
fix: detect duplicate page tokens in REST catalog listing methods#17767waterWang wants to merge 2 commits into
Conversation
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessageContaining("duplicate page token"); | ||
| } | ||
| } No newline at end of file |
uros-b
left a comment
There was a problem hiding this comment.
Also, one note below regarding testing coverage
| assertThatThrownBy(() -> catalog.listViews(Namespace.of("demo"))) | ||
| .isInstanceOf(IllegalStateException.class) | ||
| .hasMessageContaining("duplicate page token"); | ||
| } |
There was a problem hiding this comment.
Coverage gap on the new guard code path. All three tests assert only the throw, driven by an always-"stuck" server that repeats one token immediately. There is no positive test proving the guard leaves a valid multi-page listing intact (distinct tokens across N pages, null terminator, all results returned), and none covering the alternating-token loop (A to B to A to B) that the full-HashSet design specifically exists to catch. A no-throw happy-path test plus an alternating-token test should be added so a future regression in the guard (for example a stray double-add of the "" sentinel) that silently truncates real listings would be caught. Live regression risk is bounded because the existing CatalogTests/RESTCatalog pagination suites exercise normal multi-page listing, which is why this is minor rather than blocking, but the strengthening belongs in this PR since it introduces the new code path.
Detect duplicate page tokens in
RESTSessionCatalogpagination to prevent infinite loops when a REST catalog server returns a repeatednext-page-token.Problem
RESTSessionCatalog.listTables(),listNamespaces(), andlistViews()all use ado-whileloop that continues as long asnextPageToken != null. If a REST catalog server returns a token that the client has already seen, the loop never terminates — each repeated response is also added to the result builder, so memory use and request volume grow unbounded.Fix
Track every page token used by each listing call with a
HashSet<String>. If the set already contains the token, throwIllegalStateExceptionwith a clear error message before the next request.Changes
RESTSessionCatalog.java: AddedseenPageTokensset and duplicate detection guard tolistTables(),listNamespaces(), andlistViews()TestDuplicatePageToken.java: New test class verifying all three methods throw on a repeated page token