Browse Source

EventListenerMethodProcessor does not validate target classes behind proxies anymore

Issue: SPR-13526
Issue: SPR-13538
pull/883/head
Juergen Hoeller 10 years ago
parent
commit
dbec2121a0
  1. 80
      spring-context/src/main/java/org/springframework/context/event/EventListenerMethodProcessor.java
  2. 35
      spring-context/src/test/java/org/springframework/context/event/AnnotationDrivenEventListenerTests.java

80
spring-context/src/main/java/org/springframework/context/event/EventListenerMethodProcessor.java

@ -28,9 +28,7 @@ import java.util.concurrent.ConcurrentHashMap;
import org.apache.commons.logging.Log; import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory; import org.apache.commons.logging.LogFactory;
import org.springframework.aop.SpringProxy;
import org.springframework.aop.scope.ScopedProxyUtils; import org.springframework.aop.scope.ScopedProxyUtils;
import org.springframework.aop.support.AopUtils;
import org.springframework.beans.BeansException; import org.springframework.beans.BeansException;
import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.BeanInitializationException;
import org.springframework.beans.factory.SmartInitializingSingleton; import org.springframework.beans.factory.SmartInitializingSingleton;
@ -48,6 +46,7 @@ import org.springframework.util.ReflectionUtils;
* instances. * instances.
* *
* @author Stephane Nicoll * @author Stephane Nicoll
* @author Juergen Hoeller
* @since 4.2 * @since 4.2
*/ */
public class EventListenerMethodProcessor implements SmartInitializingSingleton, ApplicationContextAware { public class EventListenerMethodProcessor implements SmartInitializingSingleton, ApplicationContextAware {
@ -61,6 +60,7 @@ public class EventListenerMethodProcessor implements SmartInitializingSingleton,
private final Set<Class<?>> nonAnnotatedClasses = private final Set<Class<?>> nonAnnotatedClasses =
Collections.newSetFromMap(new ConcurrentHashMap<Class<?>, Boolean>(64)); Collections.newSetFromMap(new ConcurrentHashMap<Class<?>, Boolean>(64));
@Override @Override
public void setApplicationContext(ApplicationContext applicationContext) throws BeansException { public void setApplicationContext(ApplicationContext applicationContext) throws BeansException {
Assert.isTrue(applicationContext instanceof ConfigurableApplicationContext, Assert.isTrue(applicationContext instanceof ConfigurableApplicationContext,
@ -69,18 +69,6 @@ public class EventListenerMethodProcessor implements SmartInitializingSingleton,
} }
/**
* Return the {@link EventListenerFactory} instances to use to handle {@link EventListener}
* annotated methods.
*/
protected List<EventListenerFactory> getEventListenerFactories() {
Map<String, EventListenerFactory> beans =
this.applicationContext.getBeansOfType(EventListenerFactory.class);
List<EventListenerFactory> allFactories = new ArrayList<EventListenerFactory>(beans.values());
AnnotationAwareOrderComparator.sort(allFactories);
return allFactories;
}
@Override @Override
public void afterSingletonsInstantiated() { public void afterSingletonsInstantiated() {
List<EventListenerFactory> factories = getEventListenerFactories(); List<EventListenerFactory> factories = getEventListenerFactories();
@ -91,16 +79,27 @@ public class EventListenerMethodProcessor implements SmartInitializingSingleton,
try { try {
processBean(factories, beanName, type); processBean(factories, beanName, type);
} }
catch (RuntimeException e) { catch (Throwable ex) {
throw new BeanInitializationException("Failed to process @EventListener " + throw new BeanInitializationException("Failed to process @EventListener " +
"annotation on bean with name '" + beanName + "'", e); "annotation on bean with name '" + beanName + "'", ex);
} }
} }
} }
} }
protected void processBean(List<EventListenerFactory> factories, String beanName, final Class<?> type) { /**
Class<?> targetType = getTargetClass(beanName, type); * Return the {@link EventListenerFactory} instances to use to handle {@link EventListener}
* annotated methods.
*/
protected List<EventListenerFactory> getEventListenerFactories() {
Map<String, EventListenerFactory> beans =
this.applicationContext.getBeansOfType(EventListenerFactory.class);
List<EventListenerFactory> allFactories = new ArrayList<EventListenerFactory>(beans.values());
AnnotationAwareOrderComparator.sort(allFactories);
return allFactories;
}
protected void processBean(List<EventListenerFactory> factories, String beanName, final Class<?> targetType) {
if (!this.nonAnnotatedClasses.contains(targetType)) { if (!this.nonAnnotatedClasses.contains(targetType)) {
final Set<Method> annotatedMethods = new LinkedHashSet<Method>(1); final Set<Method> annotatedMethods = new LinkedHashSet<Method>(1);
Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(targetType); Method[] methods = ReflectionUtils.getUniqueDeclaredMethods(targetType);
@ -111,13 +110,10 @@ public class EventListenerMethodProcessor implements SmartInitializingSingleton,
} }
for (EventListenerFactory factory : factories) { for (EventListenerFactory factory : factories) {
if (factory.supportsMethod(method)) { if (factory.supportsMethod(method)) {
if (!type.equals(targetType)) {
method = getProxyMethod(type, method);
}
ApplicationListener<?> applicationListener = ApplicationListener<?> applicationListener =
factory.createApplicationListener(beanName, type, method); factory.createApplicationListener(beanName, targetType, method);
if (applicationListener instanceof ApplicationListenerMethodAdapter) { if (applicationListener instanceof ApplicationListenerMethodAdapter) {
((ApplicationListenerMethodAdapter)applicationListener) ((ApplicationListenerMethodAdapter) applicationListener)
.init(this.applicationContext, this.evaluator); .init(this.applicationContext, this.evaluator);
} }
this.applicationContext.addApplicationListener(applicationListener); this.applicationContext.addApplicationListener(applicationListener);
@ -127,49 +123,19 @@ public class EventListenerMethodProcessor implements SmartInitializingSingleton,
} }
} }
if (annotatedMethods.isEmpty()) { if (annotatedMethods.isEmpty()) {
this.nonAnnotatedClasses.add(type); this.nonAnnotatedClasses.add(targetType);
if (logger.isTraceEnabled()) { if (logger.isTraceEnabled()) {
logger.trace("No @EventListener annotations found on bean class: " + type); logger.trace("No @EventListener annotations found on bean class: " + targetType);
} }
} }
else { else {
// Non-empty set of methods // Non-empty set of methods
if (logger.isDebugEnabled()) { if (logger.isDebugEnabled()) {
logger.debug(annotatedMethods.size() + " @EventListener methods processed on bean '" + beanName + logger.debug(annotatedMethods.size() + " @EventListener methods processed on bean '" +
"': " + annotatedMethods); beanName + "': " + annotatedMethods);
} }
} }
} }
} }
private Class<?> getTargetClass(String beanName, Class<?> type) {
if (SpringProxy.class.isAssignableFrom(type)) {
Object bean = this.applicationContext.getBean(beanName);
return AopUtils.getTargetClass(bean);
}
else {
return type;
}
}
private Method getProxyMethod(Class<?> proxyType, Method method) {
try {
// Found a @EventListener method on the target class for this JDK proxy ->
// is it also present on the proxy itself?
return proxyType.getMethod(method.getName(), method.getParameterTypes());
}
catch (SecurityException ex) {
ReflectionUtils.handleReflectionException(ex);
}
catch (NoSuchMethodException ex) {
throw new IllegalStateException(String.format(
"@EventListener method '%s' found on bean target class '%s', " +
"but not found in any interface(s) for bean JDK proxy. Either " +
"pull the method up to an interface or switch to subclass (CGLIB) " +
"proxies by setting proxy-target-class/proxyTargetClass " +
"attribute to 'true'", method.getName(), method.getDeclaringClass().getSimpleName()));
}
return null;
}
} }

