This is an automated email from the ASF dual-hosted git repository.
jerryshao pushed a commit to branch branch-1.3
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-1.3 by this push:
new 5f585250d5 [Cherry-pick to branch-1.3] [#12783] fix(server): move
malformed path-param handling to a Jersey ExceptionMapper (#12784) (#12877)
5f585250d5 is described below
commit 5f585250d58a93e652aaa47258a61e59717ccf1c
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Thu Sep 3 21:14:51 2026 +0800
[Cherry-pick to branch-1.3] [#12783] fix(server): move malformed path-param
handling to a Jersey ExceptionMapper (#12784) (#12877)
**Cherry-pick Information:**
- Original commit: e4d076acbfa0f66427563f9e782cdb1370a70edd
- Target branch: `branch-1.3`
- Status: ✅ Clean cherry-pick (no conflicts)
Co-authored-by: Jerry Shao <[email protected]>
Co-authored-by: Claude Sonnet 5 <[email protected]>
---
.../integration/test/JsonErrorHandlerIT.java | 97 ++++++++++++++++++++++
.../apache/gravitino/server/GravitinoServer.java | 4 +
.../server/web/mapper/NotFoundExceptionMapper.java | 44 ++++++++++
.../server/web/mapper/ParamExceptionMapper.java | 58 +++++++++++++
.../web/mapper/TestNotFoundExceptionMapper.java | 51 ++++++++++++
.../web/mapper/TestParamExceptionMapper.java | 71 ++++++++++++++++
6 files changed, 325 insertions(+)
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/JsonErrorHandlerIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/JsonErrorHandlerIT.java
new file mode 100644
index 0000000000..797f9eda3c
--- /dev/null
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/JsonErrorHandlerIT.java
@@ -0,0 +1,97 @@
+/*
+ * 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.gravitino.client.integration.test;
+
+import java.net.URI;
+import java.net.http.HttpClient;
+import java.net.http.HttpRequest;
+import java.net.http.HttpResponse;
+import org.apache.gravitino.dto.responses.ErrorConstants;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.apache.gravitino.integration.test.util.BaseIT;
+import org.apache.gravitino.server.web.ObjectMapperProvider;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Integration test verifying that a malformed typed path parameter on the
metadata API (e.g. a
+ * non-numeric model version) returns the same structured JSON {@link
ErrorResponse} used by every
+ * other error on the API, instead of Jetty's default HTML error page.
+ *
+ * <p>Jersey fails to convert the path segment into the resource method's
typed {@code @PathParam}
+ * before any resource method runs, so the metalake/catalog/schema/model in
the URL need not
+ * actually exist for this failure to occur.
+ */
+public class JsonErrorHandlerIT extends BaseIT {
+
+ private final HttpClient httpClient = HttpClient.newHttpClient();
+
+ @Test
+ public void testMalformedModelVersionReturnsJsonErrorBody() throws Exception
{
+ HttpResponse<String> response =
+
sendGet("/api/metalakes/m/catalogs/c/schemas/s/models/mo/versions/abc");
+
+ Assertions.assertEquals(404, response.statusCode());
+ assertJsonNotFoundBody(response, "PathParamException");
+ }
+
+ @Test
+ public void testMalformedModelVersionUriReturnsJsonErrorBody() throws
Exception {
+ HttpResponse<String> response =
+
sendGet("/api/metalakes/m/catalogs/c/schemas/s/models/mo/versions/abc/uri");
+
+ Assertions.assertEquals(404, response.statusCode());
+ assertJsonNotFoundBody(response, "PathParamException");
+ }
+
+ @Test
+ public void testUnknownApiRouteStillReturnsJsonErrorBody() throws Exception {
+ // A route that Jersey cannot match at all is a different failure (no
@PathParam conversion
+ // is even attempted), but it must be covered by the same fix.
+ HttpResponse<String> response = sendGet("/api/v99/nonexistent/route");
+
+ Assertions.assertEquals(404, response.statusCode());
+ assertJsonNotFoundBody(response, "NotFoundException");
+ }
+
+ private HttpResponse<String> sendGet(String path) throws Exception {
+ HttpRequest request =
+ HttpRequest.newBuilder()
+ .uri(new URI("http://localhost:"; + getGravitinoServerPort() +
path))
+ .GET()
+ .build();
+ return httpClient.send(request, HttpResponse.BodyHandlers.ofString());
+ }
+
+ private void assertJsonNotFoundBody(HttpResponse<String> response, String
expectedType)
+ throws Exception {
+ String contentType =
response.headers().firstValue("Content-Type").orElse("");
+ Assertions.assertTrue(
+ contentType.contains("application/json"),
+ "Expected a JSON error body, got Content-Type: " + contentType);
+ Assertions.assertFalse(
+ response.body().contains("<html"), "Response body must not be Jetty's
HTML error page");
+
+ ErrorResponse errorResponse =
+ ObjectMapperProvider.objectMapper().readValue(response.body(),
ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(expectedType, errorResponse.getType());
+ Assertions.assertFalse(errorResponse.getMessage().isEmpty());
+ }
+}
diff --git
a/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
b/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
index e3825a7706..31f820af32 100644
--- a/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
+++ b/server/src/main/java/org/apache/gravitino/server/GravitinoServer.java
@@ -62,6 +62,8 @@ import
org.apache.gravitino.server.web.filter.GravitinoInterceptionService;
import org.apache.gravitino.server.web.mapper.JsonMappingExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonParseExceptionMapper;
import org.apache.gravitino.server.web.mapper.JsonProcessingExceptionMapper;
+import org.apache.gravitino.server.web.mapper.NotFoundExceptionMapper;
+import org.apache.gravitino.server.web.mapper.ParamExceptionMapper;
import org.apache.gravitino.server.web.ui.WebUIFilter;
import org.apache.gravitino.stats.StatisticDispatcher;
import org.apache.gravitino.tag.TagDispatcher;
@@ -165,6 +167,8 @@ public class GravitinoServer extends ResourceConfig {
register(JsonProcessingExceptionMapper.class);
register(JsonParseExceptionMapper.class);
register(JsonMappingExceptionMapper.class);
+ register(ParamExceptionMapper.class);
+ register(NotFoundExceptionMapper.class);
register(ObjectMapperProvider.class).register(JacksonFeature.class);
property(CommonProperties.JSON_JACKSON_DISABLED_MODULES,
"DefaultScalaModule");
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/mapper/NotFoundExceptionMapper.java
b/server/src/main/java/org/apache/gravitino/server/web/mapper/NotFoundExceptionMapper.java
new file mode 100644
index 0000000000..f3630619e1
--- /dev/null
+++
b/server/src/main/java/org/apache/gravitino/server/web/mapper/NotFoundExceptionMapper.java
@@ -0,0 +1,44 @@
+/*
+ * 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.gravitino.server.web.mapper;
+
+import javax.annotation.Priority;
+import javax.ws.rs.NotFoundException;
+import javax.ws.rs.core.Response;
+import javax.ws.rs.ext.ExceptionMapper;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.server.web.Utils;
+
+/**
+ * NotFoundExceptionMapper returns a structured JSON error body when Jersey
cannot match a request
+ * URI to any resource method at all, instead of letting the servlet container
fall back to Jetty's
+ * default HTML error page.
+ */
+@Priority(1)
+public class NotFoundExceptionMapper implements
ExceptionMapper<NotFoundException> {
+
+ @Override
+ public Response toResponse(NotFoundException exception) {
+ String message =
+ StringUtils.isBlank(exception.getMessage())
+ ? "Requested resource not found"
+ : exception.getMessage();
+ return Utils.notFound(NotFoundException.class.getSimpleName(), message);
+ }
+}
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
new file mode 100644
index 0000000000..80c578b1e2
--- /dev/null
+++
b/server/src/main/java/org/apache/gravitino/server/web/mapper/ParamExceptionMapper.java
@@ -0,0 +1,58 @@
+/*
+ * 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.gravitino.server.web.mapper;
+
+import javax.annotation.Nullable;
+import javax.annotation.Priority;
+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;
+
+/**
+ * ParamExceptionMapper returns a structured JSON error body when Jersey fails
to convert a request
+ * parameter (e.g. a typed {@code @PathParam}) into its declared Java type,
instead of letting the
+ * servlet container fall back to Jetty's default HTML error page.
+ *
+ * <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.
+ */
+@Priority(1)
+public class ParamExceptionMapper implements ExceptionMapper<ParamException> {
+
+ @Override
+ public Response toResponse(ParamException exception) {
+ String message =
+ String.format(
+ "Invalid value for %s parameter '%s'%s",
+ exception.getParameterType().getSimpleName(),
+ exception.getParameterName(),
+ causeMessage(exception.getCause()));
+
+ if (exception.getResponse().getStatus() ==
Response.Status.NOT_FOUND.getStatusCode()) {
+ return Utils.notFound(exception.getClass().getSimpleName(), message);
+ }
+ return Utils.illegalArguments(message);
+ }
+
+ private static String causeMessage(@Nullable Throwable cause) {
+ return cause == null || cause.getMessage() == null ? "" : ": " +
cause.getMessage();
+ }
+}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/mapper/TestNotFoundExceptionMapper.java
b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestNotFoundExceptionMapper.java
new file mode 100644
index 0000000000..f8fd02f16f
--- /dev/null
+++
b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestNotFoundExceptionMapper.java
@@ -0,0 +1,51 @@
+/*
+ * 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.gravitino.server.web.mapper;
+
+import javax.ws.rs.NotFoundException;
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.dto.responses.ErrorConstants;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestNotFoundExceptionMapper {
+
+ private final NotFoundExceptionMapper mapper = new NotFoundExceptionMapper();
+
+ @Test
+ public void testNotFoundExceptionWithoutMessage() {
+ Response response = mapper.toResponse(new NotFoundException());
+ ErrorResponse entity = (ErrorResponse) response.getEntity();
+
+ Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
response.getStatus());
+ Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, entity.getCode());
+ Assertions.assertEquals(NotFoundException.class.getSimpleName(),
entity.getType());
+ Assertions.assertFalse(entity.getMessage().isEmpty());
+ }
+
+ @Test
+ public void testNotFoundExceptionWithMessage() {
+ Response response = mapper.toResponse(new NotFoundException("no matching
route"));
+ ErrorResponse entity = (ErrorResponse) response.getEntity();
+
+ Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
response.getStatus());
+ Assertions.assertEquals("no matching route", entity.getMessage());
+ }
+}
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
new file mode 100644
index 0000000000..fa97ae8b12
--- /dev/null
+++
b/server/src/test/java/org/apache/gravitino/server/web/mapper/TestParamExceptionMapper.java
@@ -0,0 +1,71 @@
+/*
+ * 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.gravitino.server.web.mapper;
+
+import javax.ws.rs.core.Response;
+import org.apache.gravitino.dto.responses.ErrorConstants;
+import org.apache.gravitino.dto.responses.ErrorResponse;
+import org.glassfish.jersey.server.ParamException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestParamExceptionMapper {
+
+ private final ParamExceptionMapper mapper = new ParamExceptionMapper();
+
+ @Test
+ public void testPathParamExceptionReturnsNotFound() {
+ NumberFormatException cause = new NumberFormatException("For input string:
\"abc\"");
+ ParamException exception = new ParamException.PathParamException(cause,
"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("version"));
+ Assertions.assertTrue(entity.getMessage().contains("For input string"));
+ }
+
+ @Test
+ public void testQueryParamExceptionReturnsNotFound() {
+ 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.NOT_FOUND.getStatusCode(),
response.getStatus());
+ Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE, entity.getCode());
+ Assertions.assertTrue(entity.getMessage().contains("limit"));
+ }
+
+ @Test
+ public void testHeaderParamExceptionReturnsIllegalArguments() {
+ NumberFormatException cause = new NumberFormatException("For input string:
\"nope\"");
+ ParamException exception = new ParamException.HeaderParamException(cause,
"X-Count", 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.assertTrue(entity.getMessage().contains("X-Count"));
+ }
+}
