From 9920a0723ffb297a6e0019e28cdb202df8d767c3 Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Thu, 7 Jul 2022 17:43:06 +0800 Subject: [PATCH 1/3] fs: use signed types for stat data This allows to support timestamps before 1970-01-01. On Windows, it's not supported due to Y2038 issue. --- lib/internal/fs/utils.js | 11 ++-- src/aliased_buffer.h | 2 +- src/node_file-inl.h | 19 +++++-- src/node_file.h | 2 +- test/parallel/test-fs-stat-date.mjs | 79 +++++++++++++++++++++++++++++ 5 files changed, 102 insertions(+), 11 deletions(-) create mode 100644 test/parallel/test-fs-stat-date.mjs diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index cba646942785fe..0a91fb15a0e747 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -41,7 +41,7 @@ const { isArrayBufferView, isUint8Array, isDate, - isBigUint64Array + isBigInt64Array } = require('internal/util/types'); const { kEmptyObject, @@ -460,8 +460,11 @@ function nsFromTimeSpecBigInt(sec, nsec) { // 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 +// if the value is negative, we round it another way +// Refs: https://github.com/nodejs/node/pull/43714 function dateFromMs(ms) { - return new Date(Number(ms) + 0.5); + const msNumeric = Number(ms); + return new Date(msNumeric + (msNumeric >= 0 ? 0.5 : -0.5)); } function BigIntStats(dev, mode, nlink, uid, gid, rdev, blksize, @@ -526,12 +529,12 @@ Stats.prototype._checkModeProperty = function(property) { }; /** - * @param {Float64Array | BigUint64Array} stats + * @param {Float64Array | BigInt64Array} stats * @param {number} offset * @returns {BigIntStats | Stats} */ function getStatsFromBinding(stats, offset = 0) { - if (isBigUint64Array(stats)) { + if (isBigInt64Array(stats)) { return new BigIntStats( stats[0 + offset], stats[1 + offset], stats[2 + offset], stats[3 + offset], stats[4 + offset], stats[5 + offset], diff --git a/src/aliased_buffer.h b/src/aliased_buffer.h index 2cbc70aaa7c37c..6dda51c14615cc 100644 --- a/src/aliased_buffer.h +++ b/src/aliased_buffer.h @@ -307,7 +307,7 @@ typedef AliasedBufferBase AliasedInt32Array; typedef AliasedBufferBase AliasedUint8Array; typedef AliasedBufferBase AliasedUint32Array; typedef AliasedBufferBase AliasedFloat64Array; -typedef AliasedBufferBase AliasedBigUint64Array; +typedef AliasedBufferBase AliasedBigInt64Array; } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS diff --git a/src/node_file-inl.h b/src/node_file-inl.h index 351f3df809d94a..1fc55787291437 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -86,13 +86,22 @@ template void FillStatsArray(AliasedBufferBase* fields, const uv_stat_t* s, const size_t offset) { -#define SET_FIELD_WITH_STAT(stat_offset, stat) \ - fields->SetValue(offset + static_cast(FsStatsOffset::stat_offset), \ +#define SET_FIELD_WITH_STAT(stat_offset, stat) \ + fields->SetValue(offset + static_cast(FsStatsOffset::stat_offset), \ static_cast(stat)) -#define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \ - /* NOLINTNEXTLINE(runtime/int) */ \ +// On win32, time is stored in uint64_t and starts from 1601-01-01. +// libuv calculates tv_sec and tv_nsec from it and converts to signed long, +// which causes Y2038 overflow. The rest platforms should treat negative +// values as pre-epoch time. +#ifdef _WIN32 +#define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \ + /* NOLINTNEXTLINE(runtime/int) */ \ SET_FIELD_WITH_STAT(stat_offset, static_cast(stat)) +#else +#define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \ + SET_FIELD_WITH_STAT(stat_offset, static_cast(stat)) +#endif // _WIN32 SET_FIELD_WITH_STAT(kDev, s->st_dev); SET_FIELD_WITH_STAT(kMode, s->st_mode); @@ -233,7 +242,7 @@ FSReqBase* GetReqWrap(const v8::FunctionCallbackInfo& args, Environment* env = binding_data->env(); if (value->StrictEquals(env->fs_use_promises_symbol())) { if (use_bigint) { - return FSReqPromise::New(binding_data, use_bigint); + return FSReqPromise::New(binding_data, use_bigint); } else { return FSReqPromise::New(binding_data, use_bigint); } diff --git a/src/node_file.h b/src/node_file.h index cf98c5c933bb84..9d2834fa2673d6 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -18,7 +18,7 @@ class BindingData : public SnapshotableObject { explicit BindingData(Environment* env, v8::Local wrap); AliasedFloat64Array stats_field_array; - AliasedBigUint64Array stats_field_bigint_array; + AliasedBigInt64Array stats_field_bigint_array; std::vector> file_handle_read_wrap_freelist; diff --git a/test/parallel/test-fs-stat-date.mjs b/test/parallel/test-fs-stat-date.mjs new file mode 100644 index 00000000000000..c3b52f070cab18 --- /dev/null +++ b/test/parallel/test-fs-stat-date.mjs @@ -0,0 +1,79 @@ +import * as common from '../common/index.mjs'; + +// Test timestamps returned by fsPromises.stat and fs.statSync + +import fs from 'node:fs'; +import fsPromises from 'node:fs/promises'; +import path from 'node:path'; +import assert from 'node:assert'; +import tmpdir from '../common/tmpdir.js'; + +// On some platforms (for example, ppc64) boundaries are tighter +// than usual. If we catch these errors, skip corresponding test. +const ignoredErrors = new Set(['EINVAL', 'EOVERFLOW']); + +tmpdir.refresh(); +const filepath = path.resolve(tmpdir.path, 'timestamp'); + +await (await fsPromises.open(filepath, 'w')).close(); + +// Date might round down timestamp +function closeEnough(actual, expected, margin) { + // On ppc64, value is rounded to seconds + if (process.arch === 'ppc64') { + margin += 1000; + } + assert.ok(Math.abs(Number(actual - expected)) < margin, + `expected ${expected} ± ${margin}, got ${actual}`); +} + +async function runTest(atime, mtime, margin = 0) { + margin += Number.EPSILON; + try { + await fsPromises.utimes(filepath, new Date(atime), new Date(mtime)); + } catch (e) { + if (ignoredErrors.has(e.code)) return; + throw e; + } + + const stats = await fsPromises.stat(filepath); + closeEnough(stats.atimeMs, atime, margin); + closeEnough(stats.mtimeMs, mtime, margin); + closeEnough(stats.atime.getTime(), new Date(atime).getTime(), margin); + closeEnough(stats.mtime.getTime(), new Date(mtime).getTime(), margin); + + const statsBigint = await fsPromises.stat(filepath, { bigint: true }); + closeEnough(statsBigint.atimeMs, BigInt(atime), margin); + closeEnough(statsBigint.mtimeMs, BigInt(mtime), margin); + closeEnough(statsBigint.atime.getTime(), new Date(atime).getTime(), margin); + closeEnough(statsBigint.mtime.getTime(), new Date(mtime).getTime(), margin); + + const statsSync = fs.statSync(filepath); + closeEnough(statsSync.atimeMs, atime, margin); + closeEnough(statsSync.mtimeMs, mtime, margin); + closeEnough(statsSync.atime.getTime(), new Date(atime).getTime(), margin); + closeEnough(statsSync.mtime.getTime(), new Date(mtime).getTime(), margin); + + const statsSyncBigint = fs.statSync(filepath, { bigint: true }); + closeEnough(statsSyncBigint.atimeMs, BigInt(atime), margin); + closeEnough(statsSyncBigint.mtimeMs, BigInt(mtime), margin); + closeEnough(statsSyncBigint.atime.getTime(), new Date(atime).getTime(), margin); + closeEnough(statsSyncBigint.mtime.getTime(), new Date(mtime).getTime(), margin); +} + +// Too high/low numbers produce too different results on different platforms +{ + // TODO(LiviaMedeiros): investigate outdated stat time on FreeBSD. + // On Windows, filetime is stored and handled differently. Supporting dates + // after Y2038 is preferred over supporting dates before 1970-01-01. + if (!common.isFreeBSD && !common.isWindows) { + await runTest(-40691, -355, 1); // Potential precision loss on 32bit + await runTest(-355, -40691, 1); // Potential precision loss on 32bit + await runTest(-1, -1); + } + await runTest(0, 0); + await runTest(1, 1); + await runTest(355, 40691, 1); // Precision loss on 32bit + await runTest(40691, 355, 1); // Precision loss on 32bit + await runTest(1713037251360, 1713037251360, 1); // Precision loss +} From 0d1742a3edeced0c0af4c606cb61ea91eee65a11 Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Fri, 15 Jul 2022 00:09:01 +0800 Subject: [PATCH 2/3] squash: fixup --- lib/internal/fs/utils.js | 7 +++---- src/node_file-inl.h | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index 0a91fb15a0e747..f19463a7dacee2 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -12,6 +12,7 @@ const { NumberIsFinite, NumberIsInteger, MathMin, + MathRound, ObjectIs, ObjectPrototypeHasOwnProperty, ObjectSetPrototypeOf, @@ -454,17 +455,15 @@ function nsFromTimeSpecBigInt(sec, nsec) { return sec * kNsPerSecBigInt + nsec; } -// The Date constructor performs Math.floor() to the timestamp. +// The Date constructor performs Math.trunc() 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 -// if the value is negative, we round it another way // Refs: https://github.com/nodejs/node/pull/43714 function dateFromMs(ms) { - const msNumeric = Number(ms); - return new Date(msNumeric + (msNumeric >= 0 ? 0.5 : -0.5)); + return new Date(MathRound(Number(ms))); } function BigIntStats(dev, mode, nlink, uid, gid, rdev, blksize, diff --git a/src/node_file-inl.h b/src/node_file-inl.h index 1fc55787291437..1170d57754ffad 100644 --- a/src/node_file-inl.h +++ b/src/node_file-inl.h @@ -92,8 +92,8 @@ void FillStatsArray(AliasedBufferBase* fields, // On win32, time is stored in uint64_t and starts from 1601-01-01. // libuv calculates tv_sec and tv_nsec from it and converts to signed long, -// which causes Y2038 overflow. The rest platforms should treat negative -// values as pre-epoch time. +// which causes Y2038 overflow. On the other platforms it is safe to treat +// negative values as pre-epoch time. #ifdef _WIN32 #define SET_FIELD_WITH_TIME_STAT(stat_offset, stat) \ /* NOLINTNEXTLINE(runtime/int) */ \ From 7ab5e87b73da6e8714bef0e0b373e76b5d2c080d Mon Sep 17 00:00:00 2001 From: LiviaMedeiros Date: Mon, 18 Jul 2022 13:47:35 +0800 Subject: [PATCH 3/3] squash: fixup --- lib/internal/fs/utils.js | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/internal/fs/utils.js b/lib/internal/fs/utils.js index f19463a7dacee2..d1e521426f0bd3 100644 --- a/lib/internal/fs/utils.js +++ b/lib/internal/fs/utils.js @@ -40,9 +40,9 @@ const { } = require('internal/errors'); const { isArrayBufferView, - isUint8Array, + isBigInt64Array, isDate, - isBigInt64Array + isUint8Array, } = require('internal/util/types'); const { kEmptyObject, @@ -455,14 +455,15 @@ function nsFromTimeSpecBigInt(sec, nsec) { return sec * kNsPerSecBigInt + nsec; } -// The Date constructor performs Math.trunc() to the timestamp. -// https://www.ecma-international.org/ecma-262/#sec-timeclip +// The Date constructor performs Math.floor() on the absolute value +// of the timestamp: https://tc39.es/ecma262/#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 // Refs: https://github.com/nodejs/node/pull/43714 function dateFromMs(ms) { + // Coercing to number, ms can be bigint return new Date(MathRound(Number(ms))); }