Merge pull request #23336 from keszybz/fuzz-calendarspec-more-coverage

More coverage in fuzz-calendarspec

src/basic/time-util.c

@@ -29,10 +29,10 @@
 
 static clockid_t map_clock_id(clockid_t c) {
 
-        /* Some more exotic archs (s390, ppc, …) lack the "ALARM" flavour of the clocks. Thus, clock_gettime() will
+        /* Some more exotic archs (s390, ppc, …) lack the "ALARM" flavour of the clocks. Thus,
-         * fail for them. Since they are essentially the same as their non-ALARM pendants (their only difference is
+         * clock_gettime() will fail for them. Since they are essentially the same as their non-ALARM
-         * when timers are set on them), let's just map them accordingly. This way, we can get the correct time even on
+         * pendants (their only difference is when timers are set on them), let's just map them
-         * those archs. */
+         * accordingly. This way, we can get the correct time even on those archs. */
 
         switch (c) {
 
@@ -295,8 +295,8 @@ char *format_timestamp_style(
                 usec_t t,
                 TimestampStyle style) {
 
-        /* The weekdays in non-localized (English) form. We use this instead of the localized form, so that our
+        /* The weekdays in non-localized (English) form. We use this instead of the localized form, so that
-         * generated timestamps may be parsed with parse_timestamp(), and always read the same. */
+         * our generated timestamps may be parsed with parse_timestamp(), and always read the same. */
         static const char * const weekdays[] = {
                 [0] = "Sun",
                 [1] = "Mon",
@@ -383,8 +383,8 @@ char *format_timestamp_style(
         /* Append the timezone */
         n = strlen(buf);
         if (utc) {
-                /* If this is UTC then let's explicitly use the "UTC" string here, because gmtime_r() normally uses the
+                /* If this is UTC then let's explicitly use the "UTC" string here, because gmtime_r()
-                 * obsolete "GMT" instead. */
+                 * normally uses the obsolete "GMT" instead. */
                 if (n + 5 > l)
                         return NULL; /* "UTC" doesn't fit. */
 
@@ -399,12 +399,14 @@ char *format_timestamp_style(
                         /* The full time zone does not fit in. Yuck. */
 
                         if (n + 1 + _POSIX_TZNAME_MAX + 1 > l)
-                                return NULL; /* Not even enough space for the POSIX minimum (of 6)? In that case, complain that it doesn't fit */
+                                return NULL; /* Not even enough space for the POSIX minimum (of 6)? In that
+                                              * case, complain that it doesn't fit. */
 
-                        /* So the time zone doesn't fit in fully, but the caller passed enough space for the POSIX
+                        /* So the time zone doesn't fit in fully, but the caller passed enough space for the
-                         * minimum time zone length. In this case suppress the timezone entirely, in order not to dump
+                         * POSIX minimum time zone length. In this case suppress the timezone entirely, in
-                         * an overly long, hard to read string on the user. This should be safe, because the user will
+                         * order not to dump an overly long, hard to read string on the user. This should be
-                         * assume the local timezone anyway if none is shown. And so does parse_timestamp(). */
+                         * safe, because the user will assume the local timezone anyway if none is shown. And
+                         * so does parse_timestamp(). */
                 } else {
                         buf[n++] = ' ';
                         strcpy(buf + n, tm.tm_zone);
@@ -700,10 +702,11 @@ static int parse_timestamp_impl(const char *t, usec_t *usec, bool with_tz) {
 
                         tzset();
 
-                        /* See if the timestamp is suffixed by either the DST or non-DST local timezone. Note that we only
+                        /* See if the timestamp is suffixed by either the DST or non-DST local timezone. Note
-                         * support the local timezones here, nothing else. Not because we wouldn't want to, but simply because
+                         * that we only support the local timezones here, nothing else. Not because we
-                         * there are no nice APIs available to cover this. By accepting the local time zone strings, we make
+                         * wouldn't want to, but simply because there are no nice APIs available to cover
-                         * sure that all timestamps written by format_timestamp() can be parsed correctly, even though we don't
+                         * this. By accepting the local time zone strings, we make sure that all timestamps
+                         * written by format_timestamp() can be parsed correctly, even though we don't
                          * support arbitrary timezone specifications. */
 
                         for (j = 0; j <= 1; j++) {

src/fuzz/fuzz-calendarspec.c

@@ -4,20 +4,54 @@
 #include "calendarspec.h"
 #include "fd-util.h"
 #include "fuzz.h"
+#include "string-util.h"
+#include "time-util.h"
 
 int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
         _cleanup_(calendar_spec_freep) CalendarSpec *cspec = NULL;
-        _cleanup_free_ char *str = NULL, *p = NULL;
+        _cleanup_free_ char *str = NULL;
+        int r;
 
         if (!getenv("SYSTEMD_LOG_LEVEL"))
                 log_set_max_level(LOG_CRIT);
 
         str = memdup_suffix0(data, size);
 
-        if (calendar_spec_from_string(str, &cspec) >= 0) {
+        size_t l1 = strlen(str);
-                (void) calendar_spec_valid(cspec);
+        const char* usecs = l1 < size ? str + l1 + 1 : "";
-                (void) calendar_spec_normalize(cspec);
+
-                (void) calendar_spec_to_string(cspec, &p);
+        r = calendar_spec_from_string(str, &cspec);
+        if (r < 0) {
+                log_debug_errno(r, "Failed to parse \"%s\": %m", str);
+                return 0;
+        }
+
+        _cleanup_free_ char *p = NULL;
+        assert_se(calendar_spec_valid(cspec));
+        assert_se(calendar_spec_to_string(cspec, &p) == 0);
+        assert(p);
+
+        log_debug("spec: %s → %s", str, p);
+
+        _cleanup_(calendar_spec_freep) CalendarSpec *cspec2 = NULL;
+        assert_se(calendar_spec_from_string(p, &cspec2) >= 0);
+        assert_se(calendar_spec_valid(cspec2));
+
+        usec_t usec = 0;
+        (void) parse_time(usecs, &usec, 1);
+
+        /* If timezone is set, calendar_spec_next_usec() would fork, bleh :(
+         * Let's not try that. */
+        cspec->timezone = mfree(cspec->timezone);
+
+        log_debug("00: %s", strna(FORMAT_TIMESTAMP(usec)));
+        for (unsigned i = 1; i <= 20; i++) {
+                r = calendar_spec_next_usec(cspec, usec, &usec);
+                if (r < 0) {
+                        log_debug_errno(r, "%02u: %m", i);
+                        break;
+                }
+                log_debug("%02u: %s", i, FORMAT_TIMESTAMP(usec));
         }
 
         return 0;

src/shared/calendarspec.c

@@ -145,7 +145,7 @@ static void fix_year(CalendarComponent *c) {
         }
 }
 
-int calendar_spec_normalize(CalendarSpec *c) {
+static void calendar_spec_normalize(CalendarSpec *c) {
         assert(c);
 
         if (streq_ptr(c->timezone, "UTC")) {
@@ -167,8 +167,6 @@ int calendar_spec_normalize(CalendarSpec *c) {
         normalize_chain(&c->hour);
         normalize_chain(&c->minute);
         normalize_chain(&c->microsecond);
-
-        return 0;
 }
 
 static bool chain_valid(CalendarComponent *c, int from, int to, bool end_of_month) {
@@ -290,17 +288,24 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) {
         }
 }
 
-static void format_chain(FILE *f, int space, const CalendarComponent *c, bool usec) {
+static bool chain_is_star(const CalendarComponent *c, bool usec) {
+        /* Return true if the whole chain can be replaced by '*'.
+         * This happens when the chain is empty or one of the components covers all. */
+        if (!c)
+                return true;
+        if (usec)
+                for (; c; c = c->next)
+                        if (c->start == 0 && c->stop < 0 && c->repeat == USEC_PER_SEC)
+                                return true;
+        return false;
+}
+
+static void _format_chain(FILE *f, int space, const CalendarComponent *c, bool start, bool usec) {
         int d = usec ? (int) USEC_PER_SEC : 1;
 
         assert(f);
 
-        if (!c) {
+        if (start && chain_is_star(c, usec)) {
-                fputc('*', f);
-                return;
-        }
-
-        if (usec && c->start == 0 && c->repeat == USEC_PER_SEC && !c->next) {
                 fputc('*', f);
                 return;
         }
@@ -323,10 +328,14 @@ static void format_chain(FILE *f, int space, const CalendarComponent *c, bool us
 
         if (c->next) {
                 fputc(',', f);
-                format_chain(f, space, c->next, usec);
+                _format_chain(f, space, c->next, false, usec);
         }
 }
 
+static void format_chain(FILE *f, int space, const CalendarComponent *c, bool usec) {
+        _format_chain(f, space, c, /* start = */ true, usec);
+}
+
 int calendar_spec_to_string(const CalendarSpec *c, char **p) {
         char *buf = NULL;
         size_t sz = 0;
@@ -431,12 +440,10 @@ static int parse_weekdays(const char **p, CalendarSpec *c) {
                         c->weekdays_bits |= 1 << day_nr[i].nr;
 
                         if (l >= 0) {
-                                int j;
-
                                 if (l > day_nr[i].nr)
                                         return -EINVAL;
 
-                                for (j = l + 1; j < day_nr[i].nr; j++)
+                                for (int j = l + 1; j < day_nr[i].nr; j++)
                                         c->weekdays_bits |= 1 << j;
                         }
 
@@ -1086,9 +1093,7 @@ int calendar_spec_from_string(const char *p, CalendarSpec **spec) {
                         return -EINVAL;
         }
 
-        r = calendar_spec_normalize(c);
+        calendar_spec_normalize(c);
-        if (r < 0)
-                return r;
 
         if (!calendar_spec_valid(c))
                 return -EINVAL;

src/shared/calendarspec.h

@@ -35,7 +35,6 @@ typedef struct CalendarSpec {
 
 CalendarSpec* calendar_spec_free(CalendarSpec *c);
 
-int calendar_spec_normalize(CalendarSpec *spec);
 bool calendar_spec_valid(CalendarSpec *spec);
 
 int calendar_spec_to_string(const CalendarSpec *spec, char **p);

src/test/test-calendarspec.c

@@ -13,10 +13,15 @@ static void _test_one(int line, const char *input, const char *output) {
         usec_t u;
         int r;
 
-        assert_se(calendar_spec_from_string(input, &c) >= 0);
+        r = calendar_spec_from_string(input, &c);
+        if (r < 0)
+                log_error_errno(r, "Failed to parse \"%s\": %m", input);
+        assert_se(r >= 0);
 
         assert_se(calendar_spec_to_string(c, &p) >= 0);
-        log_info("line %d: \"%s\" → \"%s\"", line, input, p);
+        log_info("line %d: \"%s\" → \"%s\"%s%s", line, input, p,
+                 !streq(p, output) ? " expected:" : "",
+                 !streq(p, output) ? output : "");
 
         assert_se(streq(p, output));
 
@@ -157,6 +162,9 @@ TEST(calendar_spec_one) {
         test_one("00:00:1.0..3.8", "*-*-* 00:00:01..03");
         test_one("00:00:01..03", "*-*-* 00:00:01..03");
         test_one("00:00:01/2,02..03", "*-*-* 00:00:01/2,02..03");
+        test_one("*:4,30:0..3", "*-*-* *:04,30:00..03");
+        test_one("*:4,30:0/1", "*-*-* *:04,30:*");
+        test_one("*:4,30:0/1,3,5", "*-*-* *:04,30:*");
         test_one("*-*~1 Utc", "*-*~01 00:00:00 UTC");
         test_one("*-*~05,3 ", "*-*~03,05 00:00:00");
         test_one("*-*~* 00:00:00", "*-*-* 00:00:00");
@@ -222,31 +230,34 @@ TEST(calendar_spec_next) {
 TEST(calendar_spec_from_string) {
         CalendarSpec *c;
 
-        assert_se(calendar_spec_from_string("test", &c) < 0);
+        assert_se(calendar_spec_from_string("test", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string(" utc", &c) < 0);
+        assert_se(calendar_spec_from_string(" utc", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("    ", &c) < 0);
+        assert_se(calendar_spec_from_string("    ", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("", &c) < 0);
+        assert_se(calendar_spec_from_string("", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("7", &c) < 0);
+        assert_se(calendar_spec_from_string("7", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("121212:1:2", &c) < 0);
+        assert_se(calendar_spec_from_string("121212:1:2", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("2000-03-05.23 00:00:00", &c) < 0);
+        assert_se(calendar_spec_from_string("2000-03-05.23 00:00:00", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("2000-03-05 00:00.1:00", &c) < 0);
+        assert_se(calendar_spec_from_string("2000-03-05 00:00.1:00", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("00:00:00/0.00000001", &c) < 0);
+        assert_se(calendar_spec_from_string("00:00:00/0.00000001", &c) == -ERANGE);
-        assert_se(calendar_spec_from_string("00:00:00.0..00.9", &c) < 0);
+        assert_se(calendar_spec_from_string("00:00:00.0..00.9", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("2016~11-22", &c) < 0);
+        assert_se(calendar_spec_from_string("2016~11-22", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("*-*~5/5", &c) < 0);
+        assert_se(calendar_spec_from_string("*-*~5/5", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("Monday.. 12:00", &c) < 0);
+        assert_se(calendar_spec_from_string("Monday.. 12:00", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("Monday..", &c) < 0);
+        assert_se(calendar_spec_from_string("Monday..", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("-00:+00/-5", &c) < 0);
+        assert_se(calendar_spec_from_string("-00:+00/-5", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("00:+00/-5", &c) < 0);
+        assert_se(calendar_spec_from_string("00:+00/-5", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("2016- 11- 24 12: 30: 00", &c) < 0);
+        assert_se(calendar_spec_from_string("2016- 11- 24 12: 30: 00", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("*~29", &c) < 0);
+        assert_se(calendar_spec_from_string("*~29", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("*~16..31", &c) < 0);
+        assert_se(calendar_spec_from_string("*~16..31", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("12..1/2-*", &c) < 0);
+        assert_se(calendar_spec_from_string("12..1/2-*", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("20/4:00", &c) < 0);
+        assert_se(calendar_spec_from_string("20/4:00", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("00:00/60", &c) < 0);
+        assert_se(calendar_spec_from_string("00:00/60", &c) == -EINVAL);
-        assert_se(calendar_spec_from_string("00:00:2300", &c) < 0);
+        assert_se(calendar_spec_from_string("00:00:2300", &c) == -ERANGE);
-        assert_se(calendar_spec_from_string("00:00:18446744073709551615", &c) < 0);
+        assert_se(calendar_spec_from_string("00:00:18446744073709551615", &c) == -ERANGE);
         assert_se(calendar_spec_from_string("@88588582097858858", &c) == -ERANGE);
+        assert_se(calendar_spec_from_string("*:4,30:*,5", &c) == -EINVAL);
+        assert_se(calendar_spec_from_string("*:4,30:5,*", &c) == -EINVAL);
+        assert_se(calendar_spec_from_string("*:4,30:*\n", &c) == -EINVAL);
 }
 
 DEFINE_TEST_MAIN(LOG_INFO);

test/fuzz/fuzz-calendarspec/cant-parse-printed

@@ -0,0 +1 @@
+*:4,30:0..5,0

test/fuzz/fuzz-calendarspec/crash-parse-star-non-star

@@ -0,0 +1 @@
+*:4,30:0/01,0

test/fuzz/fuzz-calendarspec/print-loses-spec

@@ -0,0 +1 @@
+*:4,30:0..3