This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 922009a3f2 [#10667] fix(iceberg): Return JSON error body instead of
HTML for pre-JAX-RS errors (#10668)
922009a3f2 is described below
commit 922009a3f21f60d51f173d62068ab8238d4d806c
Author: akshay thorat <[email protected]>
AuthorDate: Sun Apr 5 07:45:04 2026 -0700
[#10667] fix(iceberg): Return JSON error body instead of HTML for
pre-JAX-RS errors (#10668)
### What changes were proposed in this pull request?
Override authentication error handling in the Iceberg REST server to
produce JSON error bodies conforming to the Iceberg REST API
specification, instead of Jetty's default HTML error pages.
Changes:
- Added `sendAuthErrorResponse(HttpServletResponse, Exception)` hook to
`AuthenticationFilter` (base impl: `resp.sendError()`).
- Created `IcebergAuthenticationFilter` that overrides the hook to write
Iceberg-spec JSON `ErrorResponse` bodies.
- Added `convertToIcebergException(Exception)` to
`IcebergExceptionMapper` that converts Gravitino/generic exceptions to
Iceberg spec exceptions (e.g., `UnauthorizedException` →
`NotAuthorizedException`, `IllegalArgumentException` →
`BadRequestException`).
- Made `JettyServer` non-final with a `createAuthenticationFilter()`
factory method; `RESTService` overrides it to use
`IcebergAuthenticationFilter`.
- Added `TestIcebergAuthenticationFilter` with 4 test cases.
### Why are the changes needed?
When authentication fails, `AuthenticationFilter` runs at the servlet
filter level — before the request reaches JAX-RS — so
`IcebergExceptionMapper` is never invoked. Jetty's default
`ErrorHandler` produces an HTML error page, violating the Iceberg REST
API specification which requires all errors to be JSON `ErrorResponse`
bodies. This causes Iceberg REST clients (e.g., Java `RESTCatalog`) to
fail with a secondary JSON parse error, masking the real authentication
failure.
Fix: #10667
### Does this PR introduce _any_ user-facing change?
No API changes. Error responses from the Iceberg REST server for
pre-JAX-RS failures (e.g., 401 Unauthorized) will now be properly
formatted JSON instead of HTML, which is what clients already expect.
### How was this patch tested?
Added `TestIcebergAuthenticationFilter` covering:
1. `UnauthorizedException` → 401 with `NotAuthorizedException` type
2. `RuntimeException` → 500 with `ServiceFailureException` type
3. `ForbiddenException` → 403 with `ForbiddenException` type
4. Null message → falls back to HTTP status message
All existing tests in server-common and iceberg-rest-server continue to
pass.
---
.../org/apache/gravitino/iceberg/RESTService.java | 9 +-
.../service/IcebergAuthenticationFilter.java | 61 ++++++++++
.../iceberg/service/IcebergExceptionMapper.java | 42 +++++++
.../iceberg/service/IcebergRESTUtils.java | 12 ++
.../service/TestIcebergAuthenticationFilter.java | 123 +++++++++++++++++++++
.../authentication/AuthenticationFilter.java | 23 +++-
.../apache/gravitino/server/web/JettyServer.java | 12 +-
7 files changed, 277 insertions(+), 5 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
index 23f1fcf042..f4bcfedd8b 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/RESTService.java
@@ -27,6 +27,7 @@ import org.apache.gravitino.Configs;
import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.auxiliary.GravitinoAuxiliaryService;
import org.apache.gravitino.iceberg.common.IcebergConfig;
+import org.apache.gravitino.iceberg.service.IcebergAuthenticationFilter;
import org.apache.gravitino.iceberg.service.IcebergCatalogWrapperManager;
import org.apache.gravitino.iceberg.service.IcebergExceptionMapper;
import org.apache.gravitino.iceberg.service.IcebergObjectMapperProvider;
@@ -79,7 +80,13 @@ public class RESTService implements
GravitinoAuxiliaryService {
private void initServer(IcebergConfig icebergConfig) {
JettyServerConfig serverConfig =
JettyServerConfig.fromConfig(icebergConfig);
- server = new JettyServer();
+ server =
+ new JettyServer() {
+ @Override
+ protected javax.servlet.Filter createAuthenticationFilter() {
+ return new IcebergAuthenticationFilter();
+ }
+ };
MetricsSystem metricsSystem = GravitinoEnv.getInstance().metricsSystem();
server.initialize(serverConfig, SERVICE_NAME, false /* shouldEnableUI */);
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergAuthenticationFilter.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergAuthenticationFilter.java
new file mode 100644
index 0000000000..20209b056e
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergAuthenticationFilter.java
@@ -0,0 +1,61 @@
+/*
+ * 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.iceberg.service;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.server.authentication.AuthenticationFilter;
+import org.apache.iceberg.rest.responses.ErrorResponse;
+import org.eclipse.jetty.http.HttpStatus;
+
+/**
+ * An {@link AuthenticationFilter} subclass for the Iceberg REST server that
produces JSON error
+ * responses conforming to the Iceberg REST API specification.
+ *
+ * <p>When authentication fails, the default {@link AuthenticationFilter}
calls {@code
+ * resp.sendError()} which produces HTML error pages via Jetty's default error
handler. This
+ * subclass overrides the error response to write a proper Iceberg {@link
ErrorResponse} JSON body,
+ * which Iceberg REST clients (e.g., the Java {@code RESTCatalog}) expect.
+ */
+public class IcebergAuthenticationFilter extends AuthenticationFilter {
+
+ private static final ObjectMapper MAPPER = IcebergObjectMapper.getInstance();
+
+ @Override
+ protected void sendAuthErrorResponse(HttpServletResponse response, Exception
exception)
+ throws IOException {
+ Exception icebergException =
IcebergExceptionMapper.convertToIcebergException(exception);
+ int status = IcebergExceptionMapper.getErrorCode(icebergException);
+ String message = icebergException.getMessage();
+ if (StringUtils.isBlank(message)) {
+ message = HttpStatus.getMessage(status);
+ }
+
+ String type = icebergException.getClass().getSimpleName();
+ ErrorResponse errorResponse = IcebergRESTUtils.errorResponse(status, type,
message);
+
+ response.setStatus(status);
+ response.setContentType("application/json");
+ response.setCharacterEncoding(StandardCharsets.UTF_8.name());
+ MAPPER.writeValue(response.getWriter(), errorResponse);
+ }
+}
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
index c1a443b24e..ecc16caca9 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
@@ -26,7 +26,9 @@ import javax.ws.rs.ext.ExceptionMapper;
import javax.ws.rs.ext.Provider;
import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
+import org.apache.gravitino.exceptions.UnauthorizedException;
import org.apache.iceberg.exceptions.AlreadyExistsException;
+import org.apache.iceberg.exceptions.BadRequestException;
import org.apache.iceberg.exceptions.CommitFailedException;
import org.apache.iceberg.exceptions.CommitStateUnknownException;
import org.apache.iceberg.exceptions.ForbiddenException;
@@ -36,6 +38,7 @@ import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.exceptions.NoSuchViewException;
import org.apache.iceberg.exceptions.NotAuthorizedException;
+import org.apache.iceberg.exceptions.ServiceFailureException;
import org.apache.iceberg.exceptions.ServiceUnavailableException;
import org.apache.iceberg.exceptions.UnprocessableEntityException;
import org.apache.iceberg.exceptions.ValidationException;
@@ -56,6 +59,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
.put(IllegalNameIdentifierException.class, 400)
.put(NamespaceNotEmptyException.class, 400)
.put(NotAuthorizedException.class, 401)
+ .put(UnauthorizedException.class, 401)
.put(org.apache.gravitino.exceptions.ForbiddenException.class, 403)
.put(ForbiddenException.class, 403)
.put(NoSuchNamespaceException.class, 404)
@@ -68,9 +72,47 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
.put(CommitFailedException.class, 409)
.put(UnprocessableEntityException.class, 422)
.put(CommitStateUnknownException.class, 500)
+ .put(ServiceFailureException.class, 500)
.put(ServiceUnavailableException.class, 503)
.build();
+ /**
+ * Returns the HTTP status code for the given exception based on the Iceberg
REST spec.
+ *
+ * @param ex the exception
+ * @return the HTTP status code, defaulting to 500 for unmapped exceptions
+ */
+ public static int getErrorCode(Exception ex) {
+ return EXCEPTION_ERROR_CODES.getOrDefault(
+ ex.getClass(), Status.INTERNAL_SERVER_ERROR.getStatusCode());
+ }
+
+ /**
+ * Converts a Gravitino or generic exception to the corresponding Iceberg
REST spec exception.
+ *
+ * @param e the original exception
+ * @return the equivalent Iceberg exception, or the original if already an
Iceberg exception
+ */
+ public static Exception convertToIcebergException(Exception e) {
+ String message = e.getMessage() != null ? e.getMessage() : "";
+ if (e instanceof UnauthorizedException) {
+ return new NotAuthorizedException("%s", message);
+ }
+ if (e instanceof org.apache.gravitino.exceptions.ForbiddenException) {
+ return new ForbiddenException("%s", message);
+ }
+ if (e instanceof IllegalArgumentException
+ || e instanceof IllegalNameIdentifierException
+ || e instanceof ValidationException
+ || e instanceof NamespaceNotEmptyException) {
+ return new BadRequestException("%s", message);
+ }
+ if (EXCEPTION_ERROR_CODES.containsKey(e.getClass())) {
+ return e;
+ }
+ return new ServiceFailureException("%s", message);
+ }
+
@Override
public Response toResponse(Exception ex) {
return toRESTResponse(ex);
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergRESTUtils.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergRESTUtils.java
index 84ea56cc37..78ffd7e801 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergRESTUtils.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergRESTUtils.java
@@ -74,6 +74,18 @@ public class IcebergRESTUtils {
.build();
}
+ /**
+ * Build an Iceberg {@link ErrorResponse} for a given HTTP status code and
message, without an
+ * exception. Used by the Jetty error handler for pre-JAX-RS errors.
+ */
+ public static ErrorResponse errorResponse(int httpStatus, String type,
String message) {
+ return ErrorResponse.builder()
+ .responseCode(httpStatus)
+ .withType(type)
+ .withMessage(message)
+ .build();
+ }
+
public static Instant calculateNewTimestamp(Instant currentTimestamp, int
hours) {
LocalDateTime currentDateTime =
LocalDateTime.ofInstant(currentTimestamp, ZoneId.systemDefault());
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
new file mode 100644
index 0000000000..404ba94827
--- /dev/null
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergAuthenticationFilter.java
@@ -0,0 +1,123 @@
+/*
+ * 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.iceberg.service;
+
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+import com.fasterxml.jackson.databind.ObjectMapper;
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import javax.servlet.http.HttpServletResponse;
+import org.apache.gravitino.exceptions.UnauthorizedException;
+import org.apache.iceberg.rest.responses.ErrorResponse;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestIcebergAuthenticationFilter {
+
+ private static final ObjectMapper MAPPER = IcebergObjectMapper.getInstance();
+
+ @Test
+ public void testUnauthorizedErrorReturnsJson() throws Exception {
+ IcebergAuthenticationFilter filter = new IcebergAuthenticationFilter();
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(
+ response, new UnauthorizedException("The provided credentials did not
support"));
+
+ verify(response).setStatus(HttpServletResponse.SC_UNAUTHORIZED);
+ verify(response).setContentType("application/json");
+ verify(response).setCharacterEncoding("UTF-8");
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ErrorResponse errorResponse = MAPPER.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(401, errorResponse.code());
+ Assertions.assertEquals("NotAuthorizedException", errorResponse.type());
+ Assertions.assertEquals("The provided credentials did not support",
errorResponse.message());
+ }
+
+ @Test
+ public void testInternalServerErrorReturnsJson() throws Exception {
+ IcebergAuthenticationFilter filter = new IcebergAuthenticationFilter();
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(response, new RuntimeException("Something
went wrong"));
+
+ verify(response).setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ErrorResponse errorResponse = MAPPER.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(500, errorResponse.code());
+ Assertions.assertEquals("ServiceFailureException", errorResponse.type());
+ Assertions.assertEquals("Something went wrong", errorResponse.message());
+ }
+
+ @Test
+ public void testForbiddenExceptionReturnsJson() throws Exception {
+ IcebergAuthenticationFilter filter = new IcebergAuthenticationFilter();
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(
+ response, new
org.apache.gravitino.exceptions.ForbiddenException("Access denied"));
+
+ verify(response).setStatus(HttpServletResponse.SC_FORBIDDEN);
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ErrorResponse errorResponse = MAPPER.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(403, errorResponse.code());
+ Assertions.assertEquals("ForbiddenException", errorResponse.type());
+ Assertions.assertEquals("Access denied", errorResponse.message());
+ }
+
+ @Test
+ public void testNullMessageUsesDefaultStatusMessage() throws Exception {
+ IcebergAuthenticationFilter filter = new IcebergAuthenticationFilter();
+
+ HttpServletResponse response = mock(HttpServletResponse.class);
+ StringWriter stringWriter = new StringWriter();
+ PrintWriter printWriter = new PrintWriter(stringWriter);
+ when(response.getWriter()).thenReturn(printWriter);
+
+ filter.sendAuthErrorResponse(response, new RuntimeException((String)
null));
+
+ printWriter.flush();
+ String json = stringWriter.toString();
+ ErrorResponse errorResponse = MAPPER.readValue(json, ErrorResponse.class);
+ Assertions.assertEquals(500, errorResponse.code());
+ Assertions.assertEquals("ServiceFailureException", errorResponse.type());
+ Assertions.assertEquals("Server Error", errorResponse.message());
+ }
+}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
b/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
index 7df4e68d6a..ae234285cd 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/authentication/AuthenticationFilter.java
@@ -99,13 +99,32 @@ public class AuthenticationFilter implements Filter {
resp.setHeader(AuthConstants.HTTP_CHALLENGE_HEADER, challenge);
}
}
- resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, ue.getMessage());
+ sendAuthErrorResponse(resp, ue);
} catch (Exception e) {
HttpServletResponse resp = (HttpServletResponse) response;
- resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, e.getMessage());
+ sendAuthErrorResponse(resp, e);
}
}
+ /**
+ * Sends an error response when authentication fails. Subclasses can
override this to customize
+ * the error response format (e.g., Iceberg REST server returns JSON error
bodies).
+ *
+ * <p>TODO: Gravitino server should override this method to return a correct
JSON response
+ * following the Gravitino error response spec.
+ *
+ * @param response the HTTP servlet response
+ * @param exception the authentication exception
+ */
+ protected void sendAuthErrorResponse(HttpServletResponse response, Exception
exception)
+ throws IOException {
+ int status =
+ exception instanceof UnauthorizedException
+ ? HttpServletResponse.SC_UNAUTHORIZED
+ : HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
+ response.sendError(status, exception.getMessage());
+ }
+
@Override
public void destroy() {}
}
diff --git
a/server-common/src/main/java/org/apache/gravitino/server/web/JettyServer.java
b/server-common/src/main/java/org/apache/gravitino/server/web/JettyServer.java
index e81b4fa9e1..1af9ed8259 100644
---
a/server-common/src/main/java/org/apache/gravitino/server/web/JettyServer.java
+++
b/server-common/src/main/java/org/apache/gravitino/server/web/JettyServer.java
@@ -64,7 +64,7 @@ import org.eclipse.jetty.webapp.WebAppContext;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public final class JettyServer {
+public class JettyServer {
private static final Logger LOG = LoggerFactory.getLogger(JettyServer.class);
@@ -470,6 +470,14 @@ public final class JettyServer {
servletContextHandler.addFilter(
CorsFilterHolder.create(serverConfig), pathSpec,
EnumSet.allOf(DispatcherType.class));
}
- addFilter(new AuthenticationFilter(), pathSpec);
+ addFilter(createAuthenticationFilter(), pathSpec);
+ }
+
+ /**
+ * Creates the authentication filter for this server. Subclasses can
override this to provide a
+ * custom authentication filter (e.g., one that returns Iceberg-spec JSON
error responses).
+ */
+ protected Filter createAuthenticationFilter() {
+ return new AuthenticationFilter();
}
}
