Skip to content

Commit 5e69527

Browse files
refackrefack
committed
src: expand known issue
1 parent 01c7de7 commit 5e69527

File tree

4 files changed

+139
-16
lines changed

4 files changed

+139
-16
lines changed

‎doc/api/fs.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,7 @@ The numeric identifier of the device containing the file.
638638

639639
The file system specific "Inode" number for the file.
640640

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

643643
### stats.mode
644644

‎src/node_file.h

+13-12
Original file line numberDiff line numberDiff line change
@@ -194,22 +194,23 @@ 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));
198-
fields->SetValue(offset + 1, gsl::narrow<NativeT>(s->st_mode));
199-
fields->SetValue(offset + 2, gsl::narrow<NativeT>(s->st_nlink));
200-
fields->SetValue(offset + 3, gsl::narrow<NativeT>(s->st_uid));
201-
fields->SetValue(offset + 4, gsl::narrow<NativeT>(s->st_gid));
202-
fields->SetValue(offset + 5, gsl::narrow<NativeT>(s->st_rdev));
197+
fields->SetValue(offset + 0, gsl::narrow_cast<NativeT>(s->st_dev));
198+
fields->SetValue(offset + 1, gsl::narrow_cast<NativeT>(s->st_mode));
199+
fields->SetValue(offset + 2, gsl::narrow_cast<NativeT>(s->st_nlink));
200+
fields->SetValue(offset + 3, gsl::narrow_cast<NativeT>(s->st_uid));
201+
fields->SetValue(offset + 4, gsl::narrow_cast<NativeT>(s->st_gid));
202+
fields->SetValue(offset + 5, gsl::narrow_cast<NativeT>(s->st_rdev));
203203
#if defined(__POSIX__)
204-
fields->SetValue(offset + 6, gsl::narrow<NativeT>(s->st_blksize));
205-
fields->SetValue(offset + 7, gsl::narrow<NativeT>(s->st_ino));
206-
fields->SetValue(offset + 8, gsl::narrow<NativeT>(s->st_size));
207-
fields->SetValue(offset + 9, gsl::narrow<NativeT>(s->st_blocks));
204+
fields->SetValue(offset + 6, gsl::narrow_cast<NativeT>(s->st_blksize));
205+
// Using the noop `narrow_cast` since this overflows.
206+
fields->SetValue(offset + 7, gsl::narrow_cast<NativeT>(s->st_ino));
207+
fields->SetValue(offset + 8, gsl::narrow_cast<NativeT>(s->st_size));
208+
fields->SetValue(offset + 9, gsl::narrow_cast<NativeT>(s->st_blocks));
208209
#else
209210
fields->SetValue(offset + 6, 0);
210-
// This overflows on Windows for NativeT == double
211+
// Using the noop `narrow_cast` since this overflows.
211212
fields->SetValue(offset + 7, gsl::narrow_cast<NativeT>(s->st_ino));
212-
fields->SetValue(offset + 8, gsl::narrow<NativeT>(s->st_size));
213+
fields->SetValue(offset + 8, gsl::narrow_cast<NativeT>(s->st_size));
213214
fields->SetValue(offset + 9, 0);
214215
#endif
215216

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

+19-3
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ 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,25 @@ 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+
'dev', 'mode', 'nlink', 'uid',
27+
'gid', 'rdev', 'ino', 'size',
28+
];
29+
if (!common.isWindows) {
30+
keys.push('blocks', 'blksize');
31+
}
32+
for (const key of keys) {
33+
const nVal = numStats[key];
34+
const bVal = bigintStats[key];
35+
assert.ok(
36+
Number.isSafeInteger(nVal),
37+
`numStats.${key}: ${nVal} is not a safe integer`
38+
);
39+
assert.strictEqual(
40+
bigintStats[key], BigInt(numStats[key]),
41+
`bigintStats.${key}: ${bVal} is not equal to numStats.${key}: ${nVal}`
42+
);
43+
}
2844
}
2945

3046
{

‎test/parallel/test-fs-stat-ino-overflow.js

+106
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
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+
'dev', 'mode', 'nlink', 'uid', 'gid', 'rdev', 'size',
24+
];
25+
if (!common.isWindows) {
26+
keys.push('blocks', 'blksize', 'ino');
27+
}
28+
for (const key of keys) {
29+
const nVal = numStats[key];
30+
const bVal = bigintStats[key];
31+
assert.strictEqual(
32+
bigintStats[key], BigInt(numStats[key]),
33+
`bigintStats.${key}: ${bVal} is not equal to numStats.${key}: ${nVal}`
34+
);
35+
assert.ok(
36+
Number.isSafeInteger(nVal),
37+
`numStats.${key}: ${nVal} is not a safe integer`
38+
);
39+
}
40+
}
41+
42+
{
43+
const filename = getFilename();
44+
const bigintStats = fs.statSync(filename, { bigint: true });
45+
const numStats = fs.statSync(filename);
46+
verifyStats(bigintStats, numStats);
47+
}
48+
49+
{
50+
const filename = __filename;
51+
const bigintStats = fs.statSync(filename, { bigint: true });
52+
const numStats = fs.statSync(filename);
53+
verifyStats(bigintStats, numStats);
54+
}
55+
56+
{
57+
const filename = __dirname;
58+
const bigintStats = fs.statSync(filename, { bigint: true });
59+
const numStats = fs.statSync(filename);
60+
verifyStats(bigintStats, numStats);
61+
}
62+
63+
{
64+
const filename = getFilename();
65+
const fd = fs.openSync(filename, 'r');
66+
const bigintStats = fs.fstatSync(fd, { bigint: true });
67+
const numStats = fs.fstatSync(fd);
68+
verifyStats(bigintStats, numStats);
69+
fs.closeSync(fd);
70+
}
71+
72+
{
73+
const filename = getFilename();
74+
fs.stat(filename, { bigint: true }, (err, bigintStats) => {
75+
fs.stat(filename, (err, numStats) => {
76+
verifyStats(bigintStats, numStats);
77+
});
78+
});
79+
}
80+
81+
{
82+
const filename = getFilename();
83+
const fd = fs.openSync(filename, 'r');
84+
fs.fstat(fd, { bigint: true }, (err, bigintStats) => {
85+
fs.fstat(fd, (err, numStats) => {
86+
verifyStats(bigintStats, numStats);
87+
fs.closeSync(fd);
88+
});
89+
});
90+
}
91+
92+
(async function() {
93+
const filename = getFilename();
94+
const bigintStats = await promiseFs.stat(filename, { bigint: true });
95+
const numStats = await promiseFs.stat(filename);
96+
verifyStats(bigintStats, numStats);
97+
})();
98+
99+
(async function() {
100+
const filename = getFilename();
101+
const handle = await promiseFs.open(filename, 'r');
102+
const bigintStats = await handle.stat({ bigint: true });
103+
const numStats = await handle.stat();
104+
verifyStats(bigintStats, numStats);
105+
await handle.close();
106+
})();

0 commit comments

Comments
 (0)