From 25e48f1b9fff10866e4d6a0f875912527a5160bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Fri, 14 Dec 2018 18:58:35 +0200 Subject: [PATCH] croniter.is_valid() throws exceptions for some bad inputs, so must use try ... except --- hc/front/tests/test_cron_preview.py | 9 ++++++++- hc/front/tests/test_update_timeout.py | 21 ++++++++++++--------- hc/front/validators.py | 4 +++- hc/front/views.py | 26 +++++++++++++------------- 4 files changed, 36 insertions(+), 24 deletions(-) diff --git a/hc/front/tests/test_cron_preview.py b/hc/front/tests/test_cron_preview.py index 5ed68d0e..791ebc9d 100644 --- a/hc/front/tests/test_cron_preview.py +++ b/hc/front/tests/test_cron_preview.py @@ -16,7 +16,14 @@ class CronPreviewTestCase(BaseTestCase): self.assertContains(r, "cron-preview-title", status_code=200) def test_it_rejects_invalid_cron_expression(self): - for schedule in [None, "", "*", "100 100 100 100 100", "* * * * * *"]: + samples = [None, + "", + "*", + "100 100 100 100 100", + "* * * * * *", + "1,2 3,* * * *"] + + for schedule in samples: payload = {"schedule": schedule, "tz": "UTC"} r = self.client.post("/checks/cron_preview/", payload) self.assertContains(r, "Invalid cron expression", status_code=200) diff --git a/hc/front/tests/test_update_timeout.py b/hc/front/tests/test_update_timeout.py index c4a03933..d90ba1de 100644 --- a/hc/front/tests/test_update_timeout.py +++ b/hc/front/tests/test_update_timeout.py @@ -67,16 +67,19 @@ class UpdateTimeoutTestCase(BaseTestCase): self.assertEqual(self.check.schedule, "5 * * * *") def test_it_validates_cron_expression(self): - payload = { - "kind": "cron", - "schedule": "* invalid *", - "tz": "UTC", - "grace": 60 - } - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, data=payload) - self.assertEqual(r.status_code, 400) + samples = ["* invalid *", "1,2 3,* * * *"] + + for sample in samples: + payload = { + "kind": "cron", + "schedule": sample, + "tz": "UTC", + "grace": 60 + } + + r = self.client.post(self.url, data=payload) + self.assertEqual(r.status_code, 400) # Check should still have its original data: self.check.refresh_from_db() diff --git a/hc/front/validators.py b/hc/front/validators.py index 89750ff9..0b5fc8f2 100644 --- a/hc/front/validators.py +++ b/hc/front/validators.py @@ -24,7 +24,9 @@ class CronExpressionValidator(object): if len(value.split()) != 5: raise ValidationError(message=self.message) - if not croniter.is_valid(value): + try: + croniter(value) + except: raise ValidationError(message=self.message) diff --git a/hc/front/views.py b/hc/front/views.py index 29700479..71a7b442 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -325,21 +325,21 @@ def cron_preview(request): tz = request.POST.get("tz") ctx = {"tz": tz, "dates": []} - if len(schedule.split()) != 5: - ctx["bad_schedule"] = True - elif not croniter.is_valid(schedule): + try: + zone = pytz.timezone(tz) + now_local = timezone.localtime(timezone.now(), zone) + + if len(schedule.split()) != 5: + raise ValueError() + + it = croniter(schedule, now_local) + for i in range(0, 6): + ctx["dates"].append(it.get_next(datetime)) + except UnknownTimeZoneError: + ctx["bad_tz"] = True + except: ctx["bad_schedule"] = True - if "bad_schedule" not in ctx: - try: - zone = pytz.timezone(tz) - now_local = timezone.localtime(timezone.now(), zone) - it = croniter(schedule, now_local) - for i in range(0, 6): - ctx["dates"].append(it.get_next(datetime)) - except UnknownTimeZoneError: - ctx["bad_tz"] = True - return render(request, "front/cron_preview.html", ctx)