From 2ccf8d4922c77dd417345e101f7d021002be49f0 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 24 Mar 2020 21:13:14 +0100 Subject: [PATCH] unix: fix utime/futime timestamp rounding errors `uv_fs_utime()` and `uv_fs_futime()` receive the timestamp as a `double` and then convert it to `struct timeval` or `struct timespec` where necessary but the calculation for the sub-second part exhibited rounding errors for dates in the deep past or the far-flung future, causing the timestamps to be off by sometimes over half a second. Fixes: https://github.com/nodejs/node/issues/32369 (partially) --- src/unix/fs.c | 73 +++++++++++++++++++++++++++++++++--------------- test/test-fs.c | 31 ++++++++++++++++++++ test/test-list.h | 2 ++ 3 files changed, 84 insertions(+), 22 deletions(-) diff --git a/src/unix/fs.c b/src/unix/fs.c index 4a47237564f..fc167c5bca5 100644 --- a/src/unix/fs.c +++ b/src/unix/fs.c @@ -157,6 +157,49 @@ extern char *mkdtemp(char *template); /* See issue #740 on AIX < 7 */ while (0) +/* Returns the positive fractional part of a floating-point number. + * Garbage in, garbage out: don't pass in NaN or Infinity. + * + * Open-coded (sort of, it's not exactly equivalent to modf(3)) + * to avoid a dependency on libm. + */ +static double uv__frac(double x) { + if (x < 0) + x = -x; + + x -= (uint64_t) x; + + return x; +} + + +UV_UNUSED(static struct timespec uv__to_timespec(double x)) { + struct timespec t; + + t.tv_sec = x; + t.tv_nsec = 1e9 * uv__frac(x); + + /* TODO(bnoordhuis) Remove this. utimesat() has nanosecond resolution but we + * stick to microsecond resolution for the sake of consistency with other + * platforms. I'm the original author of this compatibility hack but I'm + * less convinced it's useful nowadays. + */ + t.tv_nsec -= t.tv_nsec % 1000; + + return t; +} + + +UV_UNUSED(static struct timeval uv__to_timeval(double x)) { + struct timeval t; + + t.tv_sec = x; + t.tv_usec = 1e6 * uv__frac(x); + + return t; +} + + static int uv__fs_close(int fd) { int rc; @@ -209,14 +252,9 @@ static ssize_t uv__fs_futime(uv_fs_t* req) { #if defined(__linux__) \ || defined(_AIX71) \ || defined(__HAIKU__) - /* utimesat() has nanosecond resolution but we stick to microseconds - * for the sake of consistency with other platforms. - */ struct timespec ts[2]; - ts[0].tv_sec = req->atime; - ts[0].tv_nsec = (uint64_t)(req->atime * 1000000) % 1000000 * 1000; - ts[1].tv_sec = req->mtime; - ts[1].tv_nsec = (uint64_t)(req->mtime * 1000000) % 1000000 * 1000; + ts[0] = uv__to_timespec(req->atime); + ts[1] = uv__to_timespec(req->mtime); #if defined(__ANDROID_API__) && __ANDROID_API__ < 21 return utimensat(req->file, NULL, ts, 0); #else @@ -230,10 +268,8 @@ static ssize_t uv__fs_futime(uv_fs_t* req) { || defined(__OpenBSD__) \ || defined(__sun) struct timeval tv[2]; - tv[0].tv_sec = req->atime; - tv[0].tv_usec = (uint64_t)(req->atime * 1000000) % 1000000; - tv[1].tv_sec = req->mtime; - tv[1].tv_usec = (uint64_t)(req->mtime * 1000000) % 1000000; + tv[0] = uv__to_timeval(req->atime); + tv[1] = uv__to_timeval(req->mtime); # if defined(__sun) return futimesat(req->file, NULL, tv); # else @@ -968,14 +1004,9 @@ static ssize_t uv__fs_utime(uv_fs_t* req) { || defined(_AIX71) \ || defined(__sun) \ || defined(__HAIKU__) - /* utimesat() has nanosecond resolution but we stick to microseconds - * for the sake of consistency with other platforms. - */ struct timespec ts[2]; - ts[0].tv_sec = req->atime; - ts[0].tv_nsec = (uint64_t)(req->atime * 1000000) % 1000000 * 1000; - ts[1].tv_sec = req->mtime; - ts[1].tv_nsec = (uint64_t)(req->mtime * 1000000) % 1000000 * 1000; + ts[0] = uv__to_timespec(req->atime); + ts[1] = uv__to_timespec(req->mtime); return utimensat(AT_FDCWD, req->path, ts, 0); #elif defined(__APPLE__) \ || defined(__DragonFly__) \ @@ -984,10 +1015,8 @@ static ssize_t uv__fs_utime(uv_fs_t* req) { || defined(__NetBSD__) \ || defined(__OpenBSD__) struct timeval tv[2]; - tv[0].tv_sec = req->atime; - tv[0].tv_usec = (uint64_t)(req->atime * 1000000) % 1000000; - tv[1].tv_sec = req->mtime; - tv[1].tv_usec = (uint64_t)(req->mtime * 1000000) % 1000000; + tv[0] = uv__to_timeval(req->atime); + tv[1] = uv__to_timeval(req->mtime); return utimes(req->path, tv); #elif defined(_AIX) \ && !defined(_AIX71) diff --git a/test/test-fs.c b/test/test-fs.c index 56413cdae57..827ba25ddd8 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -2488,6 +2488,37 @@ TEST_IMPL(fs_utime) { } +TEST_IMPL(fs_utime_round) { + const char path[] = "test_file"; + double atime; + double mtime; + uv_fs_t req; + int r; + +#ifdef __MVS__ + RETURN_SKIP("futime on z/os does not like pre-epoch timestamps"); +#endif + + loop = uv_default_loop(); + unlink(path); + r = uv_fs_open(NULL, &req, path, O_RDWR | O_CREAT, S_IWUSR | S_IRUSR, NULL); + ASSERT(r >= 0); + ASSERT(req.result >= 0); + uv_fs_req_cleanup(&req); + ASSERT(0 == uv_fs_close(loop, &req, r, NULL)); + + atime = mtime = -14245440.0; /* 1969-07-20T02:56:00.00Z */ + ASSERT(0 == uv_fs_utime(NULL, &req, path, atime, mtime, NULL)); + ASSERT(req.result == 0); + uv_fs_req_cleanup(&req); + check_utime(path, atime, mtime); + unlink(path); + + MAKE_VALGRIND_HAPPY(); + return 0; +} + + #ifdef _WIN32 TEST_IMPL(fs_stat_root) { int r; diff --git a/test/test-list.h b/test/test-list.h index 003b8d26517..432c4e80f0c 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -352,6 +352,7 @@ TEST_DECLARE (fs_open_flags) TEST_DECLARE (fs_fd_hash) #endif TEST_DECLARE (fs_utime) +TEST_DECLARE (fs_utime_round) TEST_DECLARE (fs_futime) TEST_DECLARE (fs_file_open_append) TEST_DECLARE (fs_statfs) @@ -968,6 +969,7 @@ TASK_LIST_START #endif TEST_ENTRY (fs_chown) TEST_ENTRY (fs_utime) + TEST_ENTRY (fs_utime_round) TEST_ENTRY (fs_futime) TEST_ENTRY (fs_readlink) TEST_ENTRY (fs_realpath)