From 801a2cd495c558917127961b163e93b5209a562d Mon Sep 17 00:00:00 2001 From: Jef Roosens Date: Sat, 14 Jan 2023 18:44:33 +0100 Subject: [PATCH] fix(cron): ensure valid day values; some other stuff --- Makefile | 2 +- src/cron/c/expression.c | 7 +- src/cron/c/expression.h | 8 ++- src/cron/c/parse.c | 130 +++++++++++++++++++++++++++---------- src/cron/expression.v | 11 ++-- src/cron/expression_test.v | 2 +- 6 files changed, 112 insertions(+), 48 deletions(-) diff --git a/Makefile b/Makefile index c71ff1f..3571d30 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,6 @@ # =====CONFIG===== SRC_DIR := src -SOURCES != find '$(SRC_DIR)' -iname '*.v' +SOURCES != find '$(SRC_DIR)' -\( -iname '*.v' -or -iname '*.h' -or -iname '*.c' -\) V_PATH ?= v V := $(V_PATH) -showcc -gc boehm -W -d use_openssl -skip-unused diff --git a/src/cron/c/expression.c b/src/cron/c/expression.c index ed0306f..0051e49 100644 --- a/src/cron/c/expression.c +++ b/src/cron/c/expression.c @@ -84,11 +84,6 @@ int ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *ref) { while (out->day > month_days[ce->months[month_index % ce->month_count] - 1]) { month_index++; - - // TODO find out if this can happen - if (month_index == 2 * ce->month_count) { - return 1; - } } } @@ -108,7 +103,7 @@ int ce_next_from_now(cron_simple_time *out, cron_expression *ce) { struct tm gm; gmtime_r(&t, &gm); - struct cron_simple_time ref = { + cron_simple_time ref = { .year = gm.tm_year, // tm_mon goes from 0 to 11 .month = gm.tm_mon + 1, diff --git a/src/cron/c/expression.h b/src/cron/c/expression.h index c9441f6..1c86436 100644 --- a/src/cron/c/expression.h +++ b/src/cron/c/expression.h @@ -1,3 +1,6 @@ +#ifndef VIETER_CRON +#define VIETER_CRON + #include #include #include @@ -7,7 +10,8 @@ typedef enum cron_parse_error { cron_parse_ok = 0, cron_parse_invalid_expression = 1, cron_parse_invalid_number = 2, - cron_parse_out_of_range = 3 + cron_parse_out_of_range = 3, + cron_parse_too_many_parts = 4 } cron_parse_error; typedef struct cron_expression { @@ -38,3 +42,5 @@ int cron_ce_next(cron_simple_time *out, cron_expression *ce, cron_simple_time *r int cron_ce_next_from_now(cron_simple_time *out, cron_expression *ce); enum cron_parse_error cron_ce_parse_expression(cron_expression *out, char *s); + +#endif diff --git a/src/cron/c/parse.c b/src/cron/c/parse.c index a577c6d..f54b818 100644 --- a/src/cron/c/parse.c +++ b/src/cron/c/parse.c @@ -1,21 +1,28 @@ #include "expression.h" +const uint8_t month_days[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31}; + // Allowed value ranges for the minute, hour, day and month field const uint8_t min[4] = {0, 0, 1, 1}; const uint8_t max[4] = {59, 23, 31, 12}; -// Convert a string a uint8_t value by parsing it using atoi and checking +const uint8_t min_parts = 2; +const uint8_t max_parts = 4; + +// Convert a string into a uint8_t value by parsing it using atoi and checking // whether it's contained within the given range #define SAFE_ATOI(v,s,min,max) \ int _##v = atoi(s); \ if ((_##v) == 0 && strcmp((s), "0") != 0) { \ return cron_parse_invalid_number; \ } \ - if (v < (min) || v > (max)) { \ + if (((_##v) < (min)) || ((_##v) > (max))) { \ return cron_parse_out_of_range; \ } \ v = (uint8_t) (_##v); +#define MAX(x, y) (((x) > (y)) ? (x) : (y)) + /** * Given a range expression, produce a bit field defining what numbers in the * min-max range the expression represents. Bit 0 (starting from the @@ -32,7 +39,7 @@ const uint8_t max[4] = {59, 23, 31, 12}; * - a/c * - a-b/c */ -enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { +cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_t max) { // The * expression means "every possible value" if (s[0] == '*') { // A '*' is only valid on its own @@ -98,7 +105,7 @@ enum cron_parse_error ce_parse_range(uint64_t *out, char *s, uint8_t min, uint8_ * min-max range the part represents. A part consists of one or more range * expressions, separated by commas. */ -enum cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { +cron_parse_error ce_parse_part(uint64_t *out, char *s, uint8_t min, uint8_t max) { *out = 0; char *next; @@ -175,28 +182,30 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { s = strdup(s); char *orig_s = s; - uint8_t part_count = 0; - - char *next; enum cron_parse_error res = cron_parse_ok; - uint64_t bfs[4]; + uint64_t bfs[max_parts]; + + // First we divide the input string into its parts, divided by spaces. + // Each part is delimited by a NULL byte. + uint8_t part_count = 0; + char *parts[max_parts]; + char *next; // Skip leading spaces - while (s[0] == ' ') { - s++; + size_t offset = 0; + + while (s[offset] == ' ') { + offset++; } - while (part_count < 4 && ((next = strchr(s, ' ')) != NULL)) { + s += offset; + + while (part_count < max_parts && ((next = strchr(s, ' ')) != NULL)) { next[0] = '\0'; - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); - - if (res != cron_parse_ok) { - goto end; - } - - size_t offset = 1; + parts[part_count] = s; // Skip multiple spaces + offset = 1; while (next[offset] == ' ') { offset++; } @@ -205,34 +214,87 @@ enum cron_parse_error ce_parse_expression(cron_expression *out, char *s) { part_count++; } - // Parse final trailing part - if (part_count < 4 && s[0] != '\0') { - res = ce_parse_part(&bfs[part_count], s, min[part_count], max[part_count]); + // The loop exited because we already have 4 parts, yet there's still at + // least one more part that follows. + if (next != NULL) { + res = cron_parse_too_many_parts; + } else if (s[0] != '\0') { + // There's one more excessive trailing part + if (part_count == max_parts) { + res = cron_parse_too_many_parts; + goto end; + } + + parts[part_count] = s; + part_count++; + } + + + // We now parse the parts in reverse. This is because the month part + // determines the maximum value of the day part. + + uint64_t bit_field = 0; + + // Months + if (part_count >= 4) { + res = ce_parse_part(&bit_field, parts[3], min[3], max[3]); if (res != cron_parse_ok) { goto end; } - part_count++; + out->month_count = bf_to_nums(&out->months, bit_field, min[3], max[3]); + } + // If months aren't provided, they're replaced with a * + else { + out->month_count = bf_to_nums(&out->months, ~0, min[3], max[3]); } - // At least two parts need to be provided - if (part_count < 2) { - res = cron_parse_invalid_expression; + // Determine what the largest allowed day value is, given the months + uint8_t max_day_value = 0; + + for (uint8_t i = 0; i < out->month_count; i++) { + max_day_value = MAX(max_day_value, month_days[out->months[i] - 1]); + } + + // Days + if (part_count >= 3) { + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[2], min[2], max_day_value); + + if (res != cron_parse_ok) { + goto end; + } + + out->day_count = bf_to_nums(&out->days, bit_field, min[2], max_day_value); + } + // If days aren't provided, they're replaced with a * + else { + out->day_count = bf_to_nums(&out->days, ~0, min[2], max_day_value); + } + + // Hours + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[1], min[1], max[1]); + + if (res != cron_parse_ok) { goto end; } - // Ensure there's always 4 parts, as expressions can have between 2 and 4 parts - while (part_count < 4) { - // Expression is augmented with '*' expressions - bfs[part_count] = ~0; - part_count++; + out->hour_count = bf_to_nums(&out->hours, bit_field, min[1], max[1]); + + // Minutes + bit_field = 0; + + res = ce_parse_part(&bit_field, parts[0], min[0], max[0]); + + if (res != cron_parse_ok) { + goto end; } - out->minute_count = bf_to_nums(&out->minutes, bfs[0], min[0], max[0]); - out->hour_count = bf_to_nums(&out->hours, bfs[1], min[1], max[1]); - out->day_count = bf_to_nums(&out->days, bfs[2], min[2], max[2]); - out->month_count = bf_to_nums(&out->months, bfs[3], min[3], max[3]); + out->minute_count = bf_to_nums(&out->minutes, bit_field, min[0], max[0]); end: // s is cloned diff --git a/src/cron/expression.v b/src/cron/expression.v index 0429b93..23e1354 100644 --- a/src/cron/expression.v +++ b/src/cron/expression.v @@ -2,21 +2,22 @@ module cron import time +[unsafe] +pub fn (ce &Expression) free() { + C.ce_free(ce) +} + pub fn parse_expression(exp string) !&Expression { out := C.ce_init() res := C.ce_parse_expression(out, exp.str) if res != 0 { - return error('yuhh') + return error(res.str()) } return out } -pub fn (ce &Expression) free() { - C.ce_free(ce) -} - pub fn (ce &Expression) next(ref time.Time) !time.Time { st := SimpleTime{ year: ref.year diff --git a/src/cron/expression_test.v b/src/cron/expression_test.v index e1a9849..d6ec002 100644 --- a/src/cron/expression_test.v +++ b/src/cron/expression_test.v @@ -22,7 +22,7 @@ fn test_next_simple() ! { // util_test_time('0 3', '2002-01-01 00:00:00', '2002-01-01 03:00:00')! // Overlap to next day - mut exp := '0 3 ' + mut exp := '0 3 ' util_test_time(exp, '2002-01-01 03:00:00', '2002-01-02 03:00:00')! util_test_time(exp, '2002-01-01 04:00:00', '2002-01-02 03:00:00')!