Skip to content

Commit

Permalink
Sanitize the behavior of provided ExceptionMapper classes in dev-mode
Browse files Browse the repository at this point in the history
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: quarkusio#7883
  • Loading branch information
geoand authored and gastaldi committed Apr 13, 2023
1 parent 7ea661f commit d96f118
Show file tree
Hide file tree
Showing 4 changed files with 162 additions and 5 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 javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.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 javax.ws.rs.Consumes;
import javax.ws.rs.GET;
import javax.ws.rs.Path;
import javax.ws.rs.Produces;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import javax.ws.rs.ext.ExceptionMapper;
import javax.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 @@ -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 d96f118

Please sign in to comment.