fix: return 400 instead of 500 for malformed pageToken on list endpoints by youngrae81 · Pull Request #5466 · apache/polaris

youngrae81 · 2026-09-08T10:02:56Z

PageTokenUtil.decodePageRequest decoded the client-supplied pageToken (Base64 + SMILE) without error handling. A token that is valid Base64 but not a serialized page token (garbage, truncated, or produced by an incompatible Polaris version) escaped as a Jackson 3 JacksonException or, for an unknown token type id, as IllegalStateException. Neither is covered by the REST exception mappers, so client input turned into a 500 with an ERROR-level stack trace.

Wrap the decoding step and rethrow any failure as
IllegalArgumentException("Invalid page token"), which the existing IcebergExceptionMapper maps to 400. The original exception is kept as the cause for debug logging.

Adds a parameterized unit test covering the malformed-token shapes and an integration test asserting the HTTP 400 on listNamespaces.

Fixes #5465

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)

jbonofre · 2026-09-08T12:32:24Z

+    try {
+      var bytes = Base64.getUrlDecoder().decode(requestedPageToken);
+      return SMILE_MAPPER.readValue(bytes, PageToken.class);
+    } catch (IllegalArgumentException | IllegalStateException | JacksonException e) {


I believe this catch doesn't get every exception a corrupted SMILE payload can throw.

For instance, fuzzing a valid token throws ArrayIndexOutOfBoundsException, which isn't caught here and falls through to IcebergExceptionMapper's default 500.

I suggest widening to catch (RuntimeException e) instead of enumerating types.

Good catch, thanks. Fuzzing a valid token reproduced it: 4 of 60k random corruptions escaped as ArrayIndexOutOfBoundsException. Widened to RuntimeException in 04ed32a and added the fuzz-found token as a fixed regression case.

jbonofre · 2026-09-08T12:34:02Z

+   * failure is client input that cannot be interpreted and must surface as an {@link
+   * IllegalArgumentException} (mapped to HTTP 400), never as a server error.
+   */
+  private static PageToken deserializePageToken(String requestedPageToken) {


Maybe we could reuse the Polaris standard serialization with Jackson 3?

I looked at PolarisObjectMapperUtil, but it is a Jackson 2 JSON mapper, while page tokens are binary SMILE via Jackson 3 (the SmileMapper here predates this PR).
I kept it as is; happy to switch if there is a shared Jackson 3 mapper you had in mind.

jbonofre · 2026-09-08T12:35:06Z


  private PageTokenUtil() {}

+  @VisibleForTesting


Doesn't it overlap with the existing serializePageToken()?

Agreed, removed in 04ed32a. The test now builds its own plain SmileMapper to craft non-PageToken payloads, so the production change is just the catch block.

jbonofre · 2026-09-08T12:35:57Z

+        arguments("valid base64, not SMILE", "AAAA"),
+        arguments(
+            "valid base64, plain text",
+            encode("hello world".getBytes(java.nio.charset.StandardCharsets.UTF_8))),


Maybe we should cover the byte-level/bit-flip corruption?

Added in 04ed32a: a full single-bit-flip and 0x00/0xFF byte-overwrite sweep over a valid token, plus a seeded random corruption test (overwrite/truncate/insert/flip, 20k iterations) over tokens of different shapes.

A corrupted token may still decode, but must never fail with anything other than IllegalArgumentException.

`PageTokenUtil.decodePageRequest` decoded the client-supplied `pageToken` (Base64 + SMILE) without error handling. A token that is valid Base64 but not a serialized page token (garbage, truncated, or produced by an incompatible Polaris version) escaped as a Jackson 3 `JacksonException` or, for an unknown token type id, as `IllegalStateException`. Neither is covered by the REST exception mappers, so client input turned into a 500 with an ERROR-level stack trace. Wrap the decoding step and rethrow any failure as `IllegalArgumentException("Invalid page token")`, which the existing `IcebergExceptionMapper` maps to 400. The original exception is kept as the cause for debug logging. Adds a parameterized unit test covering the malformed-token shapes and an integration test asserting the HTTP 400 on `listNamespaces`. Fixes apache#5465

…upted tokens - Widen the catch in `deserializePageToken` to `RuntimeException`: fuzzing a valid token shows the SMILE parser can fail with `ArrayIndexOutOfBoundsException`, which the enumerated catch let through to the default 500 mapping. - Remove the `smileMapperForTests()` accessor; the test builds its own plain `SmileMapper` to craft payloads that are not page tokens. - Add the fuzz-found token as a fixed regression case, a bit-flip / byte-overwrite sweep over a valid token, and a seeded random corruption test (overwrite, truncate, insert, flip) across tokens of different shapes. A corrupted token may still decode, but must never fail with anything other than `IllegalArgumentException`.

flyingImer

I think one remaining case needs fixing before merge: malformed page-token input can still return 500. Details inline.

flyingImer · 2026-09-08T19:22:27Z

+  private static PageToken deserializePageToken(String requestedPageToken) {
+    try {
+      var bytes = Base64.getUrlDecoder().decode(requestedPageToken);
+      return SMILE_MAPPER.readValue(bytes, PageToken.class);


Could we also reject a null result here? A SMILE root null deserializes successfully, so this catch never runs IIUC. With pageSize supplied, decodePageRequest then dereferences that null outside the catch and the request still returns 500. Please reject it as an invalid page token and add a regression case confirming 400 both with and without pageSize.

Good catch, thank you.
Reproduced: without pageSize the null token was returned to the caller, and with pageSize it failed with a NullPointerException outside the catch. The latest commit rejects a null result as an invalid page token, and the test class now has a "SMILE-encoded null" case plus a parameterized test covering pageSize absent, 0 and 5.

A SMILE-encoded null is valid input for the mapper and deserializes to a null PageToken, so the catch around decoding never runs. Without a page size the null was returned to the caller; with a page size, decodePageRequest dereferenced it outside the catch and the request still ended as a 500. Reject a null result as an invalid page token and add regression cases with and without a page size.

github-project-automation Bot added this to Basic Kanban Board Sep 8, 2026

github-project-automation Bot moved this to PRs In Progress in Basic Kanban Board Sep 8, 2026

jbonofre reviewed Sep 8, 2026

View reviewed changes

youngrae81 force-pushed the fix/page-token-400 branch from 4a9ffbd to 7beee54 Compare September 8, 2026 16:15

youngrae81 added 2 commits September 9, 2026 01:32

youngrae81 force-pushed the fix/page-token-400 branch from 7beee54 to 04ed32a Compare September 8, 2026 16:32

flyingImer reviewed Sep 8, 2026

View reviewed changes

Navigation Menu

Uh oh!

fix: return 400 instead of 500 for malformed pageToken on list endpoints - #5466

fix: return 400 instead of 500 for malformed pageToken on list endpoints#5466
youngrae81 wants to merge 3 commits into
apache:mainfrom
youngrae81:fix/page-token-400

youngrae81 commented Sep 8, 2026

Uh oh!

jbonofre Sep 8, 2026

Uh oh!

youngrae81 Sep 8, 2026 •

edited

Loading

Uh oh!

jbonofre Sep 8, 2026

Uh oh!

youngrae81 Sep 8, 2026

Uh oh!

jbonofre Sep 8, 2026

Uh oh!

youngrae81 Sep 8, 2026 •

edited

Loading

Uh oh!

jbonofre Sep 8, 2026

Uh oh!

youngrae81 Sep 8, 2026 •

edited

Loading

Uh oh!

flyingImer left a comment

Uh oh!

flyingImer Sep 8, 2026

Uh oh!

youngrae81 Sep 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

youngrae81 commented Sep 8, 2026

Checklist

Uh oh!

jbonofre Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

youngrae81 Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbonofre Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

youngrae81 Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

jbonofre Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

youngrae81 Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jbonofre Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

youngrae81 Sep 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flyingImer left a comment

Choose a reason for hiding this comment

Uh oh!

flyingImer Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

youngrae81 Sep 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

youngrae81 Sep 8, 2026 •

edited

Loading

youngrae81 Sep 8, 2026 •

edited

Loading

youngrae81 Sep 8, 2026 •

edited

Loading