CronSequenceGenerator explicitly rejects invalid incrementer delta

Issue: SPR-12871

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -29,7 +29,8 @@ import java.util.TimeZone;
 import org.springframework.util.StringUtils;
 
 /**
- * Date sequence generator for a <a href="http://www.manpagez.com/man/5/crontab/">Crontab pattern</a>,
+ * Date sequence generator for a
+ * <a href="http://www.manpagez.com/man/5/crontab/">Crontab pattern</a>,
  * allowing clients to specify a pattern that the sequence matches.
  *
  * <p>The pattern is a list of six single space-separated fields: representing
@@ -53,21 +54,21 @@ import org.springframework.util.StringUtils;
  */
 public class CronSequenceGenerator {
 
-	private final BitSet seconds = new BitSet(60);
-
-	private final BitSet minutes = new BitSet(60);
+	private final String expression;
 
-	private final BitSet hours = new BitSet(24);
+	private final TimeZone timeZone;
 
-	private final BitSet daysOfWeek = new BitSet(7);
+	private final BitSet months = new BitSet(12);
 
 	private final BitSet daysOfMonth = new BitSet(31);
 
-	private final BitSet months = new BitSet(12);
+	private final BitSet daysOfWeek = new BitSet(7);
 
-	private final String expression;
+	private final BitSet hours = new BitSet(24);
 
-	private final TimeZone timeZone;
+	private final BitSet minutes = new BitSet(60);
+
+	private final BitSet seconds = new BitSet(60);
 
 
 	/**
@@ -95,6 +96,14 @@ public class CronSequenceGenerator {
 	}
 
 
+	/**
+	 * Return the cron pattern that this sequence generator has been built for.
+	 */
+	String getExpression() {
+		return this.expression;
+	}
+
+
 	/**
 	 * Get the next {@link Date} in the sequence matching the Cron pattern and
 	 * after the value provided. The return value will have a whole number of
@@ -271,9 +280,9 @@ public class CronSequenceGenerator {
 	}
 
 	/**
-	 * Replace the values in the commaSeparatedList (case insensitive) with
-	 * their index in the list.
-	 * @return a new string with the values from the list replaced
+	 * Replace the values in the comma-separated list (case insensitive)
+	 * with their index in the list.
+	 * @return a new String with the values from the list replaced
 	 */
 	private String replaceOrdinals(String value, String commaSeparatedList) {
 		String[] list = StringUtils.commaDelimitedListToStringArray(commaSeparatedList);
@@ -332,6 +341,10 @@ public class CronSequenceGenerator {
 					range[1] = max - 1;
 				}
 				int delta = Integer.valueOf(split[1]);
+				if (delta <= 0) {
+					throw new IllegalArgumentException("Incrementer delta must be 1 or higher: '" +
+							field + "' in expression \"" + this.expression + "\"");
+				}
 				for (int i = range[0]; i <= range[1]; i += delta) {
 					bits.set(i);
 				}
@@ -369,30 +382,30 @@ public class CronSequenceGenerator {
 		return result;
 	}
 
-	String getExpression() {
-		return this.expression;
-	}
 
 	@Override
-	public boolean equals(Object obj) {
-		if (!(obj instanceof CronSequenceGenerator)) {
+	public boolean equals(Object other) {
+		if (this == other) {
+			return true;
+		}
+		if (!(other instanceof CronSequenceGenerator)) {
 			return false;
 		}
-		CronSequenceGenerator cron = (CronSequenceGenerator) obj;
-		return cron.months.equals(this.months) && cron.daysOfMonth.equals(this.daysOfMonth)
-				&& cron.daysOfWeek.equals(this.daysOfWeek) && cron.hours.equals(this.hours)
-				&& cron.minutes.equals(this.minutes) && cron.seconds.equals(this.seconds);
+		CronSequenceGenerator otherCron = (CronSequenceGenerator) other;
+		return (this.months.equals(otherCron.months) && this.daysOfMonth.equals(otherCron.daysOfMonth) &&
+				this.daysOfWeek.equals(otherCron.daysOfWeek) && this.hours.equals(otherCron.hours) &&
+				this.minutes.equals(otherCron.minutes) && this.seconds.equals(otherCron.seconds));
 	}
 
 	@Override
 	public int hashCode() {
-		return 37 + 17 * this.months.hashCode() + 29 * this.daysOfMonth.hashCode() + 37 * this.daysOfWeek.hashCode()
-				+ 41 * this.hours.hashCode() + 53 * this.minutes.hashCode() + 61 * this.seconds.hashCode();
+		return (17 * this.months.hashCode() + 29 * this.daysOfMonth.hashCode() + 37 * this.daysOfWeek.hashCode() +
+				41 * this.hours.hashCode() + 53 * this.minutes.hashCode() + 61 * this.seconds.hashCode());
 	}
 
 	@Override
 	public String toString() {
-		return getClass().getSimpleName() + ": " + this.expression;
+		return (getClass().getSimpleName() + ": " + this.expression);
 	}
 
 }

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -37,21 +37,29 @@ public class CronTrigger implements Trigger {
 
 	/**
 	 * Build a {@link CronTrigger} from the pattern provided in the default time zone.
-	 * @param cronExpression a space-separated list of time fields,
-	 * following cron expression conventions
+	 * @param expression a space-separated list of time fields, following cron
+	 * expression conventions
 	 */
-	public CronTrigger(String cronExpression) {
-		this.sequenceGenerator = new CronSequenceGenerator(cronExpression);
+	public CronTrigger(String expression) {
+		this.sequenceGenerator = new CronSequenceGenerator(expression);
 	}
 
 	/**
-	 * Build a {@link CronTrigger} from the pattern provided.
-	 * @param cronExpression a space-separated list of time fields,
-	 * following cron expression conventions
+	 * Build a {@link CronTrigger} from the pattern provided in the given time zone.
+	 * @param expression a space-separated list of time fields, following cron
+	 * expression conventions
 	 * @param timeZone a time zone in which the trigger times will be generated
 	 */
-	public CronTrigger(String cronExpression, TimeZone timeZone) {
-		this.sequenceGenerator = new CronSequenceGenerator(cronExpression, timeZone);
+	public CronTrigger(String expression, TimeZone timeZone) {
+		this.sequenceGenerator = new CronSequenceGenerator(expression, timeZone);
+	}
+
+
+	/**
+	 * Return the cron pattern that this trigger has been built with.
+	 */
+	public String getExpression() {
+		return this.sequenceGenerator.getExpression();
 	}
 
 
@@ -79,14 +87,11 @@ public class CronTrigger implements Trigger {
 		return this.sequenceGenerator.next(date);
 	}
 
-	public String getExpression() {
-		return this.sequenceGenerator.getExpression();
-	}
 
 	@Override
-	public boolean equals(Object obj) {
-		return (this == obj || (obj instanceof CronTrigger &&
-				this.sequenceGenerator.equals(((CronTrigger) obj).sequenceGenerator)));
+	public boolean equals(Object other) {
+		return (this == other || (other instanceof CronTrigger &&
+				this.sequenceGenerator.equals(((CronTrigger) other).sequenceGenerator)));
 	}
 
 	@Override

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2013 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -46,4 +46,14 @@ public class CronSequenceGeneratorTests {
 				new CronSequenceGenerator("0 */2 1-4 * * *").next(new Date(2012, 6, 1, 9, 0)));
 	}
 
+	@Test(expected = IllegalArgumentException.class)
+	public void testWith0Increment() {
+		new CronSequenceGenerator("*/0 * * * * *").next(new Date(2012, 6, 1, 9, 0));
+	}
+
+	@Test(expected = IllegalArgumentException.class)
+	public void testWithNegativeIncrement() {
+		new CronSequenceGenerator("*/-1 * * * * *").next(new Date(2012, 6, 1, 9, 0));
+	}
+
 }