CronExpression is *still* broken on spring-context-5.3.8

[ScottAlbertine]

Hi, over a month ago, I opened #26964, and it was pretty promptly resolved... for that one specific cron expression. I pulled in that bugfix (along with the rest of 5.3.8), and more or less immediately found another cron line that is still broken in the exact same way: As you move forward in time, the next match of a cron expression moves backwards, which it should never ever do.

Instead of just attaching another bug proof with the particular bad cron expression, here is an entire bug proof generator that will give you an infinite number of similarly broken cron lines, with test dates, until this issue is actually fixed.

package com.example;

import org.junit.jupiter.api.Test;
import org.springframework.scheduling.support.CronExpression;

import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.util.ArrayList;
import java.util.List;
import java.util.Random;
import java.util.function.BiFunction;


/**
 * @author Scott Albertine
 */
public class CronExpressionBugProof {

	private static final Random R = new Random();
	private static final int TWO_YEARS_IN_SECONDS = 63113852;
	private static final LocalDateTime EPOCH = LocalDateTime.ofEpochSecond(0, 0, ZoneOffset.UTC);
	private static final List<BiFunction<Integer, Integer, String>> CRON_SEGMENT_GENERATORS = new ArrayList<>();

	static {
		CRON_SEGMENT_GENERATORS.add(CronExpressionBugProof::numberSegment);
		CRON_SEGMENT_GENERATORS.add(CronExpressionBugProof::star);
		CRON_SEGMENT_GENERATORS.add(CronExpressionBugProof::starOverNumber);
		CRON_SEGMENT_GENERATORS.add(CronExpressionBugProof::numberDashNumber);
		CRON_SEGMENT_GENERATORS.add(CronExpressionBugProof::numberCommaNumber);
		CRON_SEGMENT_GENERATORS.add(CronExpressionBugProof::numberHashNumber);
	}

	private static CronExpression randomCronExpression() {
		while (true) { //there are bad cron expressions, skip past them
			String cronLine = randomCronLine();
			try {
				//this will throw IllegalArgumentException on some bad cron lines, but not all of them
				CronExpression cronExpression = CronExpression.parse(cronLine);
				//this will check that the cron line has at least one instance since the epoch
				//to filter out stuff like "every february 30th"
				if (cronExpression.next(EPOCH) == null) {
					continue;
				}
				return cronExpression;
			} catch (IllegalArgumentException ignored) {
			}
		}
	}

	private static String randomCronLine() {
		return randomCronSegment(0, 59) + ' ' + //seconds
			   randomCronSegment(0, 59) + ' ' + //minutes
			   randomCronSegment(0, 23) + ' ' + //hours
			   randomCronSegment(1, 28) + ' ' + //days
			   randomCronSegment(1, 12) + ' ' + //months
			   randomWeekdaySegment(); //day of week
	}

	private static String randomCronSegment(int min, int max) {
		return CRON_SEGMENT_GENERATORS.get(R.nextInt(5)).apply(min, max); //don't use numberHashNumber
	}

	private static String randomWeekdaySegment() {
		return CRON_SEGMENT_GENERATORS.get(R.nextInt(6)).apply(0, 6); //include numberHashNumber
	}

	private static String numberSegment(Integer min, Integer max) {
		return Integer.toString(R.nextInt((max - min) + 1) + min);
	}

	private static String star(Integer min, Integer max) {
		return "*";
	}

	private static String starOverNumber(Integer min, Integer max) {
		return "*/" + (R.nextInt((max - min) / 2) + 2);
	}

	private static String numberDashNumber(Integer min, Integer max) {
		int num1 = R.nextInt((max - min) + 1) + min;
		int num2 = R.nextInt((max - min) + 1) + min;
		if (num1 == num2) {
			return Integer.toString(num1);
		}
		return (num1 > num2) ? (num2 + "-" + num1)
							 : (num1 + "-" + num2);
	}

	private static String numberCommaNumber(Integer min, Integer max) {
		int num1 = R.nextInt((max - min) + 1) + min;
		int num2 = R.nextInt((max - min) + 1) + min;
		if (num1 == num2) {
			return Integer.toString(num1);
		}
		return (num1 > num2) ? (num2 + "," + num1)
							 : (num1 + "," + num2);
	}

	private static String numberHashNumber(Integer min, Integer max) {
		int num1 = R.nextInt((max - min) + 1) + min;
		int num2 = R.nextInt(4) + 1;
		return num1 + "#" + num2;
	}

	@Test
	public void proveCronExpressionIsStillBroken() {
		//pick 1000 random times in the next 2 years
		LocalDateTime now = LocalDateTime.now(ZoneOffset.UTC);
		int timesToTest = 1000;
		List<LocalDateTime> times = new ArrayList<>();
		for (int i = 0; i < timesToTest; i++) {
			times.add(now.plusSeconds(R.nextInt(TWO_YEARS_IN_SECONDS)));
		}
		//make sure they're sorted, so we can know that the firstResult and secondResult below should be in chronological order too
		times.sort(null);

		//test up to 1000 cron lines
		for (int c = 0; c < 1000; c++) {
			CronExpression cronExpression = randomCronExpression(); //pick a random cron expression
			//iterate through each pair of times to test
			for (int t = 0; t < (timesToTest - 1); t++) {
				LocalDateTime firstTime = times.get(t);
				LocalDateTime secondTime = times.get(t + 1);
				LocalDateTime firstResult = cronExpression.next(firstTime);
				LocalDateTime secondResult = cronExpression.next(secondTime);
				if (firstResult.isAfter(secondResult)) { //check for pairs of results that aren't in the right order
					System.out.println("Insane CronExpression: " + cronExpression);
					System.out.println("It thinks the next match of " + firstTime + " is " + firstResult);
					System.out.println("And the next match of       " + secondTime + " is " + secondResult);
					System.out.println("Even though " + firstResult + " is after " + secondResult);
					System.out.println("Cron lines tested: " + c);
					throw new RuntimeException("Insane CronExpression found.");
				}
			}
		}
	}

}

This is by no means an exhaustive test. However, when I run it, I get output like the following:

Insane CronExpression: 4,21 12,58 * */6 3 *
It thinks the next match of 2022-02-28T13:32:03.032096 is 2022-03-07T00:12:04
And the next match of       2022-03-01T22:44:31.032096 is 2022-03-01T22:58:04
Even though 2022-03-07T00:12:04 is after 2022-03-01T22:58:04
Cron lines tested: 27

I've never made it to 50 cron lines without finding one that's broken, and I've run this dozens of times.

[poutsma]

Thank you for providing that test, it has proven quite valuable for fixing two issues in CronExpression. The fixes are in 76b1c0f (I put the wrong github issue in the commit message by accident, so there is no link from this issue). With fixes for those two in, I am now able to run your tests multiple times without running into exceptions.

Feel free to try a recent snapshot, and see if you can find any other remaining issues. It would be good to squash them all in 5.3.9.