You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "flyingImer (via GitHub)" <gi...@apache.org> on 2026/09/22 02:27:09 UTC
[PR] Answer 400 for a query parameter that is not a number [polaris]
flyingImer opened a new pull request, #5572:
URL: https://github.com/apache/polaris/pull/5572
A list request whose `pageSize` is not a number answers `404 Not Found`. A query parameter is converted
before the resource method runs, and JAX-RS turns a failed conversion into `NotFoundException`, so no
Polaris code sees the request. On a list route that status reads as a missing catalog or namespace, which
sends the caller looking for the wrong problem.
This adds a `ParamConverterProvider` for `Integer` and `Long` that raises `BadRequestException`. That is a
`WebApplicationException`, so JAX-RS does not wrap it and the existing mappers render it as `400` with the
usual body:
```json
{"error":{"message":"pageSize must be an integer","type":"BadRequestException","code":400}}
```
The converter reads the parameter's own `@QueryParam` to name it. Registering a provider is what gives it
precedence: the runtime treats its generated numeric converter as a fallback and consults registered
providers first, so no priority annotation is involved. Worth knowing: registering any
`ParamConverterProvider` makes the runtime wrap every parameter converter in `RuntimeResolvedConverter`,
one provider lookup per parameter at startup and one null check per conversion.
`pageSize` is the only numeric query parameter declared anywhere. Affected routes:
| API | operations |
|---|---|
| Iceberg catalog | `listNamespaces`, `listTables`, `listViews` |
| Policy | `listPolicies`, `getApplicablePolicies` |
| Generic tables | `listGenericTables` |
No parameter of any other binding kind is numeric, and no resource declares a primitive numeric
parameter, so those are left to the runtime.
Tests cover each route and pin a missing catalog, an unknown route, readable, absent and empty values,
and the minimum-page-size path as unchanged.
--
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] Answer 400 for a query parameter that is not a number [polaris]
Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on PR #5572:
URL: https://github.com/apache/polaris/pull/5572#issuecomment-5785090480
@dimas-b @adutra thanks for the review! incorporated with your comments, please take another look when you get a chance
--
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] Answer 400 for a query parameter that is not a number [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra commented on code in PR #5572:
URL: https://github.com/apache/polaris/pull/5572#discussion_r4074908338
##########
runtime/service/src/main/java/org/apache/polaris/service/http/NumericParamConverterProvider.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * 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.polaris.service.http;
+
+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.ext.ParamConverter;
+import jakarta.ws.rs.ext.ParamConverterProvider;
+import jakarta.ws.rs.ext.Provider;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.function.Function;
+
+/**
+ * Converts {@code Integer} and {@code Long} request parameters so that a value which is not a
+ * number is answered with {@code 400 Bad Request}.
+ *
+ * <p>Without this provider the runtime's own numeric converter raises {@link
+ * NumberFormatException}, and JAX-RS turns a failed query parameter conversion into {@code 404 Not
+ * Found}. On a listing route that status reads as "this catalog or namespace does not exist", which
+ * sends the caller looking for the wrong problem. Throwing {@link BadRequestException}, which is a
+ * {@code WebApplicationException}, keeps the status the caller needs, and the exception mappers
+ * render it in the usual error body.
+ *
+ * <p>Only the boxed types are handled. No resource declares a primitive numeric parameter, so those
+ * are left to the runtime.
+ */
+@Provider
+public class NumericParamConverterProvider implements ParamConverterProvider {
+
+ private static final String INTEGER_EXPECTED = "must be an integer";
Review Comment:
nit: these constants only add an indirection that makes it harder for readers to grasp the whole sentence. Consider inlining them.
##########
runtime/service/src/main/java/org/apache/polaris/service/http/NumericParamConverterProvider.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * 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.polaris.service.http;
+
+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.ext.ParamConverter;
+import jakarta.ws.rs.ext.ParamConverterProvider;
+import jakarta.ws.rs.ext.Provider;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.function.Function;
+
+/**
+ * Converts {@code Integer} and {@code Long} request parameters so that a value which is not a
+ * number is answered with {@code 400 Bad Request}.
+ *
+ * <p>Without this provider the runtime's own numeric converter raises {@link
+ * NumberFormatException}, and JAX-RS turns a failed query parameter conversion into {@code 404 Not
Review Comment:
This looks surprising. A `NumberFormatException` is an `IllegalArgumentException`, which should translate as 400, not 404. Are you implying that JAX-RS may have a bug?
--
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] Answer 400 for a query parameter that is not a number [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5572:
URL: https://github.com/apache/polaris/pull/5572#discussion_r4076891585
##########
runtime/service/src/test/java/org/apache/polaris/service/http/NumericQueryParameterTest.java:
##########
@@ -0,0 +1,157 @@
+/*
+ * 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.polaris.service.http;
+
+import static io.restassured.RestAssured.given;
+import static org.hamcrest.CoreMatchers.is;
+
+import io.quarkus.test.junit.QuarkusTest;
+import io.restassured.http.ContentType;
+import io.restassured.specification.RequestSpecification;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Checks over the wire that an unreadable {@code pageSize} is answered with {@code 400 Bad Request}
+ * on every route that accepts it, and that the answers which were already correct did not move.
+ *
+ * <p>The catalog in these requests does not exist, and it does not need to: a query parameter is
+ * converted before the resource method runs, so the rejection happens whether or not the rest of
+ * the path resolves. The requests without {@code pageSize} show what the same path answers
+ * otherwise, which is how these tests tell a rejected parameter apart from a missing catalog.
+ */
+@QuarkusTest
+public class NumericQueryParameterTest {
+
+ private static final String CATALOG = "nonexistent_catalog";
+ private static final String ICEBERG = "/api/catalog/v1/" + CATALOG;
+ private static final String POLARIS = "/api/catalog/polaris/v1/" + CATALOG;
+
+ private static String token;
+
+ private static RequestSpecification authenticated() {
+ if (token == null) {
+ token =
+ given()
+ .contentType(ContentType.URLENC)
+ .formParam("grant_type", "client_credentials")
+ .formParam("scope", "PRINCIPAL_ROLE:ALL")
+ .formParam("client_id", "test-admin")
+ .formParam("client_secret", "test-secret")
+ .when()
+ .post("/api/catalog/v1/oauth/tokens")
+ .then()
+ .statusCode(200)
+ .extract()
+ .path("access_token");
+ }
+ return given().header("Authorization", "Bearer " + token);
+ }
+
+ private static void expectPageSizeRejected(String path) {
+ authenticated()
+ .when()
+ .get(path)
+ .then()
+ .statusCode(400)
+ .body("error.message", is("pageSize must be an integer"))
+ .body("error.type", is("BadRequestException"))
+ .body("error.code", is(400));
+ }
+
+ private static void expectNamespaceNotFound(String path) {
+ authenticated()
+ .when()
+ .get(path)
+ .then()
+ .statusCode(404)
+ .body("error.type", is("NoSuchNamespaceException"))
+ .body("error.code", is(404));
+ }
+
+ @Test
+ public void testListNamespacesRejectsAnUnreadablePageSize() {
+ expectPageSizeRejected(ICEBERG + "/namespaces?pageSize=large");
+ }
+
+ @Test
+ public void testListTablesRejectsAnUnreadablePageSize() {
+ expectPageSizeRejected(ICEBERG + "/namespaces/ns/tables?pageSize=large");
+ }
+
+ @Test
+ public void testListViewsRejectsAnUnreadablePageSize() {
+ expectPageSizeRejected(ICEBERG + "/namespaces/ns/views?pageSize=large");
+ }
+
+ @Test
+ public void testListPoliciesRejectsAnUnreadablePageSize() {
+ expectPageSizeRejected(POLARIS + "/namespaces/ns/policies?pageSize=large");
+ }
+
+ @Test
+ public void testGetApplicablePoliciesRejectsAnUnreadablePageSize() {
+ expectPageSizeRejected(POLARIS + "/applicable-policies?pageSize=large");
+ }
+
+ @Test
+ public void testListGenericTablesRejectsAnUnreadablePageSize() {
+ expectPageSizeRejected(POLARIS + "/namespaces/ns/generic-tables?pageSize=large");
+ }
+
+ @Test
+ public void testAMissingCatalogIsStillNotFound() {
+ expectNamespaceNotFound(ICEBERG + "/namespaces");
+ }
+
+ @Test
+ public void testAReadablePageSizeStillReachesTheResource() {
+ expectNamespaceNotFound(ICEBERG + "/namespaces?pageSize=5");
+ }
+
+ @Test
+ public void testAnEmptyPageSizeStillReachesTheResource() {
+ // An empty value binds as if the parameter had not been sent. The converter is not consulted.
+ expectNamespaceNotFound(ICEBERG + "/namespaces?pageSize=");
+ }
+
+ @Test
+ public void testAnUnknownRouteIsStillNotFound() {
+ authenticated()
+ .when()
+ .get(ICEBERG + "/no-such-subresource")
+ .then()
+ .statusCode(404)
+ .body("error.type", is("NotFoundException"))
+ .body("error.code", is(404));
+ }
+
+ @Test
+ public void testAPageSizeBelowTheMinimumIsStillRejectedByValidation() {
+ // The converter reads 0 successfully and hands it on; the minimum declared on this route is
+ // what
+ // refuses it. This pins that path so the converter cannot quietly take it over.
Review Comment:
nit: odd line breaks
--
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] Answer 400 for a query parameter that is not a number [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #5572:
URL: https://github.com/apache/polaris/pull/5572#discussion_r4076750948
##########
runtime/service/src/main/java/org/apache/polaris/service/http/NumericParamConverterProvider.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * 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.polaris.service.http;
+
+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.ext.ParamConverter;
+import jakarta.ws.rs.ext.ParamConverterProvider;
+import jakarta.ws.rs.ext.Provider;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.function.Function;
+
+/**
+ * Converts {@code Integer} and {@code Long} request parameters so that a value which is not a
+ * number is answered with {@code 400 Bad Request}.
+ *
+ * <p>Without this provider the runtime's own numeric converter raises {@link
+ * NumberFormatException}, and JAX-RS turns a failed query parameter conversion into {@code 404 Not
Review Comment:
wow :facepalm:
--
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] Answer 400 for a query parameter that is not a number [polaris]
Posted by "flyingImer (via GitHub)" <gi...@apache.org>.
flyingImer commented on code in PR #5572:
URL: https://github.com/apache/polaris/pull/5572#discussion_r4076582330
##########
runtime/service/src/main/java/org/apache/polaris/service/http/NumericParamConverterProvider.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * 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.polaris.service.http;
+
+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.ext.ParamConverter;
+import jakarta.ws.rs.ext.ParamConverterProvider;
+import jakarta.ws.rs.ext.Provider;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.function.Function;
+
+/**
+ * Converts {@code Integer} and {@code Long} request parameters so that a value which is not a
+ * number is answered with {@code 400 Bad Request}.
+ *
+ * <p>Without this provider the runtime's own numeric converter raises {@link
+ * NumberFormatException}, and JAX-RS turns a failed query parameter conversion into {@code 404 Not
Review Comment:
No bug, it is a MUST in the spec. Jakarta RESTful Web Services 4.0, section 3.2 (Fields and Bean Properties):
> Other exceptions thrown during construction of field or property values using any of the 5 steps listed above are treated as client errors: if the field or property is annotated with `@MatrixParam`, `@QueryParam` or `@PathParam` then an implementation MUST generate an instance of `NotFoundException` (404 status) that wraps the thrown exception and no entity; if the field or property is annotated with `@HeaderParam` or `@CookieParam` then an implementation MUST generate an instance of `BadRequestException` (400 status) that wraps the thrown exception and no entity.
So `NumberFormatException` never reaches a mapper as itself. It becomes the cause of a `NotFoundException`, and `IcebergExceptionMapper` switches on the thrown exception, where `WebApplicationException` yields the wrapped 404. The `IllegalArgumentException` branch is unreachable on this path, and the causal chain walk above the switch only looks for cloud exceptions. Measured on main before this change, on every route that takes `pageSize`:
```
GET /api/catalog/v1/{prefix}/namespaces?pageSize=large
404 {"error":{"message":"HTTP 404 Not Found","type":"NotFoundException","code":404}}
```
The first half of that same sentence is why this fix works: a `WebApplicationException` is the one thing the rule passes through untouched, so a converter throwing `BadRequestException` gets the status the caller needs. The asymmetry looks deliberate rather than accidental, since `@HeaderParam` and `@CookieParam` already get 400.
Good catch on the javadoc though. It stated the behaviour without naming the rule, so there was no way to tell specified from buggy. I will cite section 3.2 and say that a `WebApplicationException` is the documented exemption.
##########
runtime/service/src/main/java/org/apache/polaris/service/http/NumericParamConverterProvider.java:
##########
@@ -0,0 +1,106 @@
+/*
+ * 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.polaris.service.http;
+
+import jakarta.ws.rs.BadRequestException;
+import jakarta.ws.rs.QueryParam;
+import jakarta.ws.rs.ext.ParamConverter;
+import jakarta.ws.rs.ext.ParamConverterProvider;
+import jakarta.ws.rs.ext.Provider;
+import java.lang.annotation.Annotation;
+import java.lang.reflect.Type;
+import java.util.function.Function;
+
+/**
+ * Converts {@code Integer} and {@code Long} request parameters so that a value which is not a
+ * number is answered with {@code 400 Bad Request}.
+ *
+ * <p>Without this provider the runtime's own numeric converter raises {@link
+ * NumberFormatException}, and JAX-RS turns a failed query parameter conversion into {@code 404 Not
+ * Found}. On a listing route that status reads as "this catalog or namespace does not exist", which
+ * sends the caller looking for the wrong problem. Throwing {@link BadRequestException}, which is a
+ * {@code WebApplicationException}, keeps the status the caller needs, and the exception mappers
+ * render it in the usual error body.
+ *
+ * <p>Only the boxed types are handled. No resource declares a primitive numeric parameter, so those
+ * are left to the runtime.
+ */
+@Provider
+public class NumericParamConverterProvider implements ParamConverterProvider {
+
+ private static final String INTEGER_EXPECTED = "must be an integer";
Review Comment:
Agreed, inlining them.
--
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] Answer 400 for a query parameter that is not a number [polaris]
Posted by "adutra (via GitHub)" <gi...@apache.org>.
adutra merged PR #5572:
URL: https://github.com/apache/polaris/pull/5572
--
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