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

fs.statSync, fs.stat and fs.promises.stat returns 'Invalid Date' for atime/ctime/mtime with negative epoch time #43707

Closed
hideishi-m opened this issue Jul 7, 2022 · 5 comments · Fixed by #43714
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system.

Comments

@hideishi-m
Copy link

hideishi-m commented Jul 7, 2022

Version

v16.15.1

Platform

Linux localhost.localdomain 4.18.0-394.el8.x86_64 #1 SMP Tue May 31 16:19:11 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

File system

What steps will reproduce the bug?

  1. create a file with minus epoch time.
$ touch --date=@-1 hogehoge
$ ls --full-time hogehoge
-rw-rw-r--. 1 kusanagi kusanagi 0 1969-12-31 23:59:59.000000000 +0000 hogehoge
  1. run the following code to test fs.statSync and fs.promises.stat for file hogehoge.
import { stat } from 'fs/promises';
import { statSync } from 'fs';

async function test() {
        const stats = await stat('hogehoge');
        console.log({stats});
        console.log(stats.mtime);
        console.log(stats.mtime.getTime());
}

function testSync() {
        const stats = statSync('hogehoge');
        console.log({stats});
        console.log(stats.mtime);
        console.log(stats.mtime.getTime());
}

await test();
testSync();
{
  stats: Stats {
    dev: 64768,
    mode: 33204,
    nlink: 1,
    uid: 1000,
    gid: 1000,
    rdev: 0,
    blksize: 4096,
    ino: 56395369,
    size: 0,
    blocks: 0,
    atimeMs: 1.8446744073709552e+22,
    mtimeMs: 1.8446744073709552e+22,
    ctimeMs: 1657161984281.557,
    birthtimeMs: 1657161921560.5312,
    atime: Invalid Date,
    mtime: Invalid Date,
    ctime: 2022-07-07T02:46:24.282Z,
    birthtime: 2022-07-07T02:45:21.561Z
  }
}
Invalid Date
NaN
{
  stats: Stats {
    dev: 64768,
    mode: 33204,
    nlink: 1,
    uid: 1000,
    gid: 1000,
    rdev: 0,
    blksize: 4096,
    ino: 56395369,
    size: 0,
    blocks: 0,
    atimeMs: 1.8446744073709552e+22,
    mtimeMs: 1.8446744073709552e+22,
    ctimeMs: 1657161984281.557,
    birthtimeMs: 1657161921560.5312,
    atime: Invalid Date,
    mtime: Invalid Date,
    ctime: 2022-07-07T02:46:24.282Z,
    birthtime: 2022-07-07T02:45:21.561Z
  }
}
Invalid Date
NaN

How often does it reproduce? Is there a required condition?

Always.

What is the expected behavior?

ctime/mtime/atime with negative epoch time shall be treated as it is.
In other words, if -1 then it shall be 1 second before epoch.

Actually, Date() supports negative epoch time.

$ node
Welcome to Node.js v16.15.1.
Type ".help" for more information.
> new Date(-1)
1969-12-31T23:59:59.999Z
>

As stat(2) supports negative epoch time, which can be observed via ls command, I don't see why fs.stat/fs.statSync/fs.promises.stat treats negative epoch as NaN.

What do you see instead?

$ touch --date=@-1 hogehoge
$ ls --full-time hogehoge
-rw-rw-r--. 1 kusanagi kusanagi 0 1969-12-31 23:59:59.000000000 +0000 hogehoge

$ cat > test.mjs
import { stat } from 'fs/promises';
import { statSync } from 'fs';

async function test() {
        const stats = await stat('hogehoge');
        console.log({stats});
        console.log(stats.mtime);
        console.log(stats.mtime.getTime());
}

function testSync() {
        const stats = statSync('hogehoge');
        console.log({stats});
        console.log(stats.mtime);
        console.log(stats.mtime.getTime());
}

await test();
testSync();

$ node test.mjs
{
  stats: Stats {
    dev: 64768,
    mode: 33204,
    nlink: 1,
    uid: 1000,
    gid: 1000,
    rdev: 0,
    blksize: 4096,
    ino: 56395369,
    size: 0,
    blocks: 0,
    atimeMs: 1.8446744073709552e+22,
    mtimeMs: 1.8446744073709552e+22,
    ctimeMs: 1657161984281.557,
    birthtimeMs: 1657161921560.5312,
    atime: Invalid Date,
    mtime: Invalid Date,
    ctime: 2022-07-07T02:46:24.282Z,
    birthtime: 2022-07-07T02:45:21.561Z
  }
}
Invalid Date
NaN
{
  stats: Stats {
    dev: 64768,
    mode: 33204,
    nlink: 1,
    uid: 1000,
    gid: 1000,
    rdev: 0,
    blksize: 4096,
    ino: 56395369,
    size: 0,
    blocks: 0,
    atimeMs: 1.8446744073709552e+22,
    mtimeMs: 1.8446744073709552e+22,
    ctimeMs: 1657161984281.557,
    birthtimeMs: 1657161921560.5312,
    atime: Invalid Date,
    mtime: Invalid Date,
    ctime: 2022-07-07T02:46:24.282Z,
    birthtime: 2022-07-07T02:45:21.561Z
  }
}
Invalid Date
NaN

