Skip to content

Commit c99e3d1

Browse files
committed
fs: expand known issue to AIX and smartOS
1 parent cfb7408 commit c99e3d1

File tree

4 files changed

+151
-7
lines changed

4 files changed

+151
-7
lines changed

doc/api/fs.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -632,13 +632,15 @@ This method is only valid when using [`fs.lstat()`][].
632632

633633
The numeric identifier of the device containing the file.
634634

635+
*Note*: The `number` version is unreliable as values often overflow.
636+
635637
### stats.ino
636638

637639
* {number|bigint}
638640

639641
The file system specific "Inode" number for the file.
640642

641-
*Note*: The `number` version is unreliable on Windows as values often overflow.
643+
*Note*: The `number` version is unreliable as values often overflow.
642644

643645
### stats.mode
644646

@@ -670,6 +672,8 @@ The numeric group identifier of the group that owns the file (POSIX).
670672

671673
A numeric device identifier if the file is considered "special".
672674

675+
*Note*: The `number` version is unreliable as values often overflow.
676+
673677
### stats.size
674678

675679
* {number|bigint}

src/node_file.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,14 @@ constexpr uint64_t ToNative(uv_timespec_t ts) {
194194
template <typename NativeT, typename V8T>
195195
constexpr void FillStatsArray(AliasedBuffer<NativeT, V8T>* fields,
196196
const uv_stat_t* s, const size_t offset = 0) {
197-
fields->SetValue(offset + 0, gsl::narrow<NativeT>(s->st_dev));
197+
// Using the noop `narrow_cast` since this overflows.
198+
fields->SetValue(offset + 0, gsl::narrow_cast<NativeT>(s->st_dev));
198199
fields->SetValue(offset + 1, gsl::narrow<NativeT>(s->st_mode));
199200
fields->SetValue(offset + 2, gsl::narrow<NativeT>(s->st_nlink));
200201
fields->SetValue(offset + 3, gsl::narrow<NativeT>(s->st_uid));
201202
fields->SetValue(offset + 4, gsl::narrow<NativeT>(s->st_gid));
202-
fields->SetValue(offset + 5, gsl::narrow<NativeT>(s->st_rdev));
203+
// Using the noop `narrow_cast` since this overflows.
204+
fields->SetValue(offset + 5, gsl::narrow_cast<NativeT>(s->st_rdev));
203205
#if defined(__POSIX__)
204206
fields->SetValue(offset + 6, gsl::narrow<NativeT>(s->st_blksize));
205207
fields->SetValue(offset + 7, gsl::narrow<NativeT>(s->st_ino));

test/known_issues/test-fs-stat-ino-overflow-on-windows.js renamed to test/known_issues/test-fs-stat-overflow.js

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@
33
const common = require('../common');
44
const assert = require('assert');
55

6-
if (!common.isWindows)
6+
if (!(common.isWindows || common.isSunOS || common.isAIX))
77
assert.fail('Code should fail only on Windows.');
88

99
const fs = require('fs');
1010
const promiseFs = require('fs').promises;
1111
const path = require('path');
1212
const tmpdir = require('../common/tmpdir');
13-
const { isDate } = require('util').types;
1413

1514
tmpdir.refresh();
1615

@@ -23,8 +22,30 @@ function getFilename() {
2322
}
2423

2524
function verifyStats(bigintStats, numStats) {
26-
assert.ok(Number.isSafeInteger(numStats.ino));
27-
assert.strictEqual(bigintStats.ino, BigInt(numStats.ino));
25+
const keys = [
26+
'mode', 'nlink', 'uid', 'gid', 'size',
27+
// `rdev` can overflow on AIX and smartOS
28+
'rdev',
29+
// `dev` can overflow on AIX
30+
'dev',
31+
// `ino` can overflow on Windows
32+
'ino',
33+
];
34+
const nStats = keys.reduce(
35+
(s, k) => Object.assign(s, { [k]: String(numStats[k]) }),
36+
{}
37+
);
38+
const bStats = keys.reduce(
39+
(s, k) => Object.assign(s, { [k]: String(bigintStats[k]) }),
40+
{}
41+
);
42+
assert.deepStrictEqual(nStats, bStats);
43+
for (const key of keys) {
44+
assert.ok(
45+
Number.isSafeInteger(numStats[key]),
46+
`numStats.${key}: ${numStats[key]} is not a safe integer`
47+
);
48+
}
2849
}
2950

3051
{
Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
const fs = require('fs');
7+
const promiseFs = require('fs').promises;
8+
const path = require('path');
9+
const tmpdir = require('../common/tmpdir');
10+
11+
tmpdir.refresh();
12+
13+
let testIndex = 0;
14+
15+
function getFilename() {
16+
const filename = path.join(tmpdir.path, `test-file-${++testIndex}`);
17+
fs.writeFileSync(filename, 'test');
18+
return filename;
19+
}
20+
21+
function verifyStats(bigintStats, numStats) {
22+
const keys = [
23+
'mode', 'nlink', 'uid', 'gid', 'size',
24+
];
25+
if (!(common.isAIX || common.isSunOS)) {
26+
// `rdev` can overflow.
27+
keys.push('rdev');
28+
}
29+
if (!common.isAIX) {
30+
keys.push('dev');
31+
}
32+
if (!common.isWindows) {
33+
// These two are not defined on Windows.
34+
keys.push('blocks', 'blksize');
35+
// `ino` can overflow.
36+
keys.push('ino');
37+
}
38+
for (const key of keys) {
39+
const nVal = numStats[key];
40+
const bVal = bigintStats[key];
41+
assert.strictEqual(
42+
// coerce to string to get a common type to test
43+
String(bVal), String(nVal),
44+
`bigintStats.${key}: ${bVal} is not equal to numStats.${key}: ${nVal}`
45+
);
46+
assert.ok(
47+
Number.isSafeInteger(nVal),
48+
`numStats.${key}: ${nVal} is not a safe integer`
49+
);
50+
}
51+
}
52+
53+
{
54+
const filename = getFilename();
55+
const bigintStats = fs.statSync(filename, { bigint: true });
56+
const numStats = fs.statSync(filename);
57+
verifyStats(bigintStats, numStats);
58+
}
59+
60+
{
61+
const filename = __filename;
62+
const bigintStats = fs.statSync(filename, { bigint: true });
63+
const numStats = fs.statSync(filename);
64+
verifyStats(bigintStats, numStats);
65+
}
66+
67+
{
68+
const filename = __dirname;
69+
const bigintStats = fs.statSync(filename, { bigint: true });
70+
const numStats = fs.statSync(filename);
71+
verifyStats(bigintStats, numStats);
72+
}
73+
74+
{
75+
const filename = getFilename();
76+
const fd = fs.openSync(filename, 'r');
77+
const bigintStats = fs.fstatSync(fd, { bigint: true });
78+
const numStats = fs.fstatSync(fd);
79+
verifyStats(bigintStats, numStats);
80+
fs.closeSync(fd);
81+
}
82+
83+
{
84+
const filename = getFilename();
85+
fs.stat(filename, { bigint: true }, (err, bigintStats) => {
86+
fs.stat(filename, (err, numStats) => {
87+
verifyStats(bigintStats, numStats);
88+
});
89+
});
90+
}
91+
92+
{
93+
const filename = getFilename();
94+
const fd = fs.openSync(filename, 'r');
95+
fs.fstat(fd, { bigint: true }, (err, bigintStats) => {
96+
fs.fstat(fd, (err, numStats) => {
97+
verifyStats(bigintStats, numStats);
98+
fs.closeSync(fd);
99+
});
100+
});
101+
}
102+
103+
(async function() {
104+
const filename = getFilename();
105+
const bigintStats = await promiseFs.stat(filename, { bigint: true });
106+
const numStats = await promiseFs.stat(filename);
107+
verifyStats(bigintStats, numStats);
108+
})();
109+
110+
(async function() {
111+
const filename = getFilename();
112+
const handle = await promiseFs.open(filename, 'r');
113+
const bigintStats = await handle.stat({ bigint: true });
114+
const numStats = await handle.stat();
115+
verifyStats(bigintStats, numStats);
116+
await handle.close();
117+
})();

0 commit comments

Comments
 (0)