Bernhard Seebass opened SPR-8981 and commented

A cron expression combining fixed date and fixed weekday may or may not fail.

Example:

TimeZone tz = TimeZone.getTimeZone("UTC");
CronSequenceGenerator g = new CronSequenceGenerator("0 0 0 29 2 WED", tz);
System.out.println(g.next(new Date(112, 1, 1))); // -> Wed Feb 29 01:00:00 CET 2012
try {
g.next(new Date(113, 2, 1));
} catch (IllegalStateException ex) {
ex.printStackTrace();
// java.lang.IllegalStateException: Invalid cron expression led to runaway search for next trigger
}

This kind of cron-expression must either always trigger an exception on initialization or it must always work. To make this work, CronSequenceGenerator would have to be adapted to allow 28 years in the future instead of only 4 (line 162).


Affects: 3.1 GA

Issue Links: - #21574 CronSequenceGenerator - Friday 13th issue - #19500 Cron Expression validation

2 votes, 3 watchers

Comment From: spring-projects-issues

Bernhard Seebass commented

To be safe against a year 2100 bug, CronSequenceGenerator must calculate dates up to 40 years in the future, not only 28 years (e.g. Mo 2072-02-29 to Mo 2112-02-29).

Comment From: spring-projects-issues

Archie Cobbs commented

I've seen similar behavior. There are two basic problems: 1. The CronSequenceGenerator constructor only partially validates the string, so it's possible that construction can succeed but an IllegalArgumentException is be thrown at some later time by CronSequenceGenerator.next(). 2. Whether an IllegalArgumentException is thrown by CronSequenceGenerator.next() depends on the Date parameter given.

So if you have a bad cron expression, you get a bug with two undesirable characteristics: it's both indeterminate and fail-slow.

My particular run-time stack trace looked like this (3.2.2-RELEASE):

java.lang.IllegalArgumentException: Overflow in day for expression "*/5 0 0 0 * *"
        at org.springframework.scheduling.support.CronSequenceGenerator.findNextDay(CronSequenceGenerator.java:207)
        at org.springframework.scheduling.support.CronSequenceGenerator.doNext(CronSequenceGenerator.java:173)
        at org.springframework.scheduling.support.CronSequenceGenerator.doNext(CronSequenceGenerator.java:168)
        at org.springframework.scheduling.support.CronSequenceGenerator.doNext(CronSequenceGenerator.java:159)
        at org.springframework.scheduling.support.CronSequenceGenerator.next(CronSequenceGenerator.java:132)
        at org.springframework.scheduling.support.CronTrigger.nextExecutionTime(CronTrigger.java:72)
        at org.springframework.scheduling.concurrent.ReschedulingRunnable.schedule(ReschedulingRunnable.java:68)
        at org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler.schedule(ThreadPoolTaskScheduler.java:165)
        ...

The fix is to strictly validate the expression at construction time. It should not be possible for IllegalArgumentException to be thrown at any time other than construction time.