Skip to content

Commit

Permalink
fs: fix utime/futime timestamp rounding errors
Browse files Browse the repository at this point in the history 
`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 on
unix, or to be reinterpreted as unsigned and end up off by more than
just sign but many also decades.

Fixes: nodejs/node#32369 (partially)
PR-URL: libuv#2747
Co-authored-by: Jameson Nash <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
  • Loading branch information
@bnoordhuis @vtjnash @JeffroMF
2 people authored and JeffroMF committed May 16, 2022
1 parent af3f315 commit 7c6844f
Show file tree
Hide file tree
Showing 5 changed files with 140 additions and 78 deletions.
26 changes: 18 additions & 8 deletions src/unix/fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,24 +212,37 @@ static ssize_t uv__fs_fdatasync(uv_fs_t* req) {
UV_UNUSED(static struct timespec uv__fs_to_timespec(double time)) {
struct timespec ts;
ts.tv_sec = time;
ts.tv_nsec = (uint64_t)(time * 1000000) % 1000000 * 1000;
ts.tv_nsec = (time - ts.tv_sec) * 1e9;

/* 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.
*/
ts.tv_nsec -= ts.tv_nsec % 1000;

if (ts.tv_nsec < 0) {
ts.tv_nsec += 1e9;
ts.tv_sec -= 1;
}
return ts;
}

UV_UNUSED(static struct timeval uv__fs_to_timeval(double time)) {
struct timeval tv;
tv.tv_sec = time;
tv.tv_usec = (uint64_t)(time * 1000000) % 1000000;
tv.tv_usec = (time - tv.tv_sec) * 1e6;
if (tv.tv_usec < 0) {
tv.tv_usec += 1e6;
tv.tv_sec -= 1;
}
return tv;
}

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] = uv__fs_to_timespec(req->atime);
ts[1] = uv__fs_to_timespec(req->mtime);
Expand Down Expand Up @@ -1010,9 +1023,6 @@ 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] = uv__fs_to_timespec(req->atime);
ts[1] = uv__fs_to_timespec(req->mtime);
Expand Down
46 changes: 22 additions & 24 deletions src/win/fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,30 +92,24 @@
return; \
}

#define MILLIONu (1000U * 1000U)
#define BILLIONu (1000U * 1000U * 1000U)
#define MILLION ((int64_t) 1000 * 1000)
#define BILLION ((int64_t) 1000 * 1000 * 1000)

#define FILETIME_TO_UINT(filetime) \
(*((uint64_t*) &(filetime)) - (uint64_t) 116444736 * BILLIONu)

#define FILETIME_TO_TIME_T(filetime) \
(FILETIME_TO_UINT(filetime) / (10u * MILLIONu))

#define FILETIME_TO_TIME_NS(filetime, secs) \
((FILETIME_TO_UINT(filetime) - (secs * (uint64_t) 10 * MILLIONu)) * 100U)

#define FILETIME_TO_TIMESPEC(ts, filetime) \
do { \
(ts).tv_sec = (long) FILETIME_TO_TIME_T(filetime); \
(ts).tv_nsec = (long) FILETIME_TO_TIME_NS(filetime, (ts).tv_sec); \
} while(0)
static void uv__filetime_to_timespec(uv_timespec_t *ts, int64_t filetime) {
filetime -= 116444736 * BILLION;
ts->tv_sec = (long) (filetime / (10 * MILLION));
ts->tv_nsec = (long) ((filetime - ts->tv_sec * 10 * MILLION) * 100U);
if (ts->tv_nsec < 0) {
ts->tv_sec -= 1;
ts->tv_nsec += 1e9;
}
}

#define TIME_T_TO_FILETIME(time, filetime_ptr) \
do { \
uint64_t bigtime = ((uint64_t) ((time) * (uint64_t) 10 * MILLIONu)) + \
(uint64_t) 116444736 * BILLIONu; \
(filetime_ptr)->dwLowDateTime = bigtime & 0xFFFFFFFF; \
(filetime_ptr)->dwHighDateTime = bigtime >> 32; \
int64_t bigtime = ((time) * 10 * MILLION + 116444736 * BILLION); \
(filetime_ptr)->dwLowDateTime = (uint64_t) bigtime & 0xFFFFFFFF; \
(filetime_ptr)->dwHighDateTime = (uint64_t) bigtime >> 32; \
} while(0)

#define IS_SLASH(c) ((c) == L'\\' || (c) == L'/')
Expand Down Expand Up @@ -1791,10 +1785,14 @@ INLINE static int fs__stat_handle(HANDLE handle, uv_stat_t* statbuf,
statbuf->st_mode |= (_S_IREAD | _S_IWRITE) | ((_S_IREAD | _S_IWRITE) >> 3) |
((_S_IREAD | _S_IWRITE) >> 6);

FILETIME_TO_TIMESPEC(statbuf->st_atim, file_info.BasicInformation.LastAccessTime);
FILETIME_TO_TIMESPEC(statbuf->st_ctim, file_info.BasicInformation.ChangeTime);
FILETIME_TO_TIMESPEC(statbuf->st_mtim, file_info.BasicInformation.LastWriteTime);
FILETIME_TO_TIMESPEC(statbuf->st_birthtim, file_info.BasicInformation.CreationTime);
uv__filetime_to_timespec(&statbuf->st_atim,
file_info.BasicInformation.LastAccessTime.QuadPart);
uv__filetime_to_timespec(&statbuf->st_ctim,
file_info.BasicInformation.ChangeTime.QuadPart);
uv__filetime_to_timespec(&statbuf->st_mtim,
file_info.BasicInformation.LastWriteTime.QuadPart);
uv__filetime_to_timespec(&statbuf->st_birthtim,
file_info.BasicInformation.CreationTime.QuadPart);

statbuf->st_ino = file_info.InternalInformation.IndexNumber.QuadPart;

Expand Down
36 changes: 20 additions & 16 deletions test/task.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,22 +196,26 @@ typedef enum {
} \
} while (0)

#define ASSERT_INT_BASE(a, operator, b, type, conv) \
ASSERT_BASE(a, operator, b, type, conv)

#define ASSERT_EQ(a, b) ASSERT_INT_BASE(a, ==, b, int64_t, PRId64)
#define ASSERT_GE(a, b) ASSERT_INT_BASE(a, >=, b, int64_t, PRId64)
#define ASSERT_GT(a, b) ASSERT_INT_BASE(a, >, b, int64_t, PRId64)
#define ASSERT_LE(a, b) ASSERT_INT_BASE(a, <=, b, int64_t, PRId64)
#define ASSERT_LT(a, b) ASSERT_INT_BASE(a, <, b, int64_t, PRId64)
#define ASSERT_NE(a, b) ASSERT_INT_BASE(a, !=, b, int64_t, PRId64)

#define ASSERT_UINT64_EQ(a, b) ASSERT_INT_BASE(a, ==, b, uint64_t, PRIu64)
#define ASSERT_UINT64_GE(a, b) ASSERT_INT_BASE(a, >=, b, uint64_t, PRIu64)
#define ASSERT_UINT64_GT(a, b) ASSERT_INT_BASE(a, >, b, uint64_t, PRIu64)
#define ASSERT_UINT64_LE(a, b) ASSERT_INT_BASE(a, <=, b, uint64_t, PRIu64)
#define ASSERT_UINT64_LT(a, b) ASSERT_INT_BASE(a, <, b, uint64_t, PRIu64)
#define ASSERT_UINT64_NE(a, b) ASSERT_INT_BASE(a, !=, b, uint64_t, PRIu64)
#define ASSERT_EQ(a, b) ASSERT_BASE(a, ==, b, int64_t, PRId64)
#define ASSERT_GE(a, b) ASSERT_BASE(a, >=, b, int64_t, PRId64)
#define ASSERT_GT(a, b) ASSERT_BASE(a, >, b, int64_t, PRId64)
#define ASSERT_LE(a, b) ASSERT_BASE(a, <=, b, int64_t, PRId64)
#define ASSERT_LT(a, b) ASSERT_BASE(a, <, b, int64_t, PRId64)
#define ASSERT_NE(a, b) ASSERT_BASE(a, !=, b, int64_t, PRId64)

#define ASSERT_UINT64_EQ(a, b) ASSERT_BASE(a, ==, b, uint64_t, PRIu64)
#define ASSERT_UINT64_GE(a, b) ASSERT_BASE(a, >=, b, uint64_t, PRIu64)
#define ASSERT_UINT64_GT(a, b) ASSERT_BASE(a, >, b, uint64_t, PRIu64)
#define ASSERT_UINT64_LE(a, b) ASSERT_BASE(a, <=, b, uint64_t, PRIu64)
#define ASSERT_UINT64_LT(a, b) ASSERT_BASE(a, <, b, uint64_t, PRIu64)
#define ASSERT_UINT64_NE(a, b) ASSERT_BASE(a, !=, b, uint64_t, PRIu64)

#define ASSERT_DOUBLE_EQ(a, b) ASSERT_BASE(a, ==, b, double, "f")
#define ASSERT_DOUBLE_GE(a, b) ASSERT_BASE(a, >=, b, double, "f")
#define ASSERT_DOUBLE_GT(a, b) ASSERT_BASE(a, >, b, double, "f")
#define ASSERT_DOUBLE_LE(a, b) ASSERT_BASE(a, <=, b, double, "f")
#define ASSERT_DOUBLE_LT(a, b) ASSERT_BASE(a, <, b, double, "f")
#define ASSERT_DOUBLE_NE(a, b) ASSERT_BASE(a, !=, b, double, "f")

#define ASSERT_STR_EQ(a, b) \
ASSERT_BASE_STR(strcmp(a, b) == 0, a, == , b, char*, "s")
Expand Down
108 changes: 78 additions & 30 deletions test/test-fs.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -816,13 +816,44 @@ static void check_utime(const char* path,
else
r = uv_fs_stat(loop, &req, path, NULL);

ASSERT(r == 0);
ASSERT_EQ(r, 0);

ASSERT(req.result == 0);
ASSERT_EQ(req.result, 0);
s = &req.statbuf;

ASSERT(s->st_atim.tv_sec + (s->st_atim.tv_nsec / 1000000000.0) == atime);
ASSERT(s->st_mtim.tv_sec + (s->st_mtim.tv_nsec / 1000000000.0) == mtime);
if (s->st_atim.tv_nsec == 0 && s->st_mtim.tv_nsec == 0) {
/*
* Test sub-second timestamps only when supported (such as Windows with
* NTFS). Some other platforms support sub-second timestamps, but that
* support is filesystem-dependent. Notably OS X (HFS Plus) does NOT
* support sub-second timestamps. But kernels may round or truncate in
* either direction, so we may accept either possible answer.
*/
#ifdef _WIN32
ASSERT_DOUBLE_EQ(atime, (long) atime);
ASSERT_DOUBLE_EQ(mtime, (long) atime);
#endif
if (atime > 0 || (long) atime == atime)
ASSERT_EQ(s->st_atim.tv_sec, (long) atime);
if (mtime > 0 || (long) mtime == mtime)
ASSERT_EQ(s->st_mtim.tv_sec, (long) mtime);
ASSERT_GE(s->st_atim.tv_sec, (long) atime - 1);
ASSERT_GE(s->st_mtim.tv_sec, (long) mtime - 1);
ASSERT_LE(s->st_atim.tv_sec, (long) atime);
ASSERT_LE(s->st_mtim.tv_sec, (long) mtime);
} else {
double st_atim;
double st_mtim;
#ifndef __APPLE__
/* TODO(vtjnash): would it be better to normalize this? */
ASSERT_DOUBLE_GE(s->st_atim.tv_nsec, 0);
ASSERT_DOUBLE_GE(s->st_mtim.tv_nsec, 0);
#endif
st_atim = s->st_atim.tv_sec + s->st_atim.tv_nsec / 1e9;
st_mtim = s->st_mtim.tv_sec + s->st_mtim.tv_nsec / 1e9;
ASSERT_DOUBLE_EQ(st_atim, atime);
ASSERT_DOUBLE_EQ(st_mtim, mtime);
}

uv_fs_req_cleanup(&req);
}
Expand Down Expand Up @@ -2523,16 +2554,7 @@ TEST_IMPL(fs_utime) {
uv_fs_req_cleanup(&req);
uv_fs_close(loop, &req, r, NULL);

atime = mtime = 400497753; /* 1982-09-10 11:22:33 */

/*
* Test sub-second timestamps only on Windows (assuming NTFS). Some other
* platforms support sub-second timestamps, but that support is filesystem-
* dependent. Notably OS X (HFS Plus) does NOT support sub-second timestamps.
*/
#ifdef _WIN32
mtime += 0.444; /* 1982-09-10 11:22:33.444 */
#endif
atime = mtime = 400497753.25; /* 1982-09-10 11:22:33.25 */

r = uv_fs_utime(NULL, &req, path, atime, mtime, NULL);
ASSERT(r == 0);
Expand All @@ -2541,7 +2563,7 @@ TEST_IMPL(fs_utime) {

check_utime(path, atime, mtime, /* test_lutime */ 0);

atime = mtime = 1291404900; /* 2010-12-03 20:35:00 - mees <3 */
atime = mtime = 1291404900.25; /* 2010-12-03 20:35:00.25 - mees <3 */
checkme.path = path;
checkme.atime = atime;
checkme.mtime = mtime;
Expand All @@ -2561,6 +2583,45 @@ 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_GE(r, 0);
ASSERT_GE(req.result, 0);
uv_fs_req_cleanup(&req);
ASSERT_EQ(0, uv_fs_close(loop, &req, r, NULL));

atime = mtime = -14245440.25; /* 1969-07-20T02:56:00.25Z */

r = uv_fs_utime(NULL, &req, path, atime, mtime, NULL);
#if !defined(__linux__) && \
!defined(_WIN32) && \
!defined(__APPLE__) && \
!defined(__FreeBSD__) && \
!defined(__sun)
if (r != 0) {
ASSERT_EQ(r, UV_EINVAL);
RETURN_SKIP("utime on some OS (z/OS, IBM i PASE, AIX) or filesystems may reject pre-epoch timestamps");
}
#endif
ASSERT_EQ(0, r);
ASSERT_EQ(0, req.result);
uv_fs_req_cleanup(&req);
check_utime(path, atime, mtime, /* test_lutime */ 0);
unlink(path);

MAKE_VALGRIND_HAPPY();
return 0;
}


#ifdef _WIN32
TEST_IMPL(fs_stat_root) {
int r;
Expand Down Expand Up @@ -2614,16 +2675,7 @@ TEST_IMPL(fs_futime) {
uv_fs_req_cleanup(&req);
uv_fs_close(loop, &req, r, NULL);

atime = mtime = 400497753; /* 1982-09-10 11:22:33 */

/*
* Test sub-second timestamps only on Windows (assuming NTFS). Some other
* platforms support sub-second timestamps, but that support is filesystem-
* dependent. Notably OS X (HFS Plus) does NOT support sub-second timestamps.
*/
#ifdef _WIN32
mtime += 0.444; /* 1982-09-10 11:22:33.444 */
#endif
atime = mtime = 400497753.25; /* 1982-09-10 11:22:33.25 */

r = uv_fs_open(NULL, &req, path, O_RDWR, 0, NULL);
ASSERT(r >= 0);
Expand Down Expand Up @@ -2700,11 +2752,7 @@ TEST_IMPL(fs_lutime) {
uv_fs_req_cleanup(&req);

/* Test the synchronous version. */
atime = mtime = 400497753; /* 1982-09-10 11:22:33 */

#ifdef _WIN32
mtime += 0.444; /* 1982-09-10 11:22:33.444 */
#endif
atime = mtime = 400497753.25; /* 1982-09-10 11:22:33.25 */

checkme.atime = atime;
checkme.mtime = mtime;
Expand Down
2 changes: 2 additions & 0 deletions test/test-list.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,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_lutime)
TEST_DECLARE (fs_file_open_append)
Expand Down Expand Up @@ -996,6 +997,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_lutime)
TEST_ENTRY (fs_readlink)
Expand Down

0 comments on commit 7c6844f

Please sign in to comment.