calendarspec: warn on weekday/date conflict in systemd-analyze and systemd-run
When a fixed date (e.g. 2027-01-01) is paired with a weekday constraint (e.g. Thu) that does not match, the timer silently never elapses. Add calendar_spec_from_string_full(..., warn_on_weekday_mismatch) so user-facing tools can opt in to a log_warning() at parse time: - systemd-analyze calendar: uses _full(true) - systemd-run --on-calendar: uses _full(true) - .timer OnCalendar=: uses log_syntax() with file/line context Add test_calendar_spec_weekday_conflict(): forks a child with stderr captured in a memfd via pidref_safe_fork_full(), verifies the warning is emitted for conflicting specs and suppressed for valid ones. Fixes: #40350 Signed-off-by: dongshengyuan <dongshengyuan@uniontech.com>
This commit is contained in:
committed by
Lennart Poettering
parent
9d37c618ee
commit
21de611f69
@@ -19,7 +19,7 @@ static int test_calendar_one(usec_t n, const char *p) {
|
||||
TableCell *cell;
|
||||
int r;
|
||||
|
||||
r = calendar_spec_from_string(p, &spec);
|
||||
r = calendar_spec_from_string_full(p, &spec, /* warn_on_weekday_mismatch= */ true);
|
||||
if (r < 0) {
|
||||
log_error_errno(r, "Failed to parse calendar specification '%s': %m", p);
|
||||
time_parsing_hint(p, /* calendar= */ false, /* timestamp= */ true, /* timespan= */ true);
|
||||
|
||||
+18
-15
@@ -321,24 +321,26 @@ struct timeval *timeval_store(struct timeval *tv, usec_t u) {
|
||||
return tv;
|
||||
}
|
||||
|
||||
/* Returns the abbreviated English weekday name for wd in Mon=0 … Sun=6 order.
|
||||
* We use non-localized (English) form so that timestamps can be parsed with
|
||||
* parse_timestamp() and always read the same regardless of locale. */
|
||||
static const char *const weekday_table[_WEEKDAY_MAX] = {
|
||||
[WEEKDAY_MON] = "Mon",
|
||||
[WEEKDAY_TUE] = "Tue",
|
||||
[WEEKDAY_WED] = "Wed",
|
||||
[WEEKDAY_THU] = "Thu",
|
||||
[WEEKDAY_FRI] = "Fri",
|
||||
[WEEKDAY_SAT] = "Sat",
|
||||
[WEEKDAY_SUN] = "Sun",
|
||||
};
|
||||
|
||||
DEFINE_STRING_TABLE_LOOKUP_TO_STRING(weekday, int);
|
||||
|
||||
char* format_timestamp_style(
|
||||
char *buf,
|
||||
size_t l,
|
||||
usec_t t,
|
||||
TimestampStyle style) {
|
||||
|
||||
/* The weekdays in non-localized (English) form. We use this instead of the localized form, so that
|
||||
* our generated timestamps may be parsed with parse_timestamp(), and always read the same. */
|
||||
static const char * const weekdays[] = {
|
||||
[0] = "Sun",
|
||||
[1] = "Mon",
|
||||
[2] = "Tue",
|
||||
[3] = "Wed",
|
||||
[4] = "Thu",
|
||||
[5] = "Fri",
|
||||
[6] = "Sat",
|
||||
};
|
||||
|
||||
struct tm tm;
|
||||
bool utc, us;
|
||||
size_t n;
|
||||
@@ -387,8 +389,9 @@ char* format_timestamp_style(
|
||||
return NULL;
|
||||
|
||||
/* Start with the week day */
|
||||
assert((size_t) tm.tm_wday < ELEMENTSOF(weekdays));
|
||||
memcpy(buf, weekdays[tm.tm_wday], 4);
|
||||
const char *weekday = weekday_to_string(tm.tm_wday == 0 ? 6 : tm.tm_wday - 1);
|
||||
assert(weekday);
|
||||
memcpy(buf, weekday, 4);
|
||||
|
||||
if (style == TIMESTAMP_DATE) {
|
||||
/* Special format string if only date should be shown. */
|
||||
|
||||
@@ -56,6 +56,18 @@ typedef enum TimestampStyle {
|
||||
#define USEC_PER_YEAR ((usec_t) (31557600ULL*USEC_PER_SEC))
|
||||
#define NSEC_PER_YEAR ((nsec_t) (31557600ULL*NSEC_PER_SEC))
|
||||
|
||||
enum {
|
||||
WEEKDAY_MON,
|
||||
WEEKDAY_TUE,
|
||||
WEEKDAY_WED,
|
||||
WEEKDAY_THU,
|
||||
WEEKDAY_FRI,
|
||||
WEEKDAY_SAT,
|
||||
WEEKDAY_SUN,
|
||||
_WEEKDAY_MAX,
|
||||
_WEEKDAY_INVALID = -EINVAL,
|
||||
};
|
||||
|
||||
/* We assume a maximum timezone length of 6. TZNAME_MAX is not defined on Linux, but glibc internally initializes this
|
||||
* to 6. Let's rely on that. */
|
||||
#define FORMAT_TIMESTAMP_MAX (3U+1U+10U+1U+8U+1U+6U+1U+6U+1U)
|
||||
@@ -125,6 +137,10 @@ char* format_timestamp_style(char *buf, size_t l, usec_t t, TimestampStyle style
|
||||
char* format_timestamp_relative_full(char *buf, size_t l, usec_t t, clockid_t clock, bool implicit_left) _warn_unused_result_;
|
||||
char* format_timespan(char *buf, size_t l, usec_t t, usec_t accuracy) _warn_unused_result_;
|
||||
|
||||
/* Returns the abbreviated English weekday name for wd in Mon=0 … Sun=6 order
|
||||
* (matching systemd's weekdays_bits layout). */
|
||||
const char* weekday_to_string(int i);
|
||||
|
||||
_warn_unused_result_
|
||||
static inline char* format_timestamp_relative(char *buf, size_t l, usec_t t) {
|
||||
return format_timestamp_relative_full(buf, l, t, CLOCK_REALTIME, /* implicit_left= */ false);
|
||||
|
||||
@@ -2082,6 +2082,14 @@ int config_parse_timer(
|
||||
log_syntax(unit, LOG_WARNING, filename, line, r, "Failed to parse calendar specification, ignoring: %s", k);
|
||||
return 0;
|
||||
}
|
||||
|
||||
int wday;
|
||||
if (calendar_spec_weekday_conflicts(c, &wday))
|
||||
log_syntax(unit, LOG_WARNING, filename, line, 0,
|
||||
"Weekday constraint does not match the fixed date %04d-%02d-%02d "
|
||||
"(which is a %s), so this timer will never elapse.",
|
||||
c->year->start, c->month->start, c->day->start,
|
||||
weekday_to_string(wday));
|
||||
} else {
|
||||
r = parse_sec(k, &usec);
|
||||
if (r < 0) {
|
||||
|
||||
+13
-8
@@ -552,19 +552,24 @@ static int parse_argv(int argc, char *argv[]) {
|
||||
OPTION_LONG("on-calendar", "SPEC", "Realtime timer"): {
|
||||
_cleanup_(calendar_spec_freep) CalendarSpec *cs = NULL;
|
||||
|
||||
r = calendar_spec_from_string(opts.arg, &cs);
|
||||
r = calendar_spec_from_string_full(opts.arg, &cs, /* warn_on_weekday_mismatch= */ true);
|
||||
if (r < 0)
|
||||
return log_error_errno(r, "Failed to parse calendar event specification: %m");
|
||||
|
||||
/* Let's make sure the given calendar event is not in the past */
|
||||
r = calendar_spec_next_usec(cs, now(CLOCK_REALTIME), NULL);
|
||||
if (r == -ENOENT)
|
||||
/* The calendar event is in the past — let's warn about this, but install it
|
||||
* anyway as is. The service manager will trigger the service right away.
|
||||
* Moreover, the server side might have a different clock or timezone than we
|
||||
* do, hence it should decide when or whether to run something. */
|
||||
log_warning("Specified calendar expression is in the past, proceeding anyway.");
|
||||
else if (r < 0)
|
||||
if (r == -ENOENT) {
|
||||
/* The calendar event might be in the past, so let's warn about this, but
|
||||
* install it anyway as is. The service manager will trigger the service
|
||||
* right away. Moreover, the server side might have a different clock or
|
||||
* timezone than we do, hence it should decide when or whether to run
|
||||
* something.
|
||||
*
|
||||
* However, a mismatching weekday for a fixed date also results in -ENOENT,
|
||||
* and was already warned about when parsing. */
|
||||
if (!calendar_spec_weekday_conflicts(cs, NULL))
|
||||
log_warning("Specified calendar expression is in the past, proceeding anyway.");
|
||||
} else if (r < 0)
|
||||
return log_error_errno(r, "Failed to calculate next time calendar expression elapses: %m");
|
||||
|
||||
r = add_timer_property("OnCalendar", opts.arg);
|
||||
|
||||
+68
-17
@@ -15,7 +15,7 @@
|
||||
#include "strv.h"
|
||||
#include "time-util.h"
|
||||
|
||||
#define BITS_WEEKDAYS 127
|
||||
#define BITS_WEEKDAYS ((1 << _WEEKDAY_MAX) - 1)
|
||||
#define MIN_YEAR 1970
|
||||
#define MAX_YEAR 2199
|
||||
|
||||
@@ -237,16 +237,6 @@ _pure_ bool calendar_spec_valid(CalendarSpec *c) {
|
||||
}
|
||||
|
||||
static void format_weekdays(FILE *f, const CalendarSpec *c) {
|
||||
static const char *const days[] = {
|
||||
"Mon",
|
||||
"Tue",
|
||||
"Wed",
|
||||
"Thu",
|
||||
"Fri",
|
||||
"Sat",
|
||||
"Sun",
|
||||
};
|
||||
|
||||
int l, x;
|
||||
bool need_comma = false;
|
||||
|
||||
@@ -254,7 +244,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) {
|
||||
assert(c);
|
||||
assert(c->weekdays_bits > 0 && c->weekdays_bits <= BITS_WEEKDAYS);
|
||||
|
||||
for (x = 0, l = -1; x < (int) ELEMENTSOF(days); x++) {
|
||||
for (x = 0, l = -1; x < _WEEKDAY_MAX; x++) {
|
||||
|
||||
if (c->weekdays_bits & (1 << x)) {
|
||||
|
||||
@@ -264,7 +254,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) {
|
||||
else
|
||||
need_comma = true;
|
||||
|
||||
fputs(days[x], f);
|
||||
fputs(weekday_to_string(x), f);
|
||||
l = x;
|
||||
}
|
||||
|
||||
@@ -272,7 +262,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) {
|
||||
|
||||
if (x > l + 1) {
|
||||
fputs(x > l + 2 ? ".." : ",", f);
|
||||
fputs(days[x-1], f);
|
||||
fputs(weekday_to_string(x-1), f);
|
||||
}
|
||||
|
||||
l = -1;
|
||||
@@ -281,7 +271,7 @@ static void format_weekdays(FILE *f, const CalendarSpec *c) {
|
||||
|
||||
if (l >= 0 && x > l + 1) {
|
||||
fputs(x > l + 2 ? ".." : ",", f);
|
||||
fputs(days[x-1], f);
|
||||
fputs(weekday_to_string(x-1), f);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -877,7 +867,7 @@ finish:
|
||||
return 0;
|
||||
}
|
||||
|
||||
int calendar_spec_from_string(const char *p, CalendarSpec **ret) {
|
||||
int calendar_spec_from_string_full(const char *p, CalendarSpec **ret, bool warn_on_weekday_mismatch) {
|
||||
const char *utc;
|
||||
_cleanup_(calendar_spec_freep) CalendarSpec *c = NULL;
|
||||
_cleanup_free_ char *p_tmp = NULL;
|
||||
@@ -1098,6 +1088,15 @@ int calendar_spec_from_string(const char *p, CalendarSpec **ret) {
|
||||
if (!calendar_spec_valid(c))
|
||||
return -EINVAL;
|
||||
|
||||
if (warn_on_weekday_mismatch) {
|
||||
int wday;
|
||||
if (calendar_spec_weekday_conflicts(c, &wday))
|
||||
log_warning("Weekday constraint does not match the fixed date %04d-%02d-%02d "
|
||||
"(which is a %s), so this timer will never elapse.",
|
||||
c->year->start, c->month->start, c->day->start,
|
||||
weekday_to_string(wday));
|
||||
}
|
||||
|
||||
if (ret)
|
||||
*ret = TAKE_PTR(c);
|
||||
return 0;
|
||||
@@ -1228,6 +1227,7 @@ static int tm_within_bounds(struct tm *tm, bool utc) {
|
||||
|
||||
static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
|
||||
struct tm t;
|
||||
usec_t usec;
|
||||
int k;
|
||||
|
||||
assert(tm);
|
||||
@@ -1236,13 +1236,64 @@ static bool matches_weekday(int weekdays_bits, const struct tm *tm, bool utc) {
|
||||
return true;
|
||||
|
||||
t = *tm;
|
||||
if (mktime_or_timegm_usec(&t, utc, /* ret= */ NULL) < 0)
|
||||
if (mktime_or_timegm_usec(&t, utc, &usec) < 0)
|
||||
return false;
|
||||
if (localtime_or_gmtime_usec(usec, utc, &t) < 0)
|
||||
return false;
|
||||
|
||||
k = t.tm_wday == 0 ? 6 : t.tm_wday - 1;
|
||||
return (weekdays_bits & (1 << k));
|
||||
}
|
||||
|
||||
static bool component_is_single_fixed(const CalendarComponent *c) {
|
||||
/* Returns true if the component is a single fixed value: no range, no repeat, no alternatives. */
|
||||
return c && !c->next && c->repeat == 0 && (c->stop < 0 || c->stop == c->start);
|
||||
}
|
||||
|
||||
bool calendar_spec_weekday_conflicts(const CalendarSpec *spec, int *ret_actual_wday) {
|
||||
assert(spec);
|
||||
|
||||
if (spec->weekdays_bits <= 0 || spec->weekdays_bits >= BITS_WEEKDAYS)
|
||||
return false;
|
||||
|
||||
/* Skip when end_of_month (~) is used: day->start holds an offset, not an
|
||||
* absolute day-of-month, so the real firing date cannot be determined here. */
|
||||
if (spec->end_of_month)
|
||||
return false;
|
||||
|
||||
/* Only detectable when year, month and day are each a single fixed value */
|
||||
if (!component_is_single_fixed(spec->year) ||
|
||||
!component_is_single_fixed(spec->month) ||
|
||||
!component_is_single_fixed(spec->day))
|
||||
return false;
|
||||
|
||||
/* CalendarSpec stores month as 1-12; struct tm uses 0-11 */
|
||||
struct tm tm = {
|
||||
.tm_year = spec->year->start - 1900,
|
||||
.tm_mon = spec->month->start - 1,
|
||||
.tm_mday = spec->day->start,
|
||||
};
|
||||
struct tm orig = tm;
|
||||
|
||||
/* Compute weekday in UTC to avoid TZ/DST skew; reject dates that would be
|
||||
* silently normalised (e.g. 2027-02-31 → 2027-03-03).
|
||||
*
|
||||
* Do not rely on timegm() to fill tm_wday, since musl does not do that. */
|
||||
usec_t usec;
|
||||
if (mktime_or_timegm_usec(&tm, /* utc= */ true, &usec) < 0 ||
|
||||
localtime_or_gmtime_usec(usec, /* utc= */ true, &tm) < 0 ||
|
||||
tm.tm_year != orig.tm_year || tm.tm_mon != orig.tm_mon ||
|
||||
tm.tm_mday != orig.tm_mday)
|
||||
return false;
|
||||
|
||||
if (matches_weekday(spec->weekdays_bits, &tm, /* utc= */ true))
|
||||
return false; /* no conflict */
|
||||
|
||||
if (ret_actual_wday)
|
||||
*ret_actual_wday = tm.tm_wday == 0 ? 6 : tm.tm_wday - 1;
|
||||
return true;
|
||||
}
|
||||
|
||||
static int tm_compare(const struct tm *t1, const struct tm *t2) {
|
||||
int r;
|
||||
|
||||
|
||||
@@ -35,8 +35,13 @@ CalendarSpec* calendar_spec_free(CalendarSpec *c);
|
||||
bool calendar_spec_valid(CalendarSpec *spec);
|
||||
|
||||
int calendar_spec_to_string(const CalendarSpec *spec, char **ret);
|
||||
int calendar_spec_from_string(const char *p, CalendarSpec **ret);
|
||||
int calendar_spec_from_string_full(const char *p, CalendarSpec **ret, bool warn_on_weekday_mismatch);
|
||||
|
||||
static inline int calendar_spec_from_string(const char *p, CalendarSpec **ret) {
|
||||
return calendar_spec_from_string_full(p, ret, /* warn_on_weekday_mismatch= */ false);
|
||||
}
|
||||
|
||||
int calendar_spec_next_usec(const CalendarSpec *spec, usec_t usec, usec_t *next);
|
||||
bool calendar_spec_weekday_conflicts(const CalendarSpec *spec, int *ret_actual_wday);
|
||||
|
||||
DEFINE_TRIVIAL_CLEANUP_FUNC(CalendarSpec*, calendar_spec_free);
|
||||
|
||||
@@ -7,6 +7,11 @@
|
||||
#include "calendarspec.h"
|
||||
#include "env-util.h"
|
||||
#include "errno-util.h"
|
||||
#include "fd-util.h"
|
||||
#include "io-util.h"
|
||||
#include "log.h"
|
||||
#include "memfd-util.h"
|
||||
#include "process-util.h"
|
||||
#include "string-util.h"
|
||||
#include "tests.h"
|
||||
#include "time-util.h"
|
||||
@@ -256,6 +261,76 @@ TEST(calendar_spec_from_string) {
|
||||
assert_se(calendar_spec_from_string("*:4,30:*\n", &c) == -EINVAL);
|
||||
}
|
||||
|
||||
/* Fork a child with stderr redirected to a memfd, invoke calendar_spec_from_string_full()
|
||||
* with warn=true, wait, then return the memfd rewound to 0. */
|
||||
static int run_in_child_capture_stderr(const char *spec) {
|
||||
_cleanup_close_ int memfd = memfd_new("calendarspec-warn-test");
|
||||
assert_se(memfd >= 0);
|
||||
|
||||
int r = pidref_safe_fork_full(
|
||||
"(calendarspec-warn)",
|
||||
(const int[3]) { -1, -1, memfd },
|
||||
NULL, 0,
|
||||
FORK_WAIT|FORK_LOG|FORK_REARRANGE_STDIO,
|
||||
NULL);
|
||||
assert_se(r >= 0);
|
||||
|
||||
if (r == 0) {
|
||||
/* log.c may have inherited a console fd that no longer points at stderr. */
|
||||
log_close();
|
||||
log_set_target_and_open(LOG_TARGET_CONSOLE);
|
||||
log_set_max_level(LOG_WARNING);
|
||||
|
||||
_cleanup_(calendar_spec_freep) CalendarSpec *c = NULL;
|
||||
(void) calendar_spec_from_string_full(spec, &c, /* warn_on_weekday_mismatch= */ true);
|
||||
/* Write PARSE_OK to stderr as a positive anchor. */
|
||||
fputs("PARSE_OK\n", stderr);
|
||||
_exit(0);
|
||||
}
|
||||
|
||||
assert_se(lseek(memfd, 0, SEEK_SET) == 0);
|
||||
return TAKE_FD(memfd);
|
||||
}
|
||||
|
||||
TEST(calendar_spec_weekday_conflict) {
|
||||
char buf[4096];
|
||||
ssize_t n;
|
||||
|
||||
/* Case 1: conflicting weekday — warning must be emitted */
|
||||
{
|
||||
_cleanup_close_ int fd = run_in_child_capture_stderr("Thu 2027-01-01");
|
||||
n = loop_read(fd, buf, sizeof(buf) - 1, /* do_poll= */ false);
|
||||
assert_se(n > 0);
|
||||
buf[n] = '\0';
|
||||
/* Check date, weekday text and warning phrase are present. */
|
||||
assert_se(strstr(buf, "2027-01-01"));
|
||||
assert_se(strstr(buf, "which is a Fri"));
|
||||
assert_se(strstr(buf, "will never elapse"));
|
||||
}
|
||||
|
||||
/* Case 2: correct weekday — no warning */
|
||||
{
|
||||
_cleanup_close_ int fd = run_in_child_capture_stderr("Fri 2027-01-01");
|
||||
n = loop_read(fd, buf, sizeof(buf) - 1, /* do_poll= */ false);
|
||||
/* Ensure child wrote PARSE_OK. */
|
||||
assert_se(n >= 0);
|
||||
buf[MAX(n, (ssize_t)0)] = '\0';
|
||||
assert_se(strstr(buf, "PARSE_OK"));
|
||||
assert_se(!strstr(buf, "will never elapse"));
|
||||
}
|
||||
|
||||
/* Case 3: wildcard date — no static conflict, no warning */
|
||||
{
|
||||
_cleanup_close_ int fd = run_in_child_capture_stderr("Thu *-*-*");
|
||||
n = loop_read(fd, buf, sizeof(buf) - 1, /* do_poll= */ false);
|
||||
/* Ensure child wrote PARSE_OK. */
|
||||
assert_se(n >= 0);
|
||||
buf[MAX(n, (ssize_t)0)] = '\0';
|
||||
assert_se(strstr(buf, "PARSE_OK"));
|
||||
assert_se(!strstr(buf, "will never elapse"));
|
||||
}
|
||||
}
|
||||
|
||||
static int intro(void) {
|
||||
/* Tests have hard-coded results that do not expect a specific timezone to be set by the caller */
|
||||
ASSERT_OK_ERRNO(unsetenv("TZ"));
|
||||
|
||||
Reference in New Issue
Block a user