35
spring-context/src/test/java/org/springframework/context/event/AnnotationDrivenEventListenerTests.java

@ -73,6 +73,7 @@ public class AnnotationDrivenEventListenerTests {
private CountDownLatch countDownLatch; // 1 call by default private CountDownLatch countDownLatch; // 1 call by default
@After @After
public void closeContext() { public void closeContext() {
if (this.context != null) { if (this.context != null) {
@ -80,6 +81,7 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Test @Test
public void simpleEventJavaConfig() { public void simpleEventJavaConfig() {
load(TestEventListener.class); load(TestEventListener.class);
@ -241,13 +243,6 @@ public class AnnotationDrivenEventListenerTests {
this.eventCollector.assertTotalEventsCount(1); this.eventCollector.assertTotalEventsCount(1);
} }
@Test
public void methodNotAvailableOnProxyIsDetected() throws Exception {
thrown.expect(BeanInitializationException.class);
thrown.expectMessage("handleIt2");
load(InvalidProxyTestBean.class);
}
@Test @Test
public void eventListenerWorksWithCglibProxy() throws Exception { public void eventListenerWorksWithCglibProxy() throws Exception {
load(CglibProxyTestBean.class); load(CglibProxyTestBean.class);
@ -409,6 +404,7 @@ public class AnnotationDrivenEventListenerTests {
assertThat(listener.order, contains("first", "second", "third")); assertThat(listener.order, contains("first", "second", "third"));
} }
private void load(Class<?>... classes) { private void load(Class<?>... classes) {
List<Class<?>> allClasses = new ArrayList<>(); List<Class<?>> allClasses = new ArrayList<>();
allClasses.add(BasicConfiguration.class); allClasses.add(BasicConfiguration.class);
@ -450,6 +446,7 @@ public class AnnotationDrivenEventListenerTests {
} }
static abstract class AbstractTestEventListener extends AbstractIdentifiable { static abstract class AbstractTestEventListener extends AbstractIdentifiable {
@Autowired @Autowired
@ -458,9 +455,9 @@ public class AnnotationDrivenEventListenerTests {
protected void collectEvent(Object content) { protected void collectEvent(Object content) {
this.eventCollector.addEvent(this, content); this.eventCollector.addEvent(this, content);
} }
} }
@Component @Component
static class TestEventListener extends AbstractTestEventListener { static class TestEventListener extends AbstractTestEventListener {
@ -473,15 +470,16 @@ public class AnnotationDrivenEventListenerTests {
public void handleString(String content) { public void handleString(String content) {
collectEvent(content); collectEvent(content);
} }
} }
@EventListener @EventListener
@Target(ElementType.METHOD) @Target(ElementType.METHOD)
@Retention(RetentionPolicy.RUNTIME) @Retention(RetentionPolicy.RUNTIME)
@interface FooListener { @interface FooListener {
} }
@Component @Component
static class MetaAnnotationListenerTestBean extends AbstractTestEventListener { static class MetaAnnotationListenerTestBean extends AbstractTestEventListener {
@ -491,6 +489,7 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Component @Component
static class ContextEventListener extends AbstractTestEventListener { static class ContextEventListener extends AbstractTestEventListener {
@ -501,6 +500,7 @@ public class AnnotationDrivenEventListenerTests {
} }
@Component @Component
static class InvalidMethodSignatureEventListener { static class InvalidMethodSignatureEventListener {
@ -509,6 +509,7 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Component @Component
static class ReplyEventListener extends AbstractTestEventListener { static class ReplyEventListener extends AbstractTestEventListener {
@ -532,6 +533,7 @@ public class AnnotationDrivenEventListenerTests {
} }
@Component @Component
static class ExceptionEventListener extends AbstractTestEventListener { static class ExceptionEventListener extends AbstractTestEventListener {
@ -557,12 +559,14 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Configuration @Configuration
@Import(BasicConfiguration.class) @Import(BasicConfiguration.class)
@EnableAsync(proxyTargetClass = true) @EnableAsync(proxyTargetClass = true)
static class AsyncConfiguration { static class AsyncConfiguration {
} }
@Component @Component
static class AsyncEventListener extends AbstractTestEventListener { static class AsyncEventListener extends AbstractTestEventListener {
@ -578,6 +582,7 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
interface SimpleService extends Identifiable { interface SimpleService extends Identifiable {
@EventListener @EventListener
@ -585,6 +590,7 @@ public class AnnotationDrivenEventListenerTests {
} }
@Component @Component
@Scope(proxyMode = ScopedProxyMode.INTERFACES) @Scope(proxyMode = ScopedProxyMode.INTERFACES)
static class ProxyTestBean extends AbstractIdentifiable implements SimpleService { static class ProxyTestBean extends AbstractIdentifiable implements SimpleService {
@ -598,14 +604,6 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Component
@Scope(proxyMode = ScopedProxyMode.INTERFACES)
static class InvalidProxyTestBean extends ProxyTestBean {
@EventListener // does not exist on any interface so it should fail
public void handleIt2(TestEvent event) {
}
}
@Component @Component
@Scope(proxyMode = ScopedProxyMode.TARGET_CLASS) @Scope(proxyMode = ScopedProxyMode.TARGET_CLASS)
@ -617,6 +615,7 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Component @Component
static class GenericEventListener extends AbstractTestEventListener { static class GenericEventListener extends AbstractTestEventListener {
@ -626,6 +625,7 @@ public class AnnotationDrivenEventListenerTests {
} }
} }
@Component @Component
static class ConditionalEventListener extends TestEventListener { static class ConditionalEventListener extends TestEventListener {
@ -648,6 +648,7 @@ public class AnnotationDrivenEventListenerTests {
} }
@Component @Component
static class OrderedTestListener extends TestEventListener { static class OrderedTestListener extends TestEventListener {

Loading…
Cancel
Save