From f1bbbd2ff265d18eee67886f2386e1b7b4f8e8b0 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 31 Oct 2025 11:02:15 +0100 Subject: [PATCH 1/3] refactor(api): unify JSON exception handling with abstract handler #11717 - Replaced `JsonParseExceptionHandler` with `JsonExceptionsHandler` to streamline exception handling - Added mappers for `JsonParsingException` and `DvUtilJsonParseException` --- .../errorhandlers/JsonExceptionsHandler.java | 52 +++++++++++++++++++ .../JsonParseExceptionHandler.java | 34 ------------ 2 files changed, 52 insertions(+), 34 deletions(-) create mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonExceptionsHandler.java delete mode 100644 src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonParseExceptionHandler.java diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonExceptionsHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonExceptionsHandler.java new file mode 100644 index 00000000000..892a0585419 --- /dev/null +++ b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonExceptionsHandler.java @@ -0,0 +1,52 @@ +package edu.harvard.iq.dataverse.api.errorhandlers; + +import edu.harvard.iq.dataverse.api.util.JsonResponseBuilder; +import edu.harvard.iq.dataverse.util.json.JsonParseException; +import jakarta.json.stream.JsonParsingException; +import jakarta.servlet.http.HttpServletRequest; +import jakarta.ws.rs.core.Context; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.ext.ExceptionMapper; +import jakarta.ws.rs.ext.Provider; + +import java.util.logging.Level; +import java.util.logging.Logger; + +/** + * Make a failing JSON parsing request appear to be a BadRequest (error code 400) + * and send a message what just failed... + */ +public abstract class JsonExceptionsHandler implements ExceptionMapper{ + + static final Logger logger = Logger.getLogger(JsonExceptionsHandler.class.getSimpleName()); + + @Context + HttpServletRequest request; + + @Override + public Response toResponse(T ex) { + return JsonResponseBuilder.error(Response.Status.BAD_REQUEST) + .log(logger, Level.FINER) + .message(ex.getMessage()) + .build(); + } + + /** + * Handler for jakarta.json.stream.JsonParsingException + */ + @Provider + public static class JsonParsingExceptionMapper extends JsonExceptionsHandler { + } + + /** + * Handler for jakarta.json.stream.JsonParsingException + */ + @Provider + public static class DvUtilJsonParseExceptionMapper extends JsonExceptionsHandler { + } + + // Add more handlers as needed (e.g., for Jackson, GSON, etc.) + // @Provider + // public static class JsonProcessingExceptionMapper extends JsonExceptionHandler { + // } +} diff --git a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonParseExceptionHandler.java b/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonParseExceptionHandler.java deleted file mode 100644 index 2f974a1c5be..00000000000 --- a/src/main/java/edu/harvard/iq/dataverse/api/errorhandlers/JsonParseExceptionHandler.java +++ /dev/null @@ -1,34 +0,0 @@ -package edu.harvard.iq.dataverse.api.errorhandlers; - -import edu.harvard.iq.dataverse.util.json.JsonParseException; - -import jakarta.json.Json; -import jakarta.servlet.http.HttpServletRequest; -import jakarta.ws.rs.BadRequestException; -import jakarta.ws.rs.core.Context; -import jakarta.ws.rs.core.MediaType; -import jakarta.ws.rs.core.Response; -import jakarta.ws.rs.ext.ExceptionMapper; -import jakarta.ws.rs.ext.Provider; - -/** - * Make a failing JSON parsing request appear to be a BadRequest (error code 400) - * and send a message what just failed... - */ -@Provider -public class JsonParseExceptionHandler implements ExceptionMapper{ - - @Context - HttpServletRequest request; - - @Override - public Response toResponse(JsonParseException ex){ - return Response.status(Response.Status.BAD_REQUEST) - .entity( Json.createObjectBuilder() - .add("status", "ERROR") - .add("code", Response.Status.BAD_REQUEST.getStatusCode()) - .add("message", ex.getMessage()) - .build()) - .type(MediaType.APPLICATION_JSON_TYPE).build(); - } -} From 5e793d0b25211fe9aad4f50f65c9ea93391c14cf Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 31 Oct 2025 11:09:42 +0100 Subject: [PATCH 2/3] test(api): add test case for invalid JSON in user creation API #11717 - Ensured API returns `BAD_REQUEST` for malformed JSON input with appropriate error message validation - Makes sure invalid JSON is not resulting in a HTML error page --- .../edu/harvard/iq/dataverse/api/AdminIT.java | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java b/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java index 6f3ffaa83b8..0313c460816 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/AdminIT.java @@ -19,7 +19,6 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; - import java.io.IOException; import java.nio.file.Files; import java.nio.file.Paths; @@ -29,7 +28,6 @@ import java.util.Map; import java.util.UUID; import java.util.logging.Logger; - import static io.restassured.RestAssured.given; import static jakarta.ws.rs.core.Response.Status.BAD_REQUEST; import static jakarta.ws.rs.core.Response.Status.CREATED; @@ -43,6 +41,7 @@ import static org.hamcrest.CoreMatchers.notNullValue; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import io.restassured.http.ContentType; import static org.junit.jupiter.api.Assertions.assertTrue; public class AdminIT { @@ -681,6 +680,19 @@ public void testCreateNonBuiltinUserViaApi() { assertEquals(200, deleteUserToConvert.getStatusCode()); } + + @Test + void testCreateUserViaAPI_WithInvalidJson() { + Response response = given() + .body("{invalid}") + .contentType(ContentType.JSON) + .post("/api/admin/authenticatedUsers"); + + response.then() + .assertThat() + .statusCode(BAD_REQUEST.getStatusCode()) + .body("message", containsString("Unexpected char")); + } @Test From b9191207f687c98dbd8cd5f5280788588bdc33f8 Mon Sep 17 00:00:00 2001 From: Oliver Bertuch Date: Fri, 31 Oct 2025 16:44:41 +0100 Subject: [PATCH 3/3] fix(api): ensure JsonObjectBuilder consistency during logging and response creation #11717 - Cloned JsonObjectBuilder to prevent unintended mutation - Simplified metadata construction logic in logging - This should allow us to no longer have HTML Error 500 pages, but a nicer JSON response. --- .../dataverse/api/util/JsonResponseBuilder.java | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java index 9095a40c608..287a99270e9 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/util/JsonResponseBuilder.java @@ -1,6 +1,7 @@ package edu.harvard.iq.dataverse.api.util; import jakarta.json.Json; +import jakarta.json.JsonObject; import jakarta.json.JsonValue; import jakarta.json.JsonObjectBuilder; import jakarta.servlet.ServletResponse; @@ -143,8 +144,9 @@ public JsonResponseBuilder internalError(Throwable ex) { * @return JAX-RS response including JSON message */ public Response build() { + JsonObject entity = entityBuilder.build(); return jerseyResponseBuilder.type(MediaType.APPLICATION_JSON_TYPE) - .entity(this.entityBuilder.build()) + .entity(entity) .build(); } @@ -215,9 +217,14 @@ public JsonResponseBuilder log(Logger logger, Level level, Optional e if ( ! logger.isLoggable(level) || alreadyLogged ) return this; - StringBuilder metadata = new StringBuilder(""); - this.entityBuilder.build() - .forEach((k,v) -> metadata.append("_"+k+"="+v.toString()+";")); + // This is necessary because we need to build in two places: logging and response creation. + // Without cloning the object builder, we'd end up with an empty entity in the response when logging before that. + JsonObject entity = this.entityBuilder.build(); + this.entityBuilder = Json.createObjectBuilder(entity); + + StringBuilder metadata = new StringBuilder(); + entity.forEach((k,v) -> metadata.append("_").append(k).append("=").append(v.toString()).append(";")); + // remove trailing ; metadata.deleteCharAt(metadata.length()-1);