Additional information

No response

@targos
Copy link
Member

targos commented Jul 7, 2022

@nodejs/fs

@bnoordhuis bnoordhuis added confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system. labels Jul 7, 2022
@bnoordhuis
Copy link
Member

bnoordhuis commented Jul 7, 2022

It's a signed-to-unsigned conversion bug:

$ touch -t 196912312359.59 x && node -p 'fs.statSync("x", {bigint: true})' | grep Ns
  atimeNs: 18446744073709548015000000000n,
  mtimeNs: 18446744073709548015000000000n,
  ctimeNs: 1657177398636601096n,
  birthtimeNs: 1657177398636510656n,

Internally, the stats are handed off from C++ to JS in a BigUint64Array but that makes negative values wrap around. The fix is to use a BigInt64Array instead. Pull request welcome.

edit: in particular, it's this (insidiously misnamed) field:

AliasedBigUint64Array stats_field_bigint_array;

And here is where it's converted to a stats object:
if (isBigUint64Array(stats)) {
return new BigIntStats(
stats[0 + offset], stats[1 + offset], stats[2 + offset],
stats[3 + offset], stats[4 + offset], stats[5 + offset],
stats[6 + offset], stats[7 + offset], stats[8 + offset],
stats[9 + offset],
nsFromTimeSpecBigInt(stats[10 + offset], stats[11 + offset]),
nsFromTimeSpecBigInt(stats[12 + offset], stats[13 + offset]),
nsFromTimeSpecBigInt(stats[14 + offset], stats[15 + offset]),
nsFromTimeSpecBigInt(stats[16 + offset], stats[17 + offset])
);
}

@LiviaMedeiros
Copy link
Contributor

Non-bigint stats are returned as Float64Array so perhaps this requires adjusting unsigned long longs (or what it's aliased to) in C++ bindings as well.

@bnoordhuis
Copy link
Member

Sorry yes, I forgot to mention that. That logic is here:

node/src/node_file-inl.h

Lines 93 to 95 in 7d13f5e

#define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \
/* NOLINTNEXTLINE(runtime/int) */ \
SET_FIELD_WITH_STAT(stat_offset, static_cast<unsigned long>(stat))

nodejs-github-bot pushed a commit that referenced this issue Jul 18, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: #43714
Fixes: #43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
@LiviaMedeiros
Copy link
Contributor

Note: on Windows platform, there still is an overflow on negative dates, to postpone Y2038 overflow.
It shouldn't lead to Invalid Date, and right now is unavoidable without breaking underlying ABI.

danielleadams pushed a commit that referenced this issue Jul 26, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: #43714
Fixes: #43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this issue Jul 31, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: #43714
Fixes: #43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
targos pushed a commit that referenced this issue Jul 31, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: #43714
Fixes: #43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
LiviaMedeiros added a commit to LiviaMedeiros/node that referenced this issue Aug 4, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: nodejs#43714
Fixes: nodejs#43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
LiviaMedeiros added a commit to LiviaMedeiros/node that referenced this issue Aug 4, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: nodejs#43714
Fixes: nodejs#43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Backport-PR-URL: https://github.com/nodejs/node/pull/00000
LiviaMedeiros added a commit to LiviaMedeiros/node that referenced this issue Aug 4, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: nodejs#43714
Fixes: nodejs#43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Backport-PR-URL: nodejs#44129
LiviaMedeiros added a commit to LiviaMedeiros/node that referenced this issue Aug 4, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: nodejs#43714
Fixes: nodejs#43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Backport-PR-URL: nodejs#44129
targos pushed a commit that referenced this issue Sep 5, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: #43714
Fixes: #43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Fyko pushed a commit to Fyko/node that referenced this issue Sep 15, 2022
This allows to support timestamps before 1970-01-01.
On Windows, it's not supported due to Y2038 issue.

PR-URL: nodejs#43714
Fixes: nodejs#43707
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Antoine du Hamel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. fs Issues and PRs related to the fs subsystem / file system.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants