Skip to main content

max / goingson

Validate NthWeekday recurrence specs instead of silently falling back An out-of-range week or weekday yielded the un-adjusted base date, which reads as 'same day next month'. Sync and import can both inject one. Reject a malformed spec, and roll a valid one that a month lacks (a 5th Monday) forward to the next month that has it.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-25 17:16 UTC
Signed with PGP, not checked
Commit: c3545b2c2199db925b9dd47107ea7f45c71d3e76
Parent: 419f18c
2 files changed, +101 insertions, -15 deletions
M crates/core/src/recurrence.rs +97 -14
@@ -237,19 +237,36 @@
237 237 Some(next)
238 238 }
239 239 Some(MonthlySpec::NthWeekday { week, weekday }) => {
240 - let next_base = add_months(base, interval as i32, None, tz);
241 - let next_local = next_base.with_timezone(&tz);
242 - let target = nth_weekday_in_month(
243 - next_local.year(),
244 - next_local.month(),
245 - *week,
246 - *weekday,
247 - next_local.hour(),
248 - next_local.minute(),
249 - next_local.second(),
250 - tz,
251 - );
252 - Some(target.unwrap_or(next_base))
240 + // `week`/`weekday` are untrusted (sync and import can inject
241 + // either). A malformed spec has no meaningful occurrence, so
242 + // reject it rather than falling back to the un-adjusted base
243 + // date, which silently reads as "same day next month".
244 + if !nth_weekday_spec_is_valid(*week, *weekday) {
245 + return None;
246 + }
247 + // A well-formed spec can still miss a given month: a 5th
248 + // Monday only exists in some. Roll forward by `interval` to
249 + // the next month that has one, again rather than falling
250 + // back to the base date.
251 + let mut months = interval as i32;
252 + for _ in 0..MAX_NTH_WEEKDAY_MONTH_SCAN {
253 + let candidate = add_months(base, months, None, tz).with_timezone(&tz);
254 + let target = nth_weekday_in_month(
255 + candidate.year(),
256 + candidate.month(),
257 + *week,
258 + *weekday,
259 + candidate.hour(),
260 + candidate.minute(),
261 + candidate.second(),
262 + tz,
263 + );
264 + if target.is_some() {
265 + return target;
266 + }
267 + months += interval as i32;
268 + }
269 + None
253 270 }
254 271 None => {
255 272 // Same as legacy monthly (end-of-month intent read in `tz`)
@@ -270,8 +287,21 @@
270 287 }
271 288 }
272 289
290 + /// How many `interval`-spaced months to scan for a well-formed `NthWeekday`
291 + /// spec before giving up. A 5th weekday exists in roughly a third of months, so
292 + /// a monthly rule lands within a few; the cap only bounds pathological
293 + /// interval/weekday combinations.
294 + const MAX_NTH_WEEKDAY_MONTH_SCAN: u32 = 48;
295 +
296 + /// Whether an `NthWeekday` spec is well-formed: `week` is 1-5 or -1 (last), and
297 + /// `weekday` is 0=Mon..6=Sun. Anything else is corrupt rather than merely absent
298 + /// from a given month.
299 + fn nth_weekday_spec_is_valid(week: i8, weekday: u8) -> bool {
300 + matches!(week, -1 | 1..=5) && weekday <= 6
301 + }
302 +
273 303 /// Find the Nth weekday in a given month, constructing the result in `tz`.
274 - /// `week`: 1-4 for ordinal, -1 for last.
304 + /// `week`: 1-5 for ordinal, -1 for last.
275 305 /// `weekday`: 0=Mon..6=Sun.
276 306 /// `year`/`month`/`hour`/`minute`/`second` are civil fields in `tz`.
277 307 #[allow(clippy::too_many_arguments)] // civil date/time fields are clearer flat than boxed in a struct
@@ -794,6 +824,59 @@
794 824 assert_eq!(next.day(), 23);
795 825 }
796 826
827 + #[test]
828 + fn test_rich_monthly_nth_weekday_rejects_out_of_range_week() {
829 + let jan = Utc.with_ymd_and_hms(2026, 1, 9, 10, 0, 0).unwrap();
830 + for week in [0i8, 6, 7, -2, i8::MIN, i8::MAX] {
831 + let rule = RecurrenceRule {
832 + pattern: Recurrence::Monthly,
833 + interval: 1,
834 + weekdays: vec![],
835 + monthly_spec: Some(MonthlySpec::NthWeekday { week, weekday: 4 }),
836 + };
837 + // Previously this silently yielded the un-adjusted base (Feb 9).
838 + assert_eq!(
839 + calculate_next_due_rich(Some(&jan), &rule),
840 + None,
841 + "week {week} should be rejected, not silently ignored"
842 + );
843 + }
844 + }
845 +
846 + #[test]
847 + fn test_rich_monthly_nth_weekday_rejects_out_of_range_weekday() {
848 + let jan = Utc.with_ymd_and_hms(2026, 1, 9, 10, 0, 0).unwrap();
849 + let rule = RecurrenceRule {
850 + pattern: Recurrence::Monthly,
851 + interval: 1,
852 + weekdays: vec![],
853 + monthly_spec: Some(MonthlySpec::NthWeekday {
854 + week: 2,
855 + weekday: 200,
856 + }),
857 + };
858 + assert_eq!(calculate_next_due_rich(Some(&jan), &rule), None);
859 + }
860 +
861 + #[test]
862 + fn test_rich_monthly_nth_weekday_skips_months_without_occurrence() {
863 + // 5th Monday: Mar 2026 has one (Mar 30), Apr and May 2026 do not,
864 + // Jun 2026 does (Jun 29). The gap months must be skipped, not
865 + // collapsed onto the base date.
866 + let mar = Utc.with_ymd_and_hms(2026, 3, 30, 10, 0, 0).unwrap();
867 + let rule = RecurrenceRule {
868 + pattern: Recurrence::Monthly,
869 + interval: 1,
870 + weekdays: vec![],
871 + monthly_spec: Some(MonthlySpec::NthWeekday {
872 + week: 5,
873 + weekday: 0,
874 + }),
875 + };
876 + let next = calculate_next_due_rich(Some(&mar), &rule).unwrap();
877 + assert_eq!((next.month(), next.day()), (6, 29));
878 + }
879 +
797 880 #[test]
798 881 fn test_expand_recurrence_weekly() {
799 882 let start = Utc.with_ymd_and_hms(2026, 3, 2, 10, 0, 0).unwrap(); // Monday
M crates/core/src/models/shared.rs +4 -1
@@ -257,8 +257,11 @@
257 257 /// Specific day of month (1-31), clamped to the month's length.
258 258 DayOfMonth { day: u32 },
259 259 /// Nth weekday of the month (e.g., 2nd Friday, last Monday).
260 - /// `week`: 1-4 for fixed, -1 for last.
260 + /// `week`: 1-5 for fixed, -1 for last.
261 261 /// `weekday`: 0=Mon..6=Sun.
262 + /// Values outside those ranges are rejected when the next occurrence is
263 + /// calculated; a 5th weekday that a month lacks rolls to the next month
264 + /// that has one.
262 265 NthWeekday { week: i8, weekday: u8 },
263 266 }
264 267