Skip to content

Commit 2c6ed6f

Browse files
committed
Interrogate candidate resources/return types to stop REST server endpoint indexer from scanning REST clients
As detailed in #48464 certail methods cause the server endpoint indexer to scan _all_ REST client methods, which can lead to errors if any client method violates the requirements for resource methods. This fixes this in two ways: ### 1. Detect `java.lang.Object` returning methods and attempt further deduction. Since the crux of the issue is `java.lang.Object` infecting the candidate list (which causes any query of does X derive Y to always result in true) this step tries to reduce the false positives here. The only method I could implement was for Kotlin suspend methods, since by view of Java they always return `Object`. If the method is a suspend method we extract the real return type. Unfortunately while this reduces the pollution of the list dramatically, any method must be allowed to return `Object` (or Kotlin `Any`) which may dynamically return a sub-resource. ### 2. Check candidates for annotations from one of the Q-REST, MP-REST, or JAX-RS REST client pacakges. Since we can’t always ensure the return type list is “free” of `Object`, the candidates are filtered based on if the class or method uses any annotatoins from the spec client packages. Reasonably this _should_ filter the majority of client interfaces because they generally have at least one client annotation (e.g. `RegisterRestClient`). —- These issuees are tested were tested in our, fairly large set of projects, which all use Kotlin. and even include some methods returning `Any`. with no client interfaces being sent to the server indexer. This wont fully ensure all client interfaces won’t ever be scanned by the server indexer, but the likelihood has been reduced dramatically. To get a REST client scanned by the server indexer you would need to: 1. Write a Java method returning `java.lang.Object` or `Kotlin.Any`, this could be anywhere in the project. 2. Write an interface with no client specific annotations and register it programmatically. In this scenario, the liklihood of an issue arising is further reduced by the fact that errors are only raised when a method on the client interface fails to validate as a server method as well. From my investigation this seems very unlikely. E.g., in the REST client that triggered the error in our project, the method used an extra parameter annotated with `@NotBody` together with the `@ClientHeaders` annotation to add special client request headers. Now, the presence of either of thse annotations causes the interface to be filtered before being scanned.
1 parent 4435c0d commit 2c6ed6f

File tree

1 file changed

+38
-4
lines changed

1 file changed

+38
-4
lines changed

extensions/resteasy-reactive/rest/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@
132132
import io.quarkus.arc.deployment.BeanDiscoveryFinishedBuildItem;
133133
import io.quarkus.arc.deployment.GeneratedBeanBuildItem;
134134
import io.quarkus.arc.deployment.UnremovableBeanBuildItem;
135+
import io.quarkus.arc.processor.KotlinUtils;
135136
import io.quarkus.arc.runtime.BeanContainer;
136137
import io.quarkus.deployment.Capabilities;
137138
import io.quarkus.deployment.Capability;
@@ -739,9 +740,13 @@ public Supplier<Boolean> apply(ClassInfo classInfo) {
739740

740741
checkForDuplicateEndpoint(config, allServerMethods);
741742

742-
Function<Type, DotName> typeToReturnName = new Function<Type, DotName>() {
743+
Function<MethodInfo, DotName> methodToReturnName = new Function<MethodInfo, DotName>() {
743744
@Override
744-
public DotName apply(Type type) {
745+
public DotName apply(MethodInfo method) {
746+
var type = method.returnType();
747+
if (type.name().equals(DotName.OBJECT_NAME) && KotlinUtils.isKotlinSuspendMethod(method)) {
748+
type = KotlinUtils.getKotlinSuspendMethodResult(method);
749+
}
745750
DotName typeName = type.name();
746751
if (type.kind() == Type.Kind.CLASS) {
747752
return typeName;
@@ -757,6 +762,19 @@ public DotName apply(Type type) {
757762
}
758763
};
759764

765+
// Provides a predicate for filtering classes/methods that have annotations from one of the client
766+
// packages. This only reduces the false positives as a "base" interface could be derived and
767+
// client-related annotation applies. Although it seems unlikely that an endpoint without any
768+
// client annotations violates the specification for server resources methods; this type of false
769+
// positive would only mean needless processing as there would be no exception thrown.
770+
Predicate<AnnotationInstance> knownClientAnnotation = new Predicate<AnnotationInstance>() {
771+
public boolean test(AnnotationInstance ann) {
772+
return ann.name().packagePrefix().startsWith("io.quarkus.rest.client") ||
773+
ann.name().packagePrefix().startsWith("org.eclipse.microprofile.rest.client") ||
774+
ann.name().packagePrefix().startsWith("org.jboss.resteasy.reactive.client");
775+
}
776+
};
777+
760778
Map<DotName, Set<DotName>> returnsBySubResources = new HashMap<>();
761779
//now index possible sub resources. These are all classes that have method annotations
762780
//that are not annotated @Path
@@ -765,8 +783,16 @@ public DotName apply(Type type) {
765783
MethodInfo method = instance.target().asMethod();
766784
ClassInfo classInfo = method.declaringClass();
767785

786+
// Reject known client interfaces (See predicate above)
787+
if (classInfo.annotations().stream().anyMatch(knownClientAnnotation)
788+
|| method.annotations().stream().anyMatch(knownClientAnnotation)
789+
|| method.parameters().stream().flatMap(p -> p.annotations().stream())
790+
.anyMatch(knownClientAnnotation)) {
791+
continue;
792+
}
793+
768794
returnsBySubResources.computeIfAbsent(classInfo.name(), ignored -> new HashSet<>())
769-
.add(typeToReturnName.apply(method.returnType()));
795+
.add(methodToReturnName.apply(method));
770796
}
771797
}
772798
//sub resources can also have just a path annotation
@@ -776,8 +802,16 @@ public DotName apply(Type type) {
776802
MethodInfo method = instance.target().asMethod();
777803
ClassInfo classInfo = method.declaringClass();
778804

805+
// Reject known client interfaces (See predicate above)
806+
if (classInfo.annotations().stream().anyMatch(knownClientAnnotation)
807+
|| method.annotations().stream().anyMatch(knownClientAnnotation)
808+
|| method.parameters().stream().flatMap(p -> p.annotations().stream())
809+
.anyMatch(knownClientAnnotation)) {
810+
continue;
811+
}
812+
779813
returnsBySubResources.computeIfAbsent(classInfo.name(), ignored -> new HashSet<>())
780-
.add(typeToReturnName.apply(method.returnType()));
814+
.add(methodToReturnName.apply(method));
781815
}
782816
}
783817

0 commit comments

Comments
 (0)