Skip to content

Commit

Permalink
Merge pull request #32425 from geoand/#7883
Browse files Browse the repository at this point in the history
Sanitize the behavior of provided ExceptionMapper classes in dev-mode
  • Loading branch information
geoand authored Apr 6, 2023
2 parents 647676b + e631d34 commit 1f6affe
Show file tree
Hide file tree
Showing 5 changed files with 166 additions and 10 deletions.
Original file line number Diff line number Diff line change
@@ -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");
}
}
}
Original file line number Diff line number Diff line change
@@ -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<Exception> {
@Override
public Response toResponse(Exception exception) {
return Response.status(999).build();
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,16 @@
/**
* When used on a method, then an implementation of {@link jakarta.ws.rs.ext.ExceptionMapper} is generated
* that calls the annotated method with the proper arguments.
*
* <p>
* When the annotation is placed on a method that is not a JAX-RS Resource class, the method handles exceptions in global
* fashion
* (as do regular JAX-RS {@code ExceptionMapper} implementations).
* fashion (as do regular JAX-RS {@code ExceptionMapper} implementations).
* However, when it is placed on a method of a JAX-RS Resource class, the method is only used to handle exceptions originating
* from
* that JAX-RS Resource class.
* Methods in a JAX-RS class annotated with this annotation will be used first when determining how to handle a thrown
* exception.
* This means that these methods take precedence over the global {@link jakarta.ws.rs.ext.ExceptionMapper} classes.
*
* <p>
* In addition to the exception being handled, an annotated method can also declare any of the following
* parameters (in any order):
* <ul>
Expand All @@ -39,7 +38,7 @@
*
* When {@code value} is not set, then the handled Exception type is deduced by the Exception type used in the method parameters
* (there must be exactly one Exception type in this case).
*
* <p>
* The return type of the method must be either be of type {@code Response}, {@code Uni<Response>}, {@code RestResponse} or
* {@code Uni<RestResponse>}.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -42,6 +43,9 @@ public class RuntimeExceptionMapper {
private final List<Predicate<Throwable>> nonBlockingProblemPredicate;
private final Set<Class<? extends Throwable>> 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<>();
Expand Down Expand Up @@ -94,9 +98,21 @@ 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)) {
if (isWebApplicationException) {
context.setResult(((WebApplicationException) throwable).getResponse());
log.debug("Application failed the request", throwable);
} else {
logBlockingErrorIfRequired(throwable, context);
logNonBlockingErrorIfRequired(throwable, context);
context.handleUnmappedException(throwable);
}
} else {
context.setResult(response);
logBlockingErrorIfRequired(mappedException, context);
logNonBlockingErrorIfRequired(mappedException, context);
}
return;
}
if (isWebApplicationException) {
Expand Down

0 comments on commit 1f6affe

Please sign in to comment.