Skip to content

Commit 674e232

Browse files
committed
CronSequenceGenerator.isValidExpression actually validates cron fields
Issue: SPR-15604 (cherry picked from commit 5f4d1a4)
1 parent fee259a commit 674e232

File tree

2 files changed

+39
-7
lines changed

2 files changed

+39
-7
lines changed

spring-context/src/main/java/org/springframework/scheduling/support/CronSequenceGenerator.java

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
package org.springframework.scheduling.support;
1818

1919
import java.util.ArrayList;
20-
import java.util.Arrays;
2120
import java.util.BitSet;
2221
import java.util.Calendar;
2322
import java.util.Collections;
@@ -50,6 +49,7 @@
5049
*
5150
* @author Dave Syer
5251
* @author Juergen Hoeller
52+
* @author Ruslan Sibgatullin
5353
* @since 3.0
5454
* @see CronTrigger
5555
*/
@@ -96,6 +96,12 @@ public CronSequenceGenerator(String expression, TimeZone timeZone) {
9696
parse(expression);
9797
}
9898

99+
private CronSequenceGenerator(String expression, String[] fields) {
100+
this.expression = expression;
101+
this.timeZone = null;
102+
doParse(fields);
103+
}
104+
99105

100106
/**
101107
* Return the cron pattern that this sequence generator has been built for.
@@ -234,7 +240,7 @@ private int findNext(BitSet bits, int value, Calendar calendar, int field, int n
234240
// roll over if needed
235241
if (nextValue == -1) {
236242
calendar.add(nextField, 1);
237-
reset(calendar, Arrays.asList(field));
243+
reset(calendar, Collections.singletonList(field));
238244
nextValue = bits.nextSetBit(0);
239245
}
240246
if (nextValue != value) {
@@ -265,12 +271,17 @@ private void parse(String expression) throws IllegalArgumentException {
265271
throw new IllegalArgumentException(String.format(
266272
"Cron expression must consist of 6 fields (found %d in \"%s\")", fields.length, expression));
267273
}
274+
doParse(fields);
275+
}
276+
277+
private void doParse(String[] fields) {
268278
setNumberHits(this.seconds, fields[0], 0, 60);
269279
setNumberHits(this.minutes, fields[1], 0, 60);
270280
setNumberHits(this.hours, fields[2], 0, 24);
271281
setDaysOfMonth(this.daysOfMonth, fields[3]);
272282
setMonths(this.months, fields[4]);
273283
setDays(this.daysOfWeek, replaceOrdinals(fields[5], "SUN,MON,TUE,WED,THU,FRI,SAT"), 8);
284+
274285
if (this.daysOfWeek.get(7)) {
275286
// Sunday can be represented as 0 or 7
276287
this.daysOfWeek.set(0);
@@ -388,15 +399,25 @@ private int[] getRange(String field, int min, int max) {
388399

389400
/**
390401
* Determine whether the specified expression represents a valid cron pattern.
391-
* <p>Specifically, this method verifies that the expression contains six
392-
* fields separated by single spaces.
393402
* @param expression the expression to evaluate
394403
* @return {@code true} if the given expression is a valid cron expression
395404
* @since 4.3
396405
*/
397406
public static boolean isValidExpression(String expression) {
407+
if (expression == null) {
408+
return false;
409+
}
398410
String[] fields = StringUtils.tokenizeToStringArray(expression, " ");
399-
return areValidCronFields(fields);
411+
if (!areValidCronFields(fields)) {
412+
return false;
413+
}
414+
try {
415+
new CronSequenceGenerator(expression, fields);
416+
return true;
417+
}
418+
catch (IllegalArgumentException ex) {
419+
return false;
420+
}
400421
}
401422

402423
private static boolean areValidCronFields(String[] fields) {

spring-context/src/test/java/org/springframework/scheduling/support/CronSequenceGeneratorTests.java

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2017 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -24,6 +24,7 @@
2424

2525
/**
2626
* @author Juergen Hoeller
27+
* @author Ruslan Sibgatullin
2728
*/
2829
@SuppressWarnings("deprecation")
2930
public class CronSequenceGeneratorTests {
@@ -82,10 +83,20 @@ public void validExpression() {
8283
}
8384

8485
@Test
85-
public void invalidExpression() {
86+
public void invalidExpressionWithLength() {
8687
assertFalse(CronSequenceGenerator.isValidExpression("0 */2 1-4 * * * *"));
8788
}
8889

90+
@Test
91+
public void invalidExpressionWithSeconds() {
92+
assertFalse(CronSequenceGenerator.isValidExpression("100 */2 1-4 * * *"));
93+
}
94+
95+
@Test
96+
public void invalidExpressionWithMonths() {
97+
assertFalse(CronSequenceGenerator.isValidExpression("0 */2 1-4 * INVALID *"));
98+
}
99+
89100
@Test
90101
public void nullExpression() {
91102
assertFalse(CronSequenceGenerator.isValidExpression(null));

0 commit comments

Comments
 (0)