-
Notifications
You must be signed in to change notification settings - Fork 29.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fs.Stat fails on pre-epoch mtime (<1970-01-01T00:00:00Z) #32369
Comments
FWIW, the fix is trivial (see below) but I'm still working on a reliable, portable regression test. Not all file systems and operating systems let you set the date that far back. diff --git a/src/node_file-inl.h b/src/node_file-inl.h
index e9ed18a75f..346a557f86 100644
--- a/src/node_file-inl.h
+++ b/src/node_file-inl.h
@@ -86,7 +86,7 @@ void FillStatsArray(AliasedBufferBase<NativeT, V8T>* fields,
#define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \
/* NOLINTNEXTLINE(runtime/int) */ \
- SET_FIELD_WITH_STAT(stat_offset, static_cast<unsigned long>(stat))
+ SET_FIELD_WITH_STAT(stat_offset, static_cast<double>(stat))
SET_FIELD_WITH_STAT(kDev, s->st_dev);
SET_FIELD_WITH_STAT(kMode, s->st_mode); |
There's also a secondary libuv bug: conversion from E.g. |
`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: nodejs/node#32369 (partially)
Introduced in commit 9836cf5 ("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: nodejs#32369
A wrong type cast prevented timestamps before 1970-01-01 from working with functions like `fs.stat()`. Fixes: nodejs#32369
Very happy you caught the rounding error too. Great work. |
`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: nodejs/node#32369 (partially)
`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: nodejs/node#32369 (partially)
`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: nodejs/node#32369 (partially)
`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: nodejs/node#32369 (partially) PR-URL: libuv#2747
`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 <vtjnash@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`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 <vtjnash@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`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 <vtjnash@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`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 <vtjnash@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`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 <vtjnash@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
`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 <vtjnash@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
What steps will reproduce the bug?
This is on Darwin (macOS)
This is on Linux (in Docker
How often does it reproduce? Is there a required condition?
Every time that mtime < unix epoch (1970-01-01T00:00:00Z)
What is the expected behavior?
Return a valid Date Object, for example
What do you see instead?
Additional information
I discovered this behavior by trying to use
fs.utimes
which is also not able to correctly handle dates before unix epoch, althoughfs.utimes
seems to have a workaround by using the string representation of unix time.The text was updated successfully, but these errors were encountered: