Skip to content
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

utimes precision errors #13255

Closed
jorangreef opened this issue May 27, 2017 · 6 comments · Fixed by #13281
Closed

utimes precision errors #13255

jorangreef opened this issue May 27, 2017 · 6 comments · Fixed by #13281
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.

Comments

@jorangreef
Copy link
Contributor

jorangreef commented May 27, 2017

Version: v7.10.0
Platform: Windows 10 64-bit
Subsystem: fs

I was working on a fuzz test to test the accuracy of timestamps set using fs.utimes() and found the following two precision bugs on Windows:

NEGATIVE MTIME:
expect mtime=2159345162531
actual mtime=-2135622133469

INACCURATE MTIME:
expect mtime=1713037251360
actual mtime=1713037251359

Here's the gist to reproduce (on a Windows fs with support for at least millisecond timestamps):

var fs = require('fs');
var path = 'test-utimes-precision';
fs.writeFileSync(path, '');

console.log('\r\nNEGATIVE MTIME:');
var mtime = 2159345162531;
fs.utimesSync(path, mtime / 1000, mtime / 1000);
var stats = fs.statSync(path);
console.log('expect mtime=' + mtime);                 // 2159345162531
console.log('actual mtime=' + stats.mtime.getTime()); // -2135622133469

console.log('\r\nINACCURATE MTIME:');
var mtime = 1713037251360;
fs.utimesSync(path, mtime / 1000, mtime / 1000);
var stats = fs.statSync(path);
console.log('expect mtime=' + mtime);                 // 1713037251360
console.log('actual mtime=' + stats.mtime.getTime()); // 1713037251359

On Mac, the negative mtime is also an issue.

@addaleax addaleax added fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform. macos Issues and PRs related to the macOS platform / OSX. labels May 27, 2017
@jorangreef
Copy link
Contributor Author

Actually on Mac, there is no issue, only Windows.

@addaleax addaleax removed the macos Issues and PRs related to the macOS platform / OSX. label May 27, 2017
@jorangreef
Copy link
Contributor Author

I think the negative mtime has to do with a limit in NTFS (https://articles.forensicfocus.com/2013/04/06/interpretation-of-ntfs-timestamps/).

@jorangreef
Copy link
Contributor Author

That leaves the rounding error. Perhaps there could be an exception for an mtime that would overflow NTFS on Windows.

@refack
Copy link
Contributor

refack commented May 27, 2017

Hello @jorangreef , we already have a fix for the second issue (the bad rounding of time) #12607 (and it's spinoff #12818).
The second issue is caused by an overflow of the double that is the underlying type for JS number, and for that we have #13173

@jorangreef
Copy link
Contributor Author

Regarding an exception for an mtime that would overflow NTFS on Windows and lead to a negative mtime returned by fs.stats:

2147483647999 is the highest btime/mtime/atime value supported by NTFS.

Anything more than that overflows NTFS' internal structures. statSync then returns a valid Date object, but the getTime() on this Date object returns a negative Unix timestamp (negative relative to the Unix epoch).

@refack
Copy link
Contributor

refack commented May 29, 2017

@jorangreef It's not exactly an overflow, it's just bad interpretation. I think I have a fix in #13281

refack added a commit to refack/node that referenced this issue Jun 27, 2017
PR-URL: nodejs#13281
Fixes: nodejs#13255
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this issue Jun 29, 2017
PR-URL: #13281
Fixes: #13255
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this issue Jul 11, 2017
PR-URL: #13281
Fixes: #13255
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
addaleax pushed a commit that referenced this issue Jul 18, 2017
PR-URL: #13281
Fixes: #13255
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@refack refack removed their assignment Oct 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. windows Issues and PRs related to the Windows platform.
Projects
None yet
3 participants