Skip to content

Skip months that are too short for a Quartz L-n offset in CronExpression - #37686

Open
waw666waw666 wants to merge 1 commit into
spring-projects:mainfrom
waw666waw666:fix-cron-last-day-offset
Open

waw666waw666 wants to merge 1 commit into
spring-projects:mainfrom
waw666waw666:fix-cron-last-day-offset

Conversation

@waw666waw666

@waw666waw666 waw666waw666 commented Nov 14, 2026 •

Copy link
Copy Markdown

CronExpression returns a time before the given one for a Quartz L-n day-of-month field when a month has n days or fewer, and it keeps returning that same time, so CronTrigger computes the same past execution over and over.

CronExpression expression = CronExpression.parse("0 0 12 L-28 * *");
expression.next(LocalDateTime.of(2025, 1, 15, 0, 0));   // 2025-01-31T12:00, expected 2025-03-03T12:00
expression.next(LocalDateTime.of(2025, 1, 31, 12, 0));  // 2025-01-31T12:00 again
expression.next(LocalDateTime.of(2025, 1, 31, 13, 0));  // 2025-01-31T12:00, earlier than the input

CronExpression.parse("0 0 12 L-30 * *").next(LocalDateTime.of(2024, 1, 15, 0, 0));
// 2024-01-30T12:00, expected 2024-03-01T12:00

With new CronTrigger("0 0 12 L-28 * *"), nextExecution() keeps answering 2025-01-31T12:00Z after each run (I checked with SimpleTriggerContext, completion 50ms after the scheduled time), so on a ThreadPoolTaskScheduler the task is rescheduled for a time in the past every time it completes, i.e. it runs back to back. This affects L-28 (February of non-leap years), L-29 (February) and L-30 (February and 30-day months).

Cause

QuartzCronField.lastDayWithOffset(int) adds the negative offset to the last day of the month, so when the month is too short the result lands in the previous month. When the start is past the current month's own L-n day, nextOrSame() rolls forward to the next month and adjusts again; if that month is too short, the second result falls back into the current month (January 31st for L-28 in February 2025) and is returned without another check. CronExpression takes it as the next match, and the later fields only move it within that day, which is how next() ends up earlier than its input.

Fix

When the result is not in the month being adjusted, try the first day of the next month instead, up to 12 times, the same way dayOfWeekInMonth() already handles an nth day-of-week that overflows the month. Months without such a day are skipped (as in Quartz); an offset that never fits (e.g. L-31) gives no next execution.

Tests

  • Added CronExpressionTests#quartzLastDayOfMonthOffsetLongerThanMonth (L-28 across February 2025, L-30 across February and April 2024, and a start time after the bogus result). It fails on main (expected: 2025-03-03T12:00 but was: 2025-01-31T12:00) and passes with the change.
  • All tests in org.springframework.scheduling.support pass, and Checkstyle is clean for spring-context.
  • I also compared next() against a brute-force day-by-day search for L-1 to L-35 from start times spread over 2023 to 2025: about 20k mismatches before, none after.

The "L-n" day-of-month adjuster subtracts n days from the last day of
the month. When the month has n days or fewer ("L-28" in February of a
non-leap year, "L-30" in a 30-day month), the result lands in the
previous month, before the temporal being adjusted. CronExpression then
returns that earlier date from next(), even when called with that date
itself or a later time, so a CronTrigger computes the same past
execution time over and over.

Move to the next month instead when the offset does not fit, as
dayOfWeekInMonth() already does for an nth day-of-week that overflows
the month.

Signed-off-by: waw666waw666 <[email protected]>
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 14, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: waiting-for-triage An issue we've not yet triaged or decided on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants