From 083b4a582a8c32ad31883f4f7110a9d42351b564 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 21 Mar 2020 14:07:21 +0100 Subject: [PATCH 1/3] drop! libuv fs fixes --- deps/uv/src/unix/fs.c | 73 ++++++++++++++++++++++++++++------------ deps/uv/test/test-fs.c | 27 +++++++++++++++ deps/uv/test/test-list.h | 2 ++ 3 files changed, 80 insertions(+), 22 deletions(-) diff --git a/deps/uv/src/unix/fs.c b/deps/uv/src/unix/fs.c index d5017ebbc8c65d..87e5c9c35e7c2e 100644 --- a/deps/uv/src/unix/fs.c +++ b/deps/uv/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 @@ -973,14 +1009,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__) \ @@ -989,10 +1020,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/deps/uv/test/test-fs.c b/deps/uv/test/test-fs.c index b6b2b19981bdd4..be8c78b63eb588 100644 --- a/deps/uv/test/test-fs.c +++ b/deps/uv/test/test-fs.c @@ -2493,6 +2493,33 @@ TEST_IMPL(fs_utime) { } +TEST_IMPL(fs_utime_round) { + const char path[] = "test_file"; + double atime; + double mtime; + uv_fs_t req; + int r; + + 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/deps/uv/test/test-list.h b/deps/uv/test/test-list.h index 003b8d26517570..432c4e80f0cd6c 100644 --- a/deps/uv/test/test-list.h +++ b/deps/uv/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) From a05e273fdeeda72054f144c4a23d388a2d03c00b Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 21 Mar 2020 14:07:21 +0100 Subject: [PATCH 2/3] lib: remove file timestamp rounding hack Introduced in commit 9836cf5717 ("lib: lazy instantiation of fs.Stats dates") without providing any rationale - that wasn't added until later, by someone else - and it doesn't look the least bit correct to me. It certainly makes an upcoming regression test fail because a timestamp in the deep past doesn't round-trip correctly, it's off by a fraction of a second. Refs: https://github.com/nodejs/node/issues/32369 --- lib/internal/fs/utils.js | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 763a940f6e98a7..b7e7c5e398c778 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -362,16 +362,6 @@ function nsFromTimeSpecBigInt(sec, nsec) { return sec * kNsPerSecBigInt + nsec; } -// The Date constructor performs Math.floor() to the timestamp. -// https://www.ecma-international.org/ecma-262/#sec-timeclip -// Since there may be a precision loss when the timestamp is -// converted to a floating point number, we manually round -// the timestamp here before passing it to Date(). -// Refs: https://github.com/nodejs/node/pull/12607 -function dateFromMs(ms) { - return new Date(Number(ms) + 0.5); -} - function BigIntStats(dev, mode, nlink, uid, gid, rdev, blksize, ino, size, blocks, atimeNs, mtimeNs, ctimeNs, birthtimeNs) { @@ -386,10 +376,10 @@ function BigIntStats(dev, mode, nlink, uid, gid, rdev, blksize, this.mtimeNs = mtimeNs; this.ctimeNs = ctimeNs; this.birthtimeNs = birthtimeNs; - this.atime = dateFromMs(this.atimeMs); - this.mtime = dateFromMs(this.mtimeMs); - this.ctime = dateFromMs(this.ctimeMs); - this.birthtime = dateFromMs(this.birthtimeMs); + this.atime = new Date(Number(this.atimeMs)); + this.mtime = new Date(Number(this.mtimeMs)); + this.ctime = new Date(Number(this.ctimeMs)); + this.birthtime = new Date(Number(this.birthtimeMs)); } ObjectSetPrototypeOf(BigIntStats.prototype, StatsBase.prototype); @@ -412,10 +402,10 @@ function Stats(dev, mode, nlink, uid, gid, rdev, blksize, this.mtimeMs = mtimeMs; this.ctimeMs = ctimeMs; this.birthtimeMs = birthtimeMs; - this.atime = dateFromMs(atimeMs); - this.mtime = dateFromMs(mtimeMs); - this.ctime = dateFromMs(ctimeMs); - this.birthtime = dateFromMs(birthtimeMs); + this.atime = new Date(atimeMs); + this.mtime = new Date(mtimeMs); + this.ctime = new Date(ctimeMs); + this.birthtime = new Date(birthtimeMs); } ObjectSetPrototypeOf(Stats.prototype, StatsBase.prototype); From 2a96f50cc9b2915f25026392cb9f5d2391887b51 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 21 Mar 2020 14:07:21 +0100 Subject: [PATCH 3/3] src: make fs functions handle pre-epoch timestamps A wrong type cast prevented timestamps before 1970-01-01 from working with functions like `fs.stat()`. Fixes: https://github.com/nodejs/node/issues/32369 --- src/node_file-inl.h | 3 +-- .../test-fs-stat-before-unix-epoch.js | 20 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 test/parallel/test-fs-stat-before-unix-epoch.js diff --git a/src/node_file-inl.h b/src/node_file-inl.h index e9ed18a75fa48b..8a61fee1257083 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -85,8 +85,7 @@ void FillStatsArray(AliasedBufferBase* fields, static_cast(stat)) #define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \ - /* NOLINTNEXTLINE(runtime/int) */ \ - SET_FIELD_WITH_STAT(stat_offset, static_cast(stat)) + SET_FIELD_WITH_STAT(stat_offset, static_cast(stat)) SET_FIELD_WITH_STAT(kDev, s->st_dev); SET_FIELD_WITH_STAT(kMode, s->st_mode); diff --git a/test/parallel/test-fs-stat-before-unix-epoch.js b/test/parallel/test-fs-stat-before-unix-epoch.js new file mode 100644 index 00000000000000..0db4cb48c6a7f5 --- /dev/null +++ b/test/parallel/test-fs-stat-before-unix-epoch.js @@ -0,0 +1,20 @@ +// Check that file timestamps from before the UNIX epoch +// are set and retrieved correctly. + +'use strict'; + +require('../common'); +const tmpdir = require('../common/tmpdir'); +const assert = require('assert'); +const fs = require('fs'); +const path = require('path'); + +const date = new Date('1969-07-20 02:56:00Z'); +const filename = path.join(tmpdir.path, '42.txt'); + +tmpdir.refresh(); +fs.writeFileSync(filename, '42'); +fs.utimesSync(filename, date, date); +const s = fs.statSync(filename); +assert.deepStrictEqual(s.atime, date); +assert.deepStrictEqual(s.mtime, date);