Skip to content

Commit

Permalink
Allow request filters to be run after the input has been read
Browse files Browse the repository at this point in the history
Resolves: #22209
  • Loading branch information
geoand committed Dec 15, 2021
1 parent 39e2e8b commit 65f0a3d
Show file tree
Hide file tree
Showing 11 changed files with 180 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ protected <T, B extends AbstractInterceptorBuildItem> void registerInterceptors(
}
if (filterItem instanceof ContainerRequestFilterBuildItem) {
interceptor.setNonBlockingRequired(((ContainerRequestFilterBuildItem) filterItem).isNonBlockingRequired());
interceptor.setReadBody(((ContainerRequestFilterBuildItem) filterItem).isReadBody());
}
if (interceptors instanceof PreMatchInterceptorContainer
&& ((ContainerRequestFilterBuildItem) filterItem).isPreMatching()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,20 @@ public final class ContainerRequestFilterBuildItem extends AbstractInterceptorBu

private final boolean preMatching;
private final boolean nonBlockingRequired;
private final boolean readBody;

protected ContainerRequestFilterBuildItem(Builder builder) {
super(builder);
this.preMatching = builder.preMatching;
this.nonBlockingRequired = builder.nonBlockingRequired;
this.readBody = builder.readBody;
}

public ContainerRequestFilterBuildItem(String className) {
super(className);
this.preMatching = false;
this.nonBlockingRequired = false;
this.readBody = false;
}

public boolean isPreMatching() {
Expand All @@ -25,9 +28,14 @@ public boolean isNonBlockingRequired() {
return nonBlockingRequired;
}

public boolean isReadBody() {
return readBody;
}

public static final class Builder extends AbstractInterceptorBuildItem.Builder<ContainerRequestFilterBuildItem, Builder> {
boolean preMatching = false;
boolean nonBlockingRequired = false;
boolean readBody = false;

public Builder(String className) {
super(className);
Expand All @@ -43,6 +51,11 @@ public Builder setNonBlockingRequired(boolean nonBlockingRequired) {
return this;
}

public Builder setReadBody(boolean readBody) {
this.readBody = readBody;
return this;
}

public ContainerRequestFilterBuildItem build() {
return new ContainerRequestFilterBuildItem(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,8 @@ public void handleCustomAnnotatedMethods(
.setRegisterAsBean(false) // it has already been made a bean
.setPriority(generated.getPriority())
.setPreMatching(generated.isPreMatching())
.setNonBlockingRequired(generated.isNonBlocking());
.setNonBlockingRequired(generated.isNonBlocking())
.setReadBody(generated.isReadBody());
if (!generated.getNameBindingNames().isEmpty()) {
builder.setNameBindingNames(generated.getNameBindingNames());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package io.quarkus.resteasy.reactive.server.test.customproviders;

import java.util.function.Supplier;

import javax.ws.rs.GET;
import javax.ws.rs.POST;
import javax.ws.rs.Path;
import javax.ws.rs.core.HttpHeaders;

import org.hamcrest.Matchers;
import org.jboss.resteasy.reactive.RestForm;
import org.jboss.resteasy.reactive.RestQuery;
import org.jboss.resteasy.reactive.server.ServerRequestFilter;
import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext;
import org.jboss.resteasy.reactive.server.spi.ResteasyReactiveContainerRequestContext;
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.QuarkusUnitTest;
import io.restassured.RestAssured;

public class ReadBodyRequestFilterTest {

@RegisterExtension
static QuarkusUnitTest test = new QuarkusUnitTest()
.setArchiveProducer(new Supplier<>() {
@Override
public JavaArchive get() {
return ShrinkWrap.create(JavaArchive.class)
.addClasses(HelloResource.class);
}
});

@Test
public void testMethodWithBody() {
RestAssured.with()
.formParam("name", "Quarkus")
.post("/hello")
.then().body(Matchers.equalTo("hello Quarkus!!!!!!!"));
}

@Test
public void testMethodWithoutBody() {
RestAssured.with()
.queryParam("name", "Quarkus")
.get("/hello")
.then().body(Matchers.equalTo("hello Quarkus!"));
}

@Path("hello")
public static class HelloResource {

@POST
public String helloPost(@RestForm String name, HttpHeaders headers) {
return "hello " + name + headers.getHeaderString("suffix");
}

@GET
public String helloGet(@RestQuery String name, HttpHeaders headers) {
return "hello " + name + headers.getHeaderString("suffix");
}
}

public static class Filters {

@ServerRequestFilter(readBody = true)
public void addSuffix(ResteasyReactiveContainerRequestContext containerRequestContext) {
ResteasyReactiveRequestContext rrContext = (ResteasyReactiveRequestContext) containerRequestContext
.getServerRequestContext();
if (containerRequestContext.getMethod().equals("POST")) {
String nameFormParam = (String) rrContext.getFormParameter("name", true, false);
containerRequestContext.getHeaders().putSingle("suffix", "!".repeat(nameFormParam.length()));
} else {
containerRequestContext.getHeaders().putSingle("suffix", "!");
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ public class ResourceInterceptor<T>
private BeanFactory<T> factory;
private int priority = Priorities.USER; // default priority as defined by spec
private boolean nonBlockingRequired; // whether or not @NonBlocking was specified on the class
private boolean readBody; // whether or not 'readBody' was set true for this filter

/**
* The class names of the {@code @NameBinding} annotations that the method is annotated with.
Expand Down Expand Up @@ -68,6 +69,14 @@ public void setNonBlockingRequired(boolean nonBlockingRequired) {
this.nonBlockingRequired = nonBlockingRequired;
}

public boolean isReadBody() {
return readBody;
}

public void setReadBody(boolean readBody) {
this.readBody = readBody;
}

// spec says that writer interceptors are sorted in ascending order
@Override
public int compareTo(ResourceInterceptor<T> o) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ public static List<GeneratedFilter> generate(IndexView index, Set<DotName> unwra
Integer priority = null;
boolean preMatching = false;
boolean nonBlockingRequired = false;
boolean readBody = false;
Set<String> nameBindingNames = new HashSet<>();

AnnotationValue priorityValue = instance.value("priority");
Expand All @@ -48,6 +49,16 @@ public static List<GeneratedFilter> generate(IndexView index, Set<DotName> unwra
if (nonBlockingRequiredValue != null) {
nonBlockingRequired = nonBlockingRequiredValue.asBoolean();
}
AnnotationValue readBodyValue = instance.value("readBody");
if (readBodyValue != null) {
readBody = readBodyValue.asBoolean();
}

if (readBody && preMatching) {
throw new IllegalStateException(
"Setting both 'readBody' and 'preMatching' to 'true' on '@ServerRequestFilter' is invalid. Offending method is '"
+ methodInfo.name() + "' of class '" + methodInfo.declaringClass().name() + "'");
}

List<AnnotationInstance> annotations = methodInfo.annotations();
for (AnnotationInstance annotation : annotations) {
Expand All @@ -65,7 +76,7 @@ public static List<GeneratedFilter> generate(IndexView index, Set<DotName> unwra
}

ret.add(new GeneratedFilter(output.getOutput(), generatedClassName, methodInfo.declaringClass().name().toString(),
true, priority, preMatching, nonBlockingRequired, nameBindingNames));
true, priority, preMatching, nonBlockingRequired, nameBindingNames, readBody));
}
for (AnnotationInstance instance : index
.getAnnotations(SERVER_RESPONSE_FILTER)) {
Expand Down Expand Up @@ -99,7 +110,7 @@ public static List<GeneratedFilter> generate(IndexView index, Set<DotName> unwra
}

ret.add(new GeneratedFilter(output.getOutput(), generatedClassName, methodInfo.declaringClass().name().toString(),
false, priority, false, false, nameBindingNames));
false, priority, false, false, nameBindingNames, false));

}
return ret;
Expand All @@ -114,10 +125,12 @@ public static class GeneratedFilter {
final boolean preMatching;
final boolean nonBlocking;
final Set<String> nameBindingNames;
final boolean readBody;

public GeneratedFilter(List<GeneratedClass> generatedClasses, String generatedClassName, String declaringClassName,
public GeneratedFilter(List<GeneratedClass> generatedClasses, String generatedClassName,
String declaringClassName,
boolean requestFilter, Integer priority, boolean preMatching, boolean nonBlocking,
Set<String> nameBindingNames) {
Set<String> nameBindingNames, boolean readBody) {
this.generatedClasses = generatedClasses;
this.generatedClassName = generatedClassName;
this.declaringClassName = declaringClassName;
Expand All @@ -126,6 +139,7 @@ public GeneratedFilter(List<GeneratedClass> generatedClasses, String generatedCl
this.preMatching = preMatching;
this.nonBlocking = nonBlocking;
this.nameBindingNames = nameBindingNames;
this.readBody = readBody;
}

public String getGeneratedClassName() {
Expand Down Expand Up @@ -159,6 +173,10 @@ public List<GeneratedClass> getGeneratedClasses() {
public Set<String> getNameBindingNames() {
return nameBindingNames;
}

public boolean isReadBody() {
return readBody;
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@
int priority() default Priorities.USER;

/**
* Whether or not the filter is a pre-matching filter
* Whether the filter is a pre-matching filter
*
* Note that this setting and {@link ServerRequestFilter#readBody()} cannot be both set to true.
*/
boolean preMatching() default false;

Expand All @@ -99,4 +101,15 @@
* For this to work, this filter must be run before any of the filters when non-blocking is not required.
*/
boolean nonBlocking() default false;

/**
* If set to {@code true}, the filter will be run after the body has been fully read but before any deserialization takes
* place.
*
* Note that this change only affects Resource Methods that do result in reading the message body. For all other
* Resource Methods that the filter applies to, it will be executed in normal fashion.
*
* Also note that this setting and {@link ServerRequestFilter#preMatching()} cannot be both set to true.
*/
boolean readBody() default false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,8 @@ public BeanFactory.BeanInstance<?> apply(Class<?> aClass) {
.getPreMatchContainerRequestFilters()
.entrySet()) {
preMatchHandlers
.add(new ResourceRequestFilterHandler(entry.getValue(), true, entry.getKey().isNonBlockingRequired()));
.add(new ResourceRequestFilterHandler(entry.getValue(), true, entry.getKey().isNonBlockingRequired(),
entry.getKey().isReadBody()));
}
}
for (int i = 0; i < info.getGlobalHandlerCustomizers().size(); i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ public RuntimeInterceptorDeployment(DeploymentInfo info, ConfigurationImpl confi
for (Map.Entry<ResourceInterceptor<ContainerRequestFilter>, ContainerRequestFilter> entry : globalRequestInterceptorsMap
.entrySet()) {
globalRequestInterceptorHandlers
.add(new ResourceRequestFilterHandler(entry.getValue(), false, entry.getKey().isNonBlockingRequired()));
.add(new ResourceRequestFilterHandler(entry.getValue(), false, entry.getKey().isNonBlockingRequired(),
entry.getKey().isReadBody()));
}

InterceptorHandler globalInterceptorHandler = null;
Expand Down Expand Up @@ -326,7 +327,8 @@ public List<ResourceRequestFilterHandler> setupRequestFilterHandler() {
for (Map.Entry<ResourceInterceptor<ContainerRequestFilter>, ContainerRequestFilter> entry : interceptorsToUse
.entrySet()) {
handlers.add(
new ResourceRequestFilterHandler(entry.getValue(), false, entry.getKey().isNonBlockingRequired()));
new ResourceRequestFilterHandler(entry.getValue(), false, entry.getKey().isNonBlockingRequired(),
entry.getKey().isNonBlockingRequired()));
}
}
return handlers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ public class RuntimeResourceDeployment {
@SuppressWarnings("rawtypes")
private static final MessageBodyWriter[] EMPTY_MESSAGE_BODY_WRITERS = new MessageBodyWriter[0];

private static final int HANDLERS_CAPACITY = 10;

private static final Logger log = Logger.getLogger(RuntimeResourceDeployment.class);

private final DeploymentInfo info;
Expand Down Expand Up @@ -182,7 +184,7 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz,
//we want interceptors in the abort handler chain
List<ServerRestHandler> abortHandlingChain = new ArrayList<>(3 + (interceptorHandler != null ? 1 : 0));

List<ServerRestHandler> handlers = new ArrayList<>(10);
List<ServerRestHandler> handlers = new ArrayList<>(HANDLERS_CAPACITY);
// we add null as the first item to make sure that subsequent items are added in the proper positions
// and that the items don't need to shifted when at the end of the method we set the
// first item
Expand Down Expand Up @@ -216,6 +218,7 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz,

//spec doesn't seem to test this, but RESTEasy does not run request filters for both root and sub resources (which makes sense)
//so only run request filters for methods that are leaf resources - i.e. have a HTTP method annotation so we ensure only one will run
boolean hasReadBodyRequestFilters = false;
if (method.getHttpMethod() != null) {
List<ResourceRequestFilterHandler> containerRequestFilterHandlers = interceptorDeployment
.setupRequestFilterHandler();
Expand All @@ -230,6 +233,9 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz,
} else {
handlers.add(handler);
}
if (handler.isReadBody()) {
hasReadBodyRequestFilters = true;
}
}
} else {
handlers.addAll(containerRequestFilterHandlers);
Expand All @@ -250,17 +256,35 @@ public RuntimeResource buildResourceMethod(ResourceClass clazz,
}
}
// form params can be everywhere (field, beanparam, param)
boolean checkReadBodyRequestFilters = false;
if (method.isFormParamRequired() || method.isMultipart()) {
// read the body as multipart in one go
handlers.add(new FormBodyHandler(bodyParameter != null, executorSupplier));
checkReadBodyRequestFilters = true;
} else if (bodyParameter != null) {
if (!defaultBlocking) {
if (!method.isBlocking()) {
// allow the body to be read by chunks
handlers.add(new InputHandler(quarkusRestConfig.getInputBufferSize(), executorSupplier));
checkReadBodyRequestFilters = true;
}
}
}
if (checkReadBodyRequestFilters && hasReadBodyRequestFilters) {
// we need to remove the corresponding filters from the handlers list and add them to its end in the same order
List<ServerRestHandler> readBodyRequestFilters = new ArrayList<>(1);
for (int i = handlers.size() - 2; i >= 0; i--) {
var serverRestHandler = handlers.get(i);
if (serverRestHandler instanceof ResourceRequestFilterHandler) {
ResourceRequestFilterHandler resourceRequestFilterHandler = (ResourceRequestFilterHandler) serverRestHandler;
if (resourceRequestFilterHandler.isReadBody()) {
readBodyRequestFilters.add(handlers.remove(i));
}
}
}
handlers.addAll(readBodyRequestFilters);
}

// if we need the body, let's deserialize it
if (bodyParameter != null) {
Class<Object> typeClass = loadClass(bodyParameter.declaredType);
Expand Down
Loading

0 comments on commit 65f0a3d

Please sign in to comment.