From 01724d3b6dbe1678e3a192de7369b2dde46579a6 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Tue, 7 Oct 2014 16:32:09 +0200 Subject: [PATCH] Explicitly detect (and prevent) private @Scheduled methods on CGLIB proxies Issue: SPR-12308 --- .../ScheduledAnnotationBeanPostProcessor.java | 28 ++++++++----- ...duledAnnotationBeanPostProcessorTests.java | 39 ++++++++++++++++--- 2 files changed, 52 insertions(+), 15 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java b/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java index e5818d6f82..6cee90081c 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java +++ b/spring-context/src/main/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessor.java @@ -17,6 +17,7 @@ package org.springframework.scheduling.annotation; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashSet; @@ -77,9 +78,8 @@ import org.springframework.util.StringValueResolver; * @see org.springframework.scheduling.TaskScheduler * @see org.springframework.scheduling.config.ScheduledTaskRegistrar */ -public class ScheduledAnnotationBeanPostProcessor - implements BeanPostProcessor, Ordered, EmbeddedValueResolverAware, BeanFactoryAware, - SmartInitializingSingleton, DisposableBean { +public class ScheduledAnnotationBeanPostProcessor implements BeanPostProcessor, Ordered, + EmbeddedValueResolverAware, BeanFactoryAware, SmartInitializingSingleton, DisposableBean { protected final Log logger = LogFactory.getLog(getClass()); @@ -227,17 +227,27 @@ public class ScheduledAnnotationBeanPostProcessor } catch (NoSuchMethodException ex) { throw new IllegalStateException(String.format( - "@Scheduled 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())); + "@Scheduled method '%s' found on bean target class '%s' but not " + + "found in any interface(s) for a dynamic proxy. Either pull the " + + "method up to a declared interface or switch to subclass (CGLIB) " + + "proxies by setting proxy-target-class/proxyTargetClass to 'true'", + method.getName(), method.getDeclaringClass().getSimpleName())); + } + } + else if (AopUtils.isCglibProxy(bean)) { + // Common problem: private methods end up in the proxy instance, not getting delegated. + if (Modifier.isPrivate(method.getModifiers())) { + throw new IllegalStateException(String.format( + "@Scheduled method '%s' found on CGLIB proxy for target class '%s' but cannot " + + "be delegated to target bean. Switch its visibility to package or protected.", + method.getName(), method.getDeclaringClass().getSimpleName())); } } Runnable runnable = new ScheduledMethodRunnable(bean, method); boolean processedSchedule = false; - String errorMessage = "Exactly one of the 'cron', 'fixedDelay(String)', or 'fixedRate(String)' attributes is required"; + String errorMessage = + "Exactly one of the 'cron', 'fixedDelay(String)', or 'fixedRate(String)' attributes is required"; // Determine initial delay long initialDelay = scheduled.initialDelay(); diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java index 238eb01d1c..e6ed71bf2c 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2013 the original author or authors. + * Copyright 2002-2014 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -28,6 +28,7 @@ import java.util.List; import java.util.Properties; import java.util.TimeZone; +import org.junit.After; import org.junit.Test; import org.springframework.beans.DirectFieldAccessor; @@ -46,6 +47,8 @@ import org.springframework.scheduling.support.ScheduledMethodRunnable; import org.springframework.scheduling.support.SimpleTriggerContext; import org.springframework.tests.Assume; import org.springframework.tests.TestGroup; +import org.springframework.validation.annotation.Validated; +import org.springframework.validation.beanvalidation.MethodValidationPostProcessor; import static org.junit.Assert.*; @@ -60,6 +63,13 @@ public class ScheduledAnnotationBeanPostProcessorTests { private final StaticApplicationContext context = new StaticApplicationContext(); + + @After + public void closeContextAfterTest() { + context.close(); + } + + @Test public void fixedDelayTask() { BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); @@ -140,7 +150,8 @@ public class ScheduledAnnotationBeanPostProcessorTests { public void severalFixedRatesWithRepeatedScheduledAnnotation() { BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); - BeanDefinition targetDefinition = new RootBeanDefinition(SeveralFixedRatesWithRepeatedScheduledAnnotationTestBean.class); + BeanDefinition targetDefinition = new RootBeanDefinition( + SeveralFixedRatesWithRepeatedScheduledAnnotationTestBean.class); severalFixedRates(context, processorDefinition, targetDefinition); } @@ -155,6 +166,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { private void severalFixedRates(StaticApplicationContext context, BeanDefinition processorDefinition, BeanDefinition targetDefinition) { + context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); context.refresh(); @@ -248,7 +260,8 @@ public class ScheduledAnnotationBeanPostProcessorTests { Date lastActualExecutionTime = cal.getTime(); cal.add(Calendar.MINUTE, 30); // 4:30 Date lastCompletionTime = cal.getTime(); - TriggerContext triggerContext = new SimpleTriggerContext(lastScheduledExecutionTime, lastActualExecutionTime, lastCompletionTime); + TriggerContext triggerContext = new SimpleTriggerContext( + lastScheduledExecutionTime, lastActualExecutionTime, lastCompletionTime); cal.add(Calendar.MINUTE, 30); cal.add(Calendar.HOUR_OF_DAY, 1); // 6:00 Date nextExecutionTime = cronTrigger.nextExecutionTime(triggerContext); @@ -269,6 +282,18 @@ public class ScheduledAnnotationBeanPostProcessorTests { Thread.sleep(10000); } + @Test(expected = BeanCreationException.class) + public void cronTaskWithMethodValidation() throws InterruptedException { + BeanDefinition validationDefinition = new RootBeanDefinition(MethodValidationPostProcessor.class); + BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); + BeanDefinition targetDefinition = new RootBeanDefinition( + ScheduledAnnotationBeanPostProcessorTests.CronTestBean.class); + context.registerBeanDefinition("methodValidation", validationDefinition); + context.registerBeanDefinition("postProcessor", processorDefinition); + context.registerBeanDefinition("target", targetDefinition); + context.refresh(); + } + @Test public void metaAnnotationWithFixedRate() { BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); @@ -527,10 +552,11 @@ public class ScheduledAnnotationBeanPostProcessorTests { } + @Validated static class CronTestBean { @Scheduled(cron="*/7 * * * * ?") - public void cron() throws IOException { + private void cron() throws IOException { throw new IOException("no no no"); } } @@ -539,7 +565,7 @@ public class ScheduledAnnotationBeanPostProcessorTests { static class CronWithTimezoneTestBean { @Scheduled(cron="0 0 0-4,6-23 * * ?", zone = "GMT+10") - public void cron() throws IOException { + protected void cron() throws IOException { throw new IOException("no no no"); } } @@ -642,7 +668,8 @@ public class ScheduledAnnotationBeanPostProcessorTests { @Scheduled(cron="${schedules.businessHours}") @Target(ElementType.METHOD) @Retention(RetentionPolicy.RUNTIME) - private static @interface BusinessHours {} + private static @interface BusinessHours { + } static class PropertyPlaceholderMetaAnnotationTestBean { -- GitLab