Skip to content

Commit

Permalink
Prevent non-public classes of method params from breaking interceptor…
Browse files Browse the repository at this point in the history
… handling

Fixes: quarkusio#18477
  • Loading branch information
geoand committed Jul 8, 2021
1 parent 80a04f0 commit fc56f80
Show file tree
Hide file tree
Showing 7 changed files with 125 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,8 @@ private Map<MethodInfo, DecorationInfo> initDecoratedMethods() {

Map<MethodKey, DecorationInfo> candidates = new HashMap<>();
addDecoratedMethods(candidates, target.get().asClass(), bound,
new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom));
new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom,
beanDeployment.getBeanArchiveIndex()));

Map<MethodInfo, DecorationInfo> decoratedMethods = new HashMap<>(candidates.size());
for (Entry<MethodKey, DecorationInfo> entry : candidates.entrySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,9 @@ static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment beanDeploym
boolean transformUnproxyableClasses) {
return addInterceptedMethodCandidates(beanDeployment, classInfo, candidates, classLevelBindings,
bytecodeTransformerConsumer, transformUnproxyableClasses,
new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom), false);
new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom,
beanDeployment.getBeanArchiveIndex()),
false);
}

static Set<MethodInfo> addInterceptedMethodCandidates(BeanDeployment beanDeployment, ClassInfo classInfo,
Expand Down Expand Up @@ -454,12 +456,14 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str
static class SubclassSkipPredicate implements Predicate<MethodInfo> {

private final BiFunction<Type, Type, Boolean> assignableFromFun;
private final IndexView beanArchiveIndex;
private ClassInfo clazz;
private List<MethodInfo> regularMethods;
private Set<MethodInfo> bridgeMethods = new HashSet<>();

public SubclassSkipPredicate(BiFunction<Type, Type, Boolean> assignableFromFun) {
public SubclassSkipPredicate(BiFunction<Type, Type, Boolean> assignableFromFun, IndexView beanArchiveIndex) {
this.assignableFromFun = assignableFromFun;
this.beanArchiveIndex = beanArchiveIndex;
}

void startProcessing(ClassInfo clazz) {
Expand Down Expand Up @@ -505,6 +509,20 @@ public boolean test(MethodInfo method) {
// Do not skip default methods - public non-abstract instance methods declared in an interface
return false;
}

List<Type> parameters = method.parameters();
if (!parameters.isEmpty() && (beanArchiveIndex != null)) {
for (Type type : parameters) {
ClassInfo parameterClassInfo = beanArchiveIndex.getClassByName(type.name());
if (parameterClassInfo == null) {
continue; // hope for the best
}
if (!Modifier.isPublic(parameterClassInfo.flags())) {
return true; // if the parameter is not public, we end up with IllegalAccessError when trying to access the use the load the class
}
}
}

// Note that we intentionally do not skip final methods here - these are handled later
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public class SubclassSkipPredicateTest {
public void testPredicate() throws IOException {
IndexView index = Basics.index(Base.class, Submarine.class, Long.class, Number.class);
AssignabilityCheck assignabilityCheck = new AssignabilityCheck(index, null);
SubclassSkipPredicate predicate = new SubclassSkipPredicate(assignabilityCheck::isAssignableFrom);
SubclassSkipPredicate predicate = new SubclassSkipPredicate(assignabilityCheck::isAssignableFrom, null);

ClassInfo submarineClass = index.getClassByName(DotName.createSimple(Submarine.class.getName()));
predicate.startProcessing(submarineClass);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
package io.quarkus.arc.test.interceptors.methodargs;

import java.util.List;
import javax.inject.Singleton;

@Singleton
@Simple
public class ExampleResource {

// Just to try to confuse our bridge method impl. algorithm
public String create(List<String> strings) {
return String.join(",", strings);
}

String otherCreate(NotPublic notPublic) {
return notPublic.toString();
}

static class NotPublic {

@Override
public String toString() {
return "NotPublic{}";
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.quarkus.arc.test.interceptors.methodargs;

import static org.junit.jupiter.api.Assertions.assertEquals;

import io.quarkus.arc.Arc;
import io.quarkus.arc.ArcContainer;
import io.quarkus.arc.test.ArcTestContainer;
import io.quarkus.arc.test.interceptors.Counter;
import java.util.List;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

public class MethodArgsInterceptionTest {

@RegisterExtension
public ArcTestContainer container = new ArcTestContainer(Simple.class, SimpleInterceptor.class, ExampleResource.class,
Counter.class);

@Test
public void testInterception() {
ArcContainer container = Arc.container();
Counter counter = container.instance(Counter.class).get();

counter.reset();
ExampleResource exampleResource = container.instance(ExampleResource.class).get();

assertEquals("first,second", exampleResource.create(List.of("first", "second")));
assertEquals(1, counter.get());

assertEquals("NotPublic{}", exampleResource.otherCreate(new ExampleResource.NotPublic()));
assertEquals(1, counter.get()); // the interceptor should not have been called because the method argument is not public
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package io.quarkus.arc.test.interceptors.methodargs;

import static java.lang.annotation.ElementType.METHOD;
import static java.lang.annotation.ElementType.TYPE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import java.lang.annotation.Documented;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import javax.interceptor.InterceptorBinding;

@Target({ TYPE, METHOD })
@Retention(RUNTIME)
@Documented
@InterceptorBinding
public @interface Simple {

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package io.quarkus.arc.test.interceptors.methodargs;

import io.quarkus.arc.test.interceptors.Counter;
import javax.annotation.Priority;
import javax.inject.Inject;
import javax.interceptor.AroundInvoke;
import javax.interceptor.Interceptor;
import javax.interceptor.InvocationContext;

@Simple
@Priority(1)
@Interceptor
public class SimpleInterceptor {

@Inject
Counter counter;

@AroundInvoke
Object mySuperCoolAroundInvoke(InvocationContext ctx) throws Exception {
counter.incrementAndGet();
return ctx.proceed();
}
}

0 comments on commit fc56f80

Please sign in to comment.