This is an automated email from the ASF dual-hosted git repository. mchades pushed a commit to branch codex/fix-query-param-status in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 4705083fd18af5b65a787cd13800ed9dbebe63d3 Author: mchades <[email protected]> AuthorDate: Wed Nov 4 22:12:42 2026 +0800 fix(server): Return 400 for invalid query parameters --- docs/open-api/jobs.yaml | 2 ++ .../server/web/mapper/ParamExceptionMapper.java | 8 +++++ .../web/mapper/TestParamExceptionMapper.java | 21 ++++++++++-- .../server/web/rest/TestJobOperations.java | 39 ++++++++++++++++++++++ 4 files changed, 68 insertions(+), 2 deletions(-) diff --git a/docs/open-api/jobs.yaml b/docs/open-api/jobs.yaml index 00ed2cd991..3e25870915 100644 --- a/docs/open-api/jobs.yaml +++ b/docs/open-api/jobs.yaml @@ -293,6 +293,8 @@ paths: examples: JobResponse: $ref: "#/components/examples/JobResponse" + "400": + $ref: "./openapi.yaml#/components/responses/BadRequestErrorResponse" "404": description: Not Found - The specified job does not exist in the specified metalake content: diff --git a/server/src/main/java/org/apache/gravitino/server/web/mapper/ParamExceptionMapper.java b/server/src/main/java/org/apache/gravitino/server/web/mapper/ParamExceptionMapper.java index 80c578b1e2..618bfb3205 100644 --- a/server/src/main/java/org/apache/gravitino/server/web/mapper/ParamExceptionMapper.java +++ b/server/src/main/java/org/apache/gravitino/server/web/mapper/ParamExceptionMapper.java @@ -24,6 +24,7 @@ import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import org.apache.gravitino.server.web.Utils; import org.glassfish.jersey.server.ParamException; +import org.glassfish.jersey.server.ParamException.QueryParamException; /** * ParamExceptionMapper returns a structured JSON error body when Jersey fails to convert a request @@ -33,6 +34,9 @@ import org.glassfish.jersey.server.ParamException; * <p>{@link ParamException} is thrown by Jersey itself while binding request parameters, before any * resource method runs, so this mapper is the only place such a conversion failure can be * intercepted. + * + * <p>Query parameter conversion failures return 400 to match other request-validation errors. Other + * URI parameter conversion failures retain Jersey's 404 response. */ @Priority(1) public class ParamExceptionMapper implements ExceptionMapper<ParamException> { @@ -46,6 +50,10 @@ public class ParamExceptionMapper implements ExceptionMapper<ParamException> { exception.getParameterName(), causeMessage(exception.getCause())); + if (exception instanceof QueryParamException) { + return Utils.illegalArguments(message); + } + if (exception.getResponse().getStatus() == Response.Status.NOT_FOUND.getStatusCode()) { return Utils.notFound(exception.getClass().getSimpleName(), message); } diff --git a/server/src/test/java/org/apache/gravitino/server/web/mapper/TestParamExceptionMapper.java b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestParamExceptionMapper.java index fa97ae8b12..3b32e156d1 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/mapper/TestParamExceptionMapper.java +++ b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestParamExceptionMapper.java @@ -43,17 +43,34 @@ public class TestParamExceptionMapper { Assertions.assertTrue(entity.getMessage().contains("For input string")); } + /** Verifies query conversion failures use the API's bad-request error response. */ @Test - public void testQueryParamExceptionReturnsNotFound() { + public void testQueryParamExceptionReturnsIllegalArguments() { NumberFormatException cause = new NumberFormatException("For input string: \"xyz\""); ParamException exception = new ParamException.QueryParamException(cause, "limit", null); Response response = mapper.toResponse(exception); ErrorResponse entity = (ErrorResponse) response.getEntity(); + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, entity.getCode()); + Assertions.assertEquals(IllegalArgumentException.class.getSimpleName(), entity.getType()); + Assertions.assertTrue(entity.getMessage().contains("limit")); + Assertions.assertTrue(entity.getMessage().contains("For input string")); + } + + /** Verifies the query-parameter override does not change other URI parameter errors. */ + @Test + public void testMatrixParamExceptionReturnsNotFound() { + ParamException exception = + new ParamException.MatrixParamException(new NumberFormatException("abc"), "version", null); + + Response response = mapper.toResponse(exception); + ErrorResponse entity = (ErrorResponse) response.getEntity(); + Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(), response.getStatus()); Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, entity.getCode()); - Assertions.assertTrue(entity.getMessage().contains("limit")); + Assertions.assertTrue(entity.getMessage().contains("version")); } @Test diff --git a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java index 14fe13afd2..7df3b2205c 100644 --- a/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java +++ b/server/src/test/java/org/apache/gravitino/server/web/rest/TestJobOperations.java @@ -29,6 +29,7 @@ import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import com.codahale.metrics.annotation.ResponseMetered; @@ -86,6 +87,7 @@ import org.apache.gravitino.meta.JobEntity; import org.apache.gravitino.meta.JobTemplateEntity; import org.apache.gravitino.rest.RESTUtils; import org.apache.gravitino.server.web.ObjectMapperProvider; +import org.apache.gravitino.server.web.mapper.ParamExceptionMapper; import org.apache.gravitino.storage.IdGenerator; import org.apache.gravitino.storage.RandomIdGenerator; import org.apache.gravitino.utils.NamespaceUtil; @@ -96,6 +98,8 @@ import org.glassfish.jersey.test.TestProperties; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; public class TestJobOperations extends JerseyTest { @@ -128,6 +132,7 @@ public class TestJobOperations extends JerseyTest { ResourceConfig resourceConfig = new ResourceConfig(); resourceConfig.register(JobOperations.class); resourceConfig.register(ObjectMapperProvider.class); + resourceConfig.register(ParamExceptionMapper.class); resourceConfig.register( new AbstractBinder() { @Override @@ -1267,6 +1272,40 @@ public class TestJobOperations extends JerseyTest { Assertions.assertEquals(stderr, jobResp.getJob().stderr()); } + /** + * Verifies malformed output limits are rejected before the job dispatcher runs. + * + * @param parameter the query parameter to validate + * @param value a non-numeric or out-of-range integer value + */ + @ParameterizedTest + @CsvSource({ + "outputMaxLines, abc", + "outputMaxLines, 2147483648", + "outputMaxLines, -2147483649", + "outputMaxBytes, abc", + "outputMaxBytes, 2147483648", + "outputMaxBytes, -2147483649" + }) + public void testGetJobWithMalformedOutputLimit(String parameter, String value) { + try (Response response = + target(jobRunPath()) + .path("job-test") + .queryParam("includeOutput", "true") + .queryParam(parameter, value) + .request(APPLICATION_JSON_TYPE) + .accept("application/vnd.gravitino.v1+json") + .get()) { + Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), response.getStatus()); + Assertions.assertTrue(response.getMediaType().isCompatible(APPLICATION_JSON_TYPE)); + ErrorResponse error = response.readEntity(ErrorResponse.class); + Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE, error.getCode()); + Assertions.assertEquals(IllegalArgumentException.class.getSimpleName(), error.getType()); + Assertions.assertTrue(error.getMessage().contains(parameter)); + verifyNoInteractions(jobOperationDispatcher); + } + } + @Test public void testGetJobWithOutputUnsupportedOperation() { JobEntity job = newJobEntity("shell_template_1", JobHandle.Status.SUCCEEDED);
