Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitize the behavior of provided ExceptionMapper classes in dev-mode #32425

Merged
merged 2 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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