HangfireIO / HangfireIO/Cronos
`GetPreviousOccurrence` returns wrong dates for `*` day-of-month, `W` and `L-n`; plus parser and API edge cases
- Dominant language
- C#
- Stars
- 1.2k
- Forks
- 137
- Avg merge
- 5d 11h
- Merged PRs (30d)
- 1
Description
While reviewing the reverse-search code added in #90 I found several cases where `GetPreviousOccurrence` returns a date that `GetNextOccurrence` would never produce, plus a few smaller bugs in the parser and the `GetOccurrences*` API. All of them reproduce on current `main` (0.13.0). A PR with fixes and regression tests follows.
### 1. Reverse search returns a date from a month the expression excludes
```csharp
CronExpression.Parse("0 1 * 3 *")
.GetPreviousOccurrence(new DateTime(2017, 3, 1, 0, 30, 0, DateTimeKind.Utc));
// actual: 2017-02-28 01:00 (February is not in the expression)
// expected: 2016-03-31 01:00
```
When `from` is on the 1st of a matching month and before that day's first match, the hour rollback makes `day = 0`. The `*` day-of-month fast path in `TryGetPreviousDay` accepts day 0, and `CalendarHelper.DateTimeToTicks` silently wraps it into the last day of the previous month. With an unrestricted month field the wrong path happens to yield the right instant, which is why the existing tests pass. The same root cause makes `Parse("0 1 * * *").GetPreviousOccurrence(new DateTime(1, 1, 1, 0, 30, 0, DateTimeKind.Utc))` throw `ArgumentOutOfRangeException` instead of returning `null`.
### 2. Reverse search fires `W` on a day that does not exist in the month
```csharp
CronExpression.Parse("0 0 31W * *")
.GetPreviousOccurrence(new DateTime(2027, 5, 15, 0, 0, 0, DateTimeKind.Utc));
// actual: 2027-04-30 (April has 30 days; the forward search never fires in April)
// expected: 2027-03-31
```
`TryGetPreviousDay` calls `MoveToNearestWeekDay(2027, 4, 31)`; day 31 is interpreted as 1 May (a Saturday), shifted back to the 30th, and accepted. The forward search rejects the month via `day > GetLastDayOfMonth`. Same for `30W` in a leap-year February when 2 March is a Saturday. As a result `GetOccurrencesDescending` is not the reverse of `GetOccurrences`.
### 3. Reverse search with `L-n` (n ≥ 28) returns a phantom day in the previous month
```csharp
CronExpression.Parse("0 0 L-28 * *")
.GetPreviousOccurrence(new DateTime(2017, 3, 1, 0, 0, 0, DateTimeKind.Utc));
// actual: 2017-01-31
// expected: 2017-01-03
```
For February, `GetLastDayOfMonth` yields `28 - 28 = 0`, which again wraps into January. The parser accepts offsets up to 30, and the forward search handles them correctly.
### 4. Time-zone reverse search skips the floored second for non-round, exclusive `from`
```csharp
var from = new DateTimeOffset(2017, 3, 22, 10, 0, 0, 500, TimeSpan.Zero);
CronExpression.EverySecond.GetPreviousOccurrence(from, eastern, inclusive: false);
// actual: 09:59:59Z
// expected: 10:00:00Z (what the UTC overload and the TimeZoneInfo.Utc fast path return)
```
The zoned path floors `from` to whole seconds but keeps `inclusive = false`, so the floored second itself is excluded even though it is strictly earlier than the original `from`. The forward path handles its mirror case correctly.
### 5. Reversed day-of-week range starting at `7` with a step is wrong
```csharp
CronExpression.Parse("* * * * 7-3/2"); // {2} instead of {0, 2, 7}; not equal to "0-3/2"
CronExpression.Parse("* * * * 7-1/2"); // empty day-of-week mask: every search returns null
```
`GetReversedRangeBits` lowers `high` to 6 for day-of-week, so `(high - num1) % step` is negative when `num1 == 7`, and the wrap-around start is computed incorrectly. The README states that `0` and `7` are both Sunday.
### 6. Parsing a long comma-separated list overflows the stack
```csharp
var list = string.Join(",", Enumerable.Repeat("1", 100_000));
CronExpression.TryParse("0 0 1 1 " + list, out _); // StackOverflowException, process crash
```
`ParseList` recurses once per element. `StackOverflowException` cannot be caught, so `TryParse` does not protect callers that parse user-supplied strings.
### 7. `GetOccurrences` / `GetOccurrencesDescending` validate arguments only on first enumeration
All six overloads are iterators, so `ArgumentException` for `from > to` (and the `DateTimeKind` / null-zone checks) is thrown on the first `MoveNext()` instead of at the call site.
### Related
- #92 / #94 (spring-forward gap in reverse search) and #93 / #97 (`DateTime` range boundaries) are independent of the above; the accompanying PR is written so that it applies cleanly alongside both. Note that #94 currently leaves three Jordan gap tests asserting the old behaviour, which fail once it is applied.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at CronExpression.GetPreviousOccurrence, TryGetPreviousDay, GetReversedRangeBits, ParseList, and the GetOccurrences/GetOccurrencesDescending overloads; trace the existing forward-search behavior and time-zone flooring. Add regression coverage for each listed expression and API case, including long-list parsing and eager validation, and confirm reverse results match forward results where stated.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 30/100