Skip to content

fix(cron): validate comma-separated lists of ranges and steps#463

Open
patchwright wants to merge 1 commit into
python-validators:masterfrom
patchwright:fix-cron-list-of-ranges
Open

fix(cron): validate comma-separated lists of ranges and steps#463
patchwright wants to merge 1 commit into
python-validators:masterfrom
patchwright:fix-cron-list-of-ranges

Conversation

@patchwright

Copy link
Copy Markdown

Summary

cron() rejects valid expressions whose components are comma-separated lists of ranges or steps, e.g. cron("1-5,10-20 * * * *") raises ValidationError even though the expression is valid.

Reproduction

from validators import cron

cron("1-5,10-20 * * * *")   # ValidationError (incorrect)
cron("1-5,30 * * * *")      # ValidationError (incorrect)

Validity currently depends on element order, which is the clearest sign this is a bug rather than an intended limitation:

cron("30,1-5 * * * *")      # True
cron("1-5,30 * * * *")      # ValidationError  (same tokens, different order)

Cause

In src/validators/cron.py, _validate_cron_component checks the "-" and "/" branches before the "," branch. A component like "1-5,10-20" therefore matches the range branch, split("-") yields three parts, and the whole expression is rejected.

Fix

Evaluate the "," branch first, recursing on each element so each element can itself be a single value, a range ("1-5"), or a step ("*/2").

     if component == "*":
         return True

+    if "," in component:
+        for item in component.split(","):
+            if not _validate_cron_component(item, min_val, max_val):
+                return False
+        return True
+
     if component.isdecimal():

No previously-accepted input changes behaviour: any component containing a comma previously hit the range branch and was rejected, so this only accepts inputs that were wrongly rejected. The existing dead commented all(...) block is removed.

Tests

Adds four cases to test_returns_true_on_valid_cron:

  • "1-5,10-20 * * * *"
  • "1-5,30 * * * *"
  • "10,20-30 * * * *"
  • "1-5,10-20,30-40 * * * *"

Each fails on the current code and passes with the fix. Full suite: 895 → 899 passing. ruff check / ruff format clean.

I couldn't find an existing issue or PR covering this — happy to fold it into one if I missed it.

The comma-list branch in _validate_cron_component was evaluated after
the '-' and '/' branches, so any list element that was itself a range
(e.g. '1-5') or step (e.g. '*/2') short-circuited and the whole
expression was rejected. Move the comma-list split to run first, so
each element is validated as a range/step/single value.

Adds regression cases for list-of-ranges, list-with-single, and
multi-list expressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant