Fix nickname handling and reject impossible day-of-month expressions

README.md

@@ -3,7 +3,7 @@
 
 Parse Unix cron expressions and calculate things like:
 
-- whether the expression is valid, as a `bool`
+- whether the expression is valid, as a `bool`. an expression that can never match, such as `0 0 30 2 *` or the reversed range `30-10 * * * *`, is invalid
 - the next time the expression will run, as a `DateTime`
 - the last time the expression would have run, as a `DateTime`
 - the duration until the next time the expression will run, as a `Duration`
@@ -35,16 +35,16 @@ void main() {
 ### High-level explanation on matching 
 
 - if the expression is a predefined nickname, translate it to the relevant actual expression. note: this library does not parse the `@reboot` nickname.
-- tokenize the expression to each part (minute, hour, day of month, month, day of week)
+- tokenize the expression to each part (minute, hour, day of month, month, day of week). parts are separated by any run of spaces or tabs
 - test each of these tokens to match
     - token values can be in a few forms: 
         - an asterisk (`*`): this always matches
         - an asterisk with skips (`*/15`): iterate through the valid range for the field, incrementing by the skip. if any of these match, the expression matches. 
-        - exact value (`48`, `feb`, `WED`): match these exactly 
+        - exact value (`48`, `feb`, `WED`): match these exactly. month and day names are case-insensitive and work anywhere a number does, including ranges (`jan-mar`, `MON-FRI`). Sunday is both `0` and `7`, so `sun` starts a range as 0 (`sun-sat`) and ends one as 7 (`fri-sun`) 
         - range of values without skips (`5-9`): iterate through the range, starting at the lower bound and ending at the higher bound, incrementing by 1. if any of these match, the value matches 
         - range of values with skip (`20-30/2`): iterate through the range, starting at the lower bound and ending at the higher bound, incrementing by the skip value. if any of these match, the value matches.
-        - set of values or ranges (`1,2,3`, `5-10,45-50`): test each value in the set using the above strategy. if any of the member values match, the value matches. 
-- the expression matches when all five tokens match, with one exception. when both day of month and day of week are restricted (neither is `*`), the expression matches if either of the two matches. this is the rule cron itself uses, so `0 0 13 * 5` runs on the 13th of the month and on every Friday
+        - set of values or ranges (`1,2,3`, `5-10,45-50`, `1-10/2,20-30/5`): test each value in the set using the above strategy. if any of the member values match, the value matches. 
+- the expression matches when all five tokens match, with one exception. when both day of month and day of week are restricted (neither starts with `*`), the expression matches if either of the two matches. this is the rule cron itself uses, so `0 0 13 * 5` runs on the 13th of the month and on every Friday. as in cron, a field like `*/2` starts with `*` and so does not count as restricted: `0 0 13 * */2` runs only on a 13th that falls on an even weekday
 
 ### `@reboot`
 

lib/src/cronparse.dart

@@ -1,175 +1,60 @@
-import './validators.dart';
+import './fields.dart';
 
 /// [Cron] is an object exposing various methods to
 /// calculate [DateTime]s and [Duration]s from a given cron expression.
 class Cron {
   Cron(this.expr) {
-    if (!isValid(expr)) {
-      throw ArgumentError('invalid cron expression: "$expr"');
-    }
     if (expr == "@reboot") {
       throw ArgumentError('nickname expression "@reboot" is not supported');
     }
-
-    if (expr.startsWith("@")) {
-      switch (expr) {
-        case "@yearly":
-          _parsedExpr = "0 0 1 1 *";
-          break;
-        case "@annually":
-          _parsedExpr = "0 0 1 1 *";
-          break;
-        case "@monthly":
-          _parsedExpr = "0 0 1 * *";
-          break;
-        case "@weekly":
-          _parsedExpr = "0 0 * * 0";
-          break;
-        case "@daily":
-          _parsedExpr = "0 0 * * *";
-          break;
-        case "@hourly":
-          _parsedExpr = "0 * * * *";
-          break;
-        case "@midnight":
-          _parsedExpr = "0 23 * * *";
-          break;
-        default:
-          _parsedExpr = expr;
-      }
-    } else {
-      _parsedExpr = expr;
+    final fields = parseFields(expr);
+    if (fields == null) {
+      throw ArgumentError('invalid cron expression: "$expr"');
     }
-
-    final field = _parsedExpr.split(" ");
-    assert(field.length == 5);
-
-    _minuteField =
-        field[0].contains("/") ? field[0].replaceFirst("*", "0-59") : field[0];
-    _hourField =
-        field[1].contains("/") ? field[1].replaceFirst("*", "0-23") : field[1];
-    _dayOfMonthField =
-        field[2].contains("/") ? field[2].replaceFirst("*", "1-31") : field[2];
-
-    final monthField =
-        field[3].contains("/") ? field[3].replaceFirst("*", "1-12") : field[3];
-    _monthField = monthField
-        .toLowerCase()
-        .replaceAll('jan', '1')
-        .replaceAll('feb', '2')
-        .replaceAll('mar', '3')
-        .replaceAll('apr', '4')
-        .replaceAll('may', '5')
-        .replaceAll('jun', '6')
-        .replaceAll('jul', '7')
-        .replaceAll('aug', '8')
-        .replaceAll('sep', '9')
-        .replaceAll('oct', '10')
-        .replaceAll('nov', '11')
-        .replaceAll('dec', '12');
-
-    final dayOfWeekField =
-        field[4].contains("/") ? field[4].replaceFirst("*", "0-7") : field[4];
-    _dayOfWeekField = dayOfWeekField
-        .toLowerCase()
-        .replaceAll('mon', '1')
-        .replaceAll('tue', '2')
-        .replaceAll('wed', '3')
-        .replaceAll('thu', '4')
-        .replaceAll('fri', '5')
-        .replaceAll('sat', '6')
-        .replaceAll('sun', '7');
+    _fields = fields;
   }
 
   final String expr;
-  late final String _parsedExpr;
-  late final String _minuteField;
-  late final String _hourField;
-  late final String _dayOfMonthField;
-  late final String _monthField;
-  late final String _dayOfWeekField;
+  late final CronFields _fields;
 
   /// `matches` returns true if the full expression matches the given time;
   bool matches(DateTime time) {
     if (!minuteMatches(time) || !hourMatches(time) || !monthMatches(time)) {
       return false;
     }
-    // POSIX cron day rule: when both the day-of-month and the day-of-week
-    // fields are restricted (neither is `*`), a time matches if EITHER field
-    // matches. When only one of them is restricted, only that field applies.
-    final domRestricted = _dayOfMonthField != '*';
-    final dowRestricted = _dayOfWeekField != '*';
-    if (domRestricted && dowRestricted) {
-      return dayOfMonthMatches(time) || dayOfWeekMatches(time);
+    // Vixie cron day rule: when either day field starts with `*`, a time must
+    // match both. Otherwise both are restricted, and a time matches if EITHER
+    // field matches.
+    if (_fields.dayOfMonthStar || _fields.dayOfWeekStar) {
+      return dayOfMonthMatches(time) && dayOfWeekMatches(time);
     }
-    return dayOfMonthMatches(time) && dayOfWeekMatches(time);
+    return dayOfMonthMatches(time) || dayOfWeekMatches(time);
   }
 
   /// `minuteMatches` returns true if the minute field of the expression
   /// matches the minute of the given time
-  bool minuteMatches(DateTime time) {
-    return _fieldMatches(_minuteField, time.minute);
-  }
+  bool minuteMatches(DateTime time) => _fields.minutes.contains(time.minute);
 
   /// `hourMatches` returns true if the hour field of the expression
   /// matches the hour of the given time
-  bool hourMatches(DateTime time) {
-    return _fieldMatches(_hourField, time.hour);
-  }
+  bool hourMatches(DateTime time) => _fields.hours.contains(time.hour);
 
   /// `dayOfMonthMatches` returns true if the day of month field of the
   /// expression matches the day of month of the given time
-  bool dayOfMonthMatches(DateTime time) {
-    return _fieldMatches(_dayOfMonthField, time.day);
-  }
+  bool dayOfMonthMatches(DateTime time) =>
+      _fields.daysOfMonth.contains(time.day);
 
   /// `monthMatches` returns true if the month field of the expression
   /// matches the month of the given time
-  bool monthMatches(DateTime time) {
-    return _fieldMatches(_monthField, time.month);
-  }
+  bool monthMatches(DateTime time) => _fields.months.contains(time.month);
 
   /// `dayOfWeekMatches` returns true if the day of week field of the expression
   /// matches the day of week of the given time.
   ///
   /// Both 0 and 7 represent Sunday. [DateTime.weekday] uses 7 for Sunday, so
-  /// a cron value of 0 is treated as matching a weekday of 7.
-  bool dayOfWeekMatches(DateTime time) {
-    return _fieldMatches(_dayOfWeekField, time.weekday, sundayAlias: true);
-  }
-
-  /// `_fieldMatches` evaluates a single cron field against an integer [value].
-  ///
-  /// It handles the shared grammar for every field: asterisk, skips
-  /// (`*/n`, `a-b/n`), exact values, ranges (`a-b`), and comma-separated sets
-  /// of any of those. When [sundayAlias] is set (day-of-week), a field value of
-  /// 0 also matches a [value] of 7 so that both 0 and 7 mean Sunday.
-  bool _fieldMatches(String field, int value, {bool sundayAlias = false}) {
-    if (field == '*') return true;
-
-    bool hits(int i) => i == value || (sundayAlias && i == 0 && value == 7);
-
-    for (final part in field.split(',')) {
-      if (part.contains('/')) {
-        final s = part.split('/');
-        final skips = int.parse(s[1]);
-        final bounds = s[0].split('-').map(int.parse).toList();
-        for (var i = bounds[0]; i <= bounds[1]; i += skips) {
-          if (hits(i)) return true;
-        }
-        continue;
-      }
-      if (part.contains('-')) {
-        final bounds = part.split('-').map(int.parse).toList();
-        for (var i = bounds[0]; i <= bounds[1]; i++) {
-          if (hits(i)) return true;
-        }
-        continue;
-      }
-      if (hits(int.parse(part))) return true;
-    }
-    return false;
-  }
+  /// it is reduced modulo 7 to match the parsed field.
+  bool dayOfWeekMatches(DateTime time) =>
+      _fields.daysOfWeek.contains(time.weekday % 7);
 
   /// `next` calculates the next [DateTime]
   /// the expression is scheduled for, relative to

lib/src/fields.dart

@@ -0,0 +1,139 @@
+const _nicknames = {
+  '@yearly': '0 0 1 1 *',
+  '@annually': '0 0 1 1 *',
+  '@monthly': '0 0 1 * *',
+  '@weekly': '0 0 * * 0',
+  '@daily': '0 0 * * *',
+  '@midnight': '0 0 * * *',
+  '@hourly': '0 * * * *',
+};
+
+const _months = [
+  'jan',
+  'feb',
+  'mar',
+  'apr',
+  'may',
+  'jun',
+  'jul',
+  'aug',
+  'sep',
+  'oct',
+  'nov',
+  'dec'
+];
+const _days = ['sun', 'mon', 'tue', 'wed', 'thu', 'fri', 'sat'];
+
+final _digits = RegExp(r'^[0-9]+$');
+
+/// [CronFields] holds the values each field of a cron expression selects.
+class CronFields {
+  CronFields._(this.minutes, this.hours, this.daysOfMonth, this.months,
+      this.daysOfWeek, this.dayOfMonthStar, this.dayOfWeekStar);
+
+  final Set<int> minutes;
+  final Set<int> hours;
+  final Set<int> daysOfMonth;
+  final Set<int> months;
+
+  /// Days of the week, where 0 is Sunday. 7 is folded into 0.
+  final Set<int> daysOfWeek;
+
+  // Vixie cron treats a day field that starts with `*` as unrestricted, even
+  // with a step such as `*/2`. This decides whether the day fields combine
+  // with AND or with OR.
+  final bool dayOfMonthStar;
+  final bool dayOfWeekStar;
+}
+
+/// `parseFields` parses a cron expression or nickname, following the rules of
+/// Vixie cron. It returns null if the expression is malformed or can never
+/// match a real date.
+CronFields? parseFields(String expr) {
+  final expanded = expr.startsWith('@') ? _nicknames[expr] : expr.trim();
+  if (expanded == null) return null;
+  final parts = expanded.split(RegExp(r'\s+'));
+  if (parts.length != 5) return null;
+
+  final minutes = _parseField(parts[0], 0, 59);
+  final hours = _parseField(parts[1], 0, 23);
+  final daysOfMonth = _parseField(parts[2], 1, 31);
+  final months = _parseField(parts[3], 1, 12, names: _months, firstName: 1);
+  final daysOfWeek = _parseField(parts[4], 0, 7, names: _days, firstName: 0);
+  if (minutes == null ||
+      hours == null ||
+      daysOfMonth == null ||
+      months == null ||
+      daysOfWeek == null) {
+    return null;
+  }
+  if (daysOfWeek.remove(7)) daysOfWeek.add(0);
+
+  final fields = CronFields._(minutes, hours, daysOfMonth, months, daysOfWeek,
+      parts[2].startsWith('*'), parts[4].startsWith('*'));
+  return _hasPossibleDay(fields) ? fields : null;
+}
+
+// Every field selects at least one value, so only the day fields can rule out
+// every date. With both restricted, a day matches on either field, and every
+// weekday occurs. Otherwise day-of-month must name a day that occurs in one of
+// the selected months (Feb 30 never does).
+bool _hasPossibleDay(CronFields f) {
+  if (!f.dayOfMonthStar && !f.dayOfWeekStar) return true;
+  for (final month in f.months) {
+    // 2000 is a leap year, so February counts its 29th.
+    final days = DateTime(2000, month + 1, 0).day;
+    if (f.daysOfMonth.any((day) => day <= days)) return true;
+  }
+  return false;
+}
+
+/// `_parseField` returns the values a field selects, or null if the field is
+/// malformed. A field is a comma-separated list of elements. Each element is
+/// `*`, a value, or a range `a-b`, and `*` or a range may take a step `/n`.
+Set<int>? _parseField(String field, int min, int max,
+    {List<String> names = const [], int firstName = 0}) {
+  int? value(String s) {
+    if (_digits.hasMatch(s)) return int.parse(s);
+    final i = names.indexOf(s.toLowerCase());
+    return i < 0 ? null : i + firstName;
+  }
+
+  final values = <int>{};
+  for (final element in field.split(',')) {
+    final stepParts = element.split('/');
+    if (stepParts.length > 2) return null;
+    final range = stepParts[0];
+    final bounds = range.split('-');
+    if (bounds.length > 2) return null;
+
+    int? lo, hi;
+    if (range == '*') {
+      lo = min;
+      hi = max;
+    } else {
+      // A step needs a range to step through: `5/2` is invalid.
+      if (bounds.length == 1 && stepParts.length == 2) return null;
+      lo = value(bounds[0]);
+      hi = value(bounds.last);
+      // `sun` is 0, so a range ending on it, like `fri-sun`, uses 7 instead.
+      if (names == _days && bounds.length == 2 && lo != 0) {
+        if (bounds[1].toLowerCase() == 'sun') hi = 7;
+      }
+    }
+
+    var step = 1;
+    if (stepParts.length == 2) {
+      if (!_digits.hasMatch(stepParts[1])) return null;
+      step = int.parse(stepParts[1]);
+      if (step < 1) return null;
+    }
+    if (lo == null || hi == null || lo < min || hi > max || lo > hi) {
+      return null;
+    }
+    for (var i = lo; i <= hi; i += step) {
+      values.add(i);
+    }
+  }
+  return values;
+}