From 37f3022a67d2304ae2271f9eb40687896b5d3fd3 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Wed, 5 Apr 2023 12:17:22 +0300 Subject: [PATCH] Sanitize the behavior of provided ExceptionMapper classes in dev-mode User provided ExceptionMapper classes now takes precedence over the Quarkus provided NotFoundExceptionMapper. This is done in a way that does not change anything but this specific behavior, everything else concerning exception handling continues to work as before. Note also that this does not change the spec compliance in any way Fixes: #7883 --- .../QuarkusDefaultExceptionHandlingTest.java | 64 +++++++++++++++++ .../UserProvidedExceptionHandlingTest.java | 72 +++++++++++++++++++ .../runtime/NotFoundExceptionMapper.java | 9 ++- .../server/core/RuntimeExceptionMapper.java | 16 ++++- 4 files changed, 156 insertions(+), 5 deletions(-) create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java create mode 100644 extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java new file mode 100644 index 0000000000000..194daf36bc05d --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/QuarkusDefaultExceptionHandlingTest.java @@ -0,0 +1,64 @@ +package io.quarkus.resteasy.reactive.server.test.devmode; + +import static org.hamcrest.CoreMatchers.containsString; + +import java.util.function.Supplier; + +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class QuarkusDefaultExceptionHandlingTest { + + @RegisterExtension + static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .setArchiveProducer(new Supplier<>() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(Resource.class); + } + + }); + + @Test + public void testDefaultErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception") + .then() + .statusCode(500) + .body(containsString("Internal Server Error"), containsString("dummy exception")); + } + + @Test + public void testNotFoundErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception2") + .then() + .statusCode(404) + .body(containsString("404 - Resource Not Found")); + } + + @Path("test") + @Produces(MediaType.TEXT_PLAIN) + @Consumes(MediaType.TEXT_PLAIN) + public static class Resource { + + @Path("exception") + @GET + @Produces("text/html") + public String exception() { + throw new RuntimeException("dummy exception"); + } + } +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java new file mode 100644 index 0000000000000..94091608508be --- /dev/null +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/devmode/UserProvidedExceptionHandlingTest.java @@ -0,0 +1,72 @@ +package io.quarkus.resteasy.reactive.server.test.devmode; + +import java.util.function.Supplier; + +import jakarta.ws.rs.Consumes; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.Produces; +import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; +import jakarta.ws.rs.ext.ExceptionMapper; +import jakarta.ws.rs.ext.Provider; + +import org.jboss.shrinkwrap.api.ShrinkWrap; +import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.test.QuarkusDevModeTest; +import io.restassured.RestAssured; + +public class UserProvidedExceptionHandlingTest { + + @RegisterExtension + static QuarkusDevModeTest TEST = new QuarkusDevModeTest() + .setArchiveProducer(new Supplier<>() { + @Override + public JavaArchive get() { + return ShrinkWrap.create(JavaArchive.class) + .addClasses(Resource.class, CustomExceptionMapper.class); + } + + }); + + @Test + public void testDefaultErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception") + .then() + .statusCode(999); + } + + @Test + public void testNotFoundErrorHandler() { + RestAssured.given().accept("text/html") + .get("/test/exception2") + .then() + .statusCode(999); + } + + @Path("test") + @Produces(MediaType.TEXT_PLAIN) + @Consumes(MediaType.TEXT_PLAIN) + public static class Resource { + + @Path("exception") + @GET + @Produces("text/html") + public String exception() { + throw new RuntimeException("dummy exception"); + } + } + + @Provider + public static class CustomExceptionMapper implements ExceptionMapper { + @Override + public Response toResponse(Exception exception) { + return Response.status(999).build(); + } + } + +} diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java index 6c9b447a175f1..d0277c57592b9 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/NotFoundExceptionMapper.java @@ -34,6 +34,7 @@ import org.jboss.logging.Logger; import org.jboss.resteasy.reactive.common.util.ServerMediaType; import org.jboss.resteasy.reactive.server.ServerExceptionMapper; +import org.jboss.resteasy.reactive.server.core.RuntimeExceptionMapper; import org.jboss.resteasy.reactive.server.core.request.ServerDrivenNegotiation; import org.jboss.resteasy.reactive.server.handlers.RestInitialHandler; import org.jboss.resteasy.reactive.server.mapping.RequestMapper; @@ -81,8 +82,12 @@ public MethodDescription(String method, String fullPath, String produces, String } } - @ServerExceptionMapper(value = NotFoundException.class, priority = Priorities.USER + 1) - public Response toResponse(HttpHeaders headers) { + // we don't use NotFoundExceptionMapper here because that would result in users not being able to provide their own catch-all exception mapper in dev-mode, see https://github.com/quarkusio/quarkus/issues/7883 + @ServerExceptionMapper(priority = Priorities.USER + 1) + public Response toResponse(Throwable t, HttpHeaders headers) { + if (!(t instanceof NotFoundException)) { + return RuntimeExceptionMapper.IGNORE_RESPONSE; + } if ((classMappers == null) || classMappers.isEmpty()) { return respond(headers); } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java index 79a94258232a3..08c61391ca895 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/RuntimeExceptionMapper.java @@ -20,6 +20,7 @@ import org.jboss.logging.Logger; import org.jboss.resteasy.reactive.ResteasyReactiveClientProblem; import org.jboss.resteasy.reactive.common.model.ResourceExceptionMapper; +import org.jboss.resteasy.reactive.server.jaxrs.ResponseBuilderImpl; import org.jboss.resteasy.reactive.server.mapping.RuntimeResource; import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveAsyncExceptionMapper; import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveExceptionMapper; @@ -42,6 +43,9 @@ public class RuntimeExceptionMapper { private final List> nonBlockingProblemPredicate; private final Set> unwrappedExceptions; + public static final Response IGNORE_RESPONSE = new ResponseBuilderImpl().status(666).header( + "RR_EX_IGN", "true").build(); + public RuntimeExceptionMapper(ExceptionMapping mapping, ClassLoader classLoader) { try { mappers = new HashMap<>(); @@ -94,9 +98,15 @@ public void mapException(Throwable throwable, ResteasyReactiveRequestContext con } else { response = exceptionMapper.toResponse(mappedException); } - context.setResult(response); - logBlockingErrorIfRequired(mappedException, context); - logNonBlockingErrorIfRequired(mappedException, context); + // this special case is used in order to ignore the mapping of built-in mappers and let the exception handling proceed to higher levels + if ((IGNORE_RESPONSE == response)) { + context.handleUnmappedException(throwable); + } else { + context.setResult(response); + logBlockingErrorIfRequired(mappedException, context); + logNonBlockingErrorIfRequired(mappedException, context); + + } return; } if (isWebApplicationException) {