basic/time-util: make parsing of dual_timestamp more strict

*scanf functions set errno on i/o error. For sscanf, this doesn't really apply,
so (based on the man page), it seems that errno is unlikely to be ever set to a
useful value. So just ignore errno. The error message includes the string that
was parsed, so it should be always pretty clear why parsing failed.

On the other hand, detect trailing characters and minus prefix that weren't
converted properly. This matches what our safe_ato* functions do. Add tests to
elucidate various edge cases.

diff --git a/src/basic/time-util.c b/src/basic/time-util.c
index a0db97c41a73fdaaeff815df9d49debf0278b11a..8d55af849225cec49fc3c940832348cca8fb94ef 100644 (file)
--- a/src/basic/time-util.c
+++ b/src/basic/time-util.c
@@ -555,15 +555,29 @@ void dual_timestamp_serialize(FILE *f, const char *name, dual_timestamp *t) {
 
 int dual_timestamp_deserialize(const char *value, dual_timestamp *t) {
         uint64_t a, b;
+        int r, pos;
 
         assert(value);
         assert(t);
 
-        if (sscanf(value, "%" PRIu64 "%" PRIu64, &a, &b) != 2) {
-                log_debug("Failed to parse dual timestamp value \"%s\": %m", value);
+        pos = strspn(value, WHITESPACE);
+        if (value[pos] == '-')
+                return -EINVAL;
+        pos += strspn(value + pos, DIGITS);
+        pos += strspn(value + pos, WHITESPACE);
+        if (value[pos] == '-')
+                return -EINVAL;
+
+        r = sscanf(value, "%" PRIu64 "%" PRIu64 "%n", &a, &b, &pos);
+        if (r != 2) {
+                log_debug("Failed to parse dual timestamp value \"%s\".", value);
                 return -EINVAL;
         }
 
+        if (value[pos] != '\0')
+                /* trailing garbage */
+                return -EINVAL;
+
         t->realtime = a;
         t->monotonic = b;
 

diff --git a/src/test/test-time.c b/src/test/test-time.c
index 911282bf0c096759263e0c7d880bda25dcde73c6..c9e31e90e123b6450d91273f2def5b2ad6818c56 100644 (file)
--- a/src/test/test-time.c
+++ b/src/test/test-time.c
@@ -273,6 +273,43 @@ static void test_format_timestamp_utc(void) {
         test_format_timestamp_utc_one(USEC_INFINITY, NULL);
 }
 
+static void test_dual_timestamp_deserialize(void) {
+        int r;
+        dual_timestamp t;
+
+        r = dual_timestamp_deserialize("1234 5678", &t);
+        assert_se(r == 0);
+        assert_se(t.realtime == 1234);
+        assert_se(t.monotonic == 5678);
+
+        r = dual_timestamp_deserialize("1234x 5678", &t);
+        assert_se(r == -EINVAL);
+
+        r = dual_timestamp_deserialize("1234 5678y", &t);
+        assert_se(r == -EINVAL);
+
+        r = dual_timestamp_deserialize("-1234 5678", &t);
+        assert_se(r == -EINVAL);
+
+        r = dual_timestamp_deserialize("1234 -5678", &t);
+        assert_se(r == -EINVAL);
+
+        /* Check that output wasn't modified. */
+        assert_se(t.realtime == 1234);
+        assert_se(t.monotonic == 5678);
+
+        r = dual_timestamp_deserialize("+123 567", &t);
+        assert_se(r == 0);
+        assert_se(t.realtime == 123);
+        assert_se(t.monotonic == 567);
+
+        /* Check that we get "infinity" on overflow. */
+        r = dual_timestamp_deserialize("18446744073709551617 0", &t);
+        assert_se(r == 0);
+        assert_se(t.realtime == USEC_INFINITY);
+        assert_se(t.monotonic == 0);
+}
+
 int main(int argc, char *argv[]) {
         uintmax_t x;
 
@@ -288,6 +325,7 @@ int main(int argc, char *argv[]) {
         test_usec_sub();
         test_format_timestamp();
         test_format_timestamp_utc();
+        test_dual_timestamp_deserialize();
 
         /* Ensure time_t is signed */
         assert_cc((time_t) -1 < (time_t) 